Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bad gotm path in ocean build #862

Closed
wants to merge 1,146 commits into from
Closed

Fix bad gotm path in ocean build #862

wants to merge 1,146 commits into from

Conversation

philipwjones
Copy link
Contributor

The current path in the ocean core build_options.mk for the gotm package points to the wrong directory for module files, causing build failures for PGI compiler. This PR points to the proper directory.
bit for bit
Fixes #861

matthewhoffman and others added 30 commits December 14, 2020 20:21
This commit makes two changes necessary to get correct behavior with
extended cells used by Albany.

The extension of the mesh for Albany requires 3 to cover all geometric possibilities.

The thermal minimum thickness is set to 0 to ensure reasonable values everywhere there
is ice, which is needed for realistic behavior when using Albany due to the way
temperature is handled in extended cells.
This merge makes two changes necessary to get correct behavior with extended cells used by Albany.

* The extension of the mesh for Albany requires 3 to cover all geometric possibilities to ensure bit reproducibility.

* The thermal minimum thickness is set to 0 to ensure reasonable values everywhere there
    is ice, which is needed for realistic behavior when using Albany due to the way
    temperature is handled in extended cells.

* landice/increase_halo:
  Update halos to 3; set thermal thickness min to 0
Step therm1 used vertex ice velocity instead of cell ice velocity
Fixed minor bug with testing system
Added a square test case
Fixed vertexDegree = 4 runs
This is needed because in setting up the vertical diffusion solve, there
is a choice of basal b.c. depending on whether the bed is frozen or
thawed.  In some situations that won't be treated properly if the
basalTemperature field is not present on a restart.
This commit adjusts the Interface to only use MPAS temperatures values
from grid cells where the thickness exceeds the thickness limit used by
the thermal solver.  This avoids inconsistencies between what are
considered valid temperatures that can lead to bad temperature values.

Note that this condition is followed even if the MPAS thermal solver is
off.  So config_thermal_thickness has meaning to the interface even if
the thermal solver is not being used.
This merge fixes subtle issues with how temperature is handled:
* Fixes a bug in basalThickness on restarts by adding it as a new
restart field.
* Forces the interface to use the same definition of which cells have
valid temperatures as MPAS is using.

* landice/thermal_consistency:
  In interface, only use temperatures where MPAS calculates them
  Add basalTemperature as restart variable
Added linear as well as quadratic ocean stress
Added periodic velocity boundaries
…estcase

Added one dimensional velocity testcase
Widen Redi halo in hmix, correct OpenMP private variables #780
Codes for a semi implicit barotropic mode solver #422
Added one dimensional velocity hex test case
…/develop

This merge fixes a bug in the scaling of thermal thickness limit that
was introduced in bcc5907

* landice/fix_thermal_thickness_scaling_bug:
  Correct scaling of thermal thickness limit
Merge PR #583 'vanroekel/ocean/addCosineBellTest' into ocean/develop
This is the first phase of a GPU implementation of all the baroclinic velocity tendencies. It contains a subset of the necessary changes for running on the GPU that retain bit-for-bit accuracy with the current version when running on the CPU
matthewhoffman and others added 10 commits April 27, 2021 15:42
This merge fixes a few intel build errors in E3SM:
* Adds missing RKIND specifications to variable declarations that Intel errs on
* Adds missing update to cmake build system for the bedtopo module

* matt/landice/intel_errors:
  Add missing mpas_li_bedtopo.F to cmake build
  Add missing (kind=RKIND) in variable declarations
FO velocity solver requires 3 halos.
edgeMask calculation also requires it.
Checks have been put in both routines so that if one of those
restrictions is ever relaxed, the other will still be triggered.
A previous commit changed the default num_halos but did not ensure the
runtime namelist was actually set that way.
This merge adds checks in the code that the runtime namelist is using the
correct number of halos.

* landice/halo_check:
  Add error checks for required number of halos (3)
This merge updates MALI in e3sm/develop for the first time since
9/3/2020.
This update serves two purposes:
* Bring in recently added calving laws and some bug fixes with temperature
* Bring E3SM up to date before transitioning from submodules to inline code

* landice/develop: (197 commits)
  Add error checks for required number of halos (3)
  Add missing mpas_li_bedtopo.F to cmake build
  Add missing (kind=RKIND) in variable declarations
  Delint MALI files
  Fix quotation mark issue in Registry
  Call subroutine specified_calving_velocity
  Make calving_rate option use the damage threshold for calving to initiate
  Rename alpha to principalStrainRateRatio
  General cleanup
  Undo eigencalving hijack that was made for testing
  Cleanup related to config_damage_calving_method option
  Add additional comments about 2-back scheme for posterity
  Remove trailing whitespace
  Make damage summary stats work on multiple procs
  Correct usage of calvingFrontMask in damage_calving routine
  Remove unneeded code from damage routines
  Update Registry description of tauMin/Max to indicate they are deviatoric
  Move nstar calculation into where loop to avoid divide by zero
  Use grounding line instead of dynamic cell mask
  Fix asymmetry in calving at grounded margins
  ...
isnan() is not Fortran standard and causes an error on PGI.
Also, this check was checking the wrong variable.
This merge removes an instance of isnan(). It is not Fortran standard and causes an error on PGI.

* landice/remove_isnan:
  Switch ieee_isnan
  Remove isnan() from calving routine
Add fix for isnan error on PGI.

* origin/landice/develop:
  Switch ieee_isnan
  Remove isnan() from calving routine
Merge PR #607 'nairita87/topographic_wave_drag' into ocean/develop
Copy link

@qingli411 qingli411 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks, @philipwjones!

Just curious, why it was working fine with GNU and Intel?

@philipwjones
Copy link
Contributor Author

@qingli411 - Within core_ocean the paths appear in two places. In the Makefile they are added to OCEAN_SHARED_INCLUDES and these are used to build the dycore library and were set correctly. But for some reason, PGI was also looking for module files at the higher mpas_subdriver level, even though there are no explicit use statements for the modules. PGI seems to follow the recursive "use" statements down the call stack? In that case, the include paths added in the build_options are being used by the higher-level makefile and the correct path wasn't there. Apparently the other compilers are only going off the explicit use statements so this isn't an issue. It's probably not great to have these set in multiple places and the build system needs some work, but this fix works for now.

@qingli411
Copy link

@philipwjones OK I see. Thanks!

mark-petersen added a commit to mark-petersen/E3SM that referenced this pull request May 8, 2021
@mattdturner
Copy link
Collaborator

It looks like the change in this PR is no longer necessary, as the include path for GOTM within E3SM is already correct. This can be closed.

@mark-petersen
Copy link
Contributor

Yes, I added this change in the transplant, as I was messing with the makefiles anyway. Thanks for closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.