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

MOM6: +Preparations to allow reference heights to change #1467

Merged
merged 10 commits into from
Aug 22, 2021

Conversation

Hallberg-NOAA
Copy link
Collaborator

This PR includes various commits that prepare to allow for the reference
heights for bathymetry and interfaces to be changed, or corrects problems that
were exposed in the process of developing that capability, but which do not
themselves change any answers or algorithms. This set of changes includes the
addition of several new optional arguments, and it adds comments noting (but
does not fix) some buts that seem likely to change answers. These new
capabilities have been tested and work, but they will not be exercised until the
subsequent commit that implements the ability to change reference heights. All
answers and output are bitwise identical with the changes in this PR.

The commits in this PR include:

  Added an optional dZref argument to the two find_eta routines so that the
bathymetry can use a different reference height than is used for the interface
heights.  By default, they use the same reference height and all answers are
bitwise identical.  There is a new optional arguments (that is not yet being
exercised with this commit, but has been tested and will be used in an upcoming
commit) to two publicly visible interfaces.
  Eliminated the depth argument to check_grid_column, along with some internal
calculations that are never used.  Also corrected some comments.  All answers
are bitwise identical.
  Lengthened a message string in get_polynomial_coordinate so that it will give
a valid fatal error message in some cases of failures rather than resulting a
segmentation fault with no message output.  All answers are bitwise identical.
  Added an optional Z_0p argument to the various Boussinesq density integral
routines as a new intercept for the linear expression between the (arbitrary)
interface heights and pressure used for the equation of state routines.  If
omitted, this is equivalent to setting this intercept to 0, and all answers are
bitwise identical.  There are new optional arguments to several publicly visible
interfaces (that are not yet being exercised with this commit, but have been
tested and will be used in an upcoming commit).
  Corrected error messages to indicate the right subroutine in
rescale_dyn_horgrid_bathymetry.  All answers are bitwise identical.
  Added the optional argument dZ_ref_eta to adjustEtaToFitBathymetry so that the
bathymetry can use a different reference height than is used in this routine.
Also changed the name and reversed the sign of the depth (now Z_bot) argument to
find_interfaces, and a new Z_bottom array in MOM_temp_salt_initialize_from_Z.
An extra unit conversion factor was eliminated from MOM_state_init_tests.  All
answers are bitwise identical, and all of these changes are internal to this
module.
  Extensive inconsequential cleanup in MOM_ALE_sponge.F90, including the
elimination of a dozen unnecessary index-range elements of the ALE_sponge_CS and
modifying a number of comments to be much more descriptive.  This included the
correction of numerous spelling errors and other typos in comments.  All answers
are bitwise identical.
  Corrected dimensional inconsistencies in the negligible thicknesses in the
denominators of the expressions for the topographic betas in MEKE_equilibrium
and MEKE_lengthScales.  This could change answers in strange cases, but seems
unlikely to do so (partly because it is in a max expression, and not added), and
did not change any answers in the MOM6-examples test suite.
  Added comments denoted with "###" indicating bugs or conceptual errors in
update_OBC_segment_data, horizontal_viscosity, thickness_diffuse and wave_speed.
Only comments and the case of some indices are changed in this commit, and all
answers are bitwise identical.  Actually correcting these bugs would probably
change answers in some cases.
@codecov
Copy link

codecov bot commented Aug 15, 2021

Codecov Report

Merging #1467 (23d0076) into dev/gfdl (a29bff9) will increase coverage by 0.00%.
The diff coverage is 21.64%.

Impacted file tree graph

@@            Coverage Diff            @@
##           dev/gfdl    #1467   +/-   ##
=========================================
  Coverage     29.12%   29.13%           
=========================================
  Files           235      235           
  Lines         71045    71056   +11     
=========================================
+ Hits          20694    20703    +9     
- Misses        50351    50353    +2     
Impacted Files Coverage Δ
src/ALE/MOM_regridding.F90 21.45% <0.00%> (+0.09%) ⬆️
src/ALE/regrid_interp.F90 1.41% <ø> (ø)
src/core/MOM_open_boundary.F90 20.00% <0.00%> (ø)
src/diagnostics/MOM_wave_speed.F90 23.68% <0.00%> (ø)
src/framework/MOM_dyn_horgrid.F90 62.68% <0.00%> (ø)
src/parameterizations/lateral/MOM_hor_visc.F90 43.07% <0.00%> (ø)
...arameterizations/lateral/MOM_thickness_diffuse.F90 31.52% <0.00%> (ø)
src/initialization/MOM_state_initialization.F90 20.92% <3.57%> (-0.01%) ⬇️
src/core/MOM_density_integrals.F90 9.27% <15.00%> (+0.06%) ⬆️
src/core/MOM_interface_heights.F90 37.64% <27.27%> (+0.61%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a29bff9...23d0076. Read the comment docs.

@herrwang0
Copy link
Contributor

herrwang0 commented Aug 17, 2021

There is a parameter called Z_ref in ocean_grid_type introduced a couple years ago.
https://github.com/NOAA-GFDL/MOM6/blob/e88cdaa785b9fab4e24286031bdd409caf673034/src/core/MOM_grid.F90#L213

It is only used once in the barotropic solver (calculating IDat[uv]). Does it make sense to utilize that for changing reference height in various subroutines (instead of optional input)? or do we want to make the variable more explicit as subroutine input argument (and maybe allow it to vary across subroutines)?

@Hallberg-NOAA
Copy link
Collaborator Author

@herrwang0 , the plan is to use exactly this G%Z_ref variable in numerous places in the code in an upcoming PR, but before doing so there are several changes that we will be making to correct bugs or otherwise minimize the locations where this variable will be used. The current PR is one of these preliminary steps.

Copy link
Collaborator

@marshallward marshallward left a comment

Choose a reason for hiding this comment

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

Adds some new computational work (e.g. MOM_density_integrals) but the offset is a fixed scalar and does not require array access, so I would not expect this to affect runtime.

@marshallward
Copy link
Collaborator

marshallward commented Aug 22, 2021

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/13413 ✔️

@marshallward marshallward merged commit cf67c92 into mom-ocean:dev/gfdl Aug 22, 2021
@Hallberg-NOAA Hallberg-NOAA deleted the Z_ref_prelude branch November 22, 2021 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants