+Revise equation of state interfaces for consistency #118
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removed unused module use statements for EOS_type, calculate_density or calculate_density_derivs in 20 files. All answers are bitwise identical.
Added comments documenting the units of the variables in diagnoseMLDbyEnergy and slightly refactored this routine to clean up its call to calculate_density and eliminated a redundant array of interface depths. Also fixed several spelling errors in comments. All answers and diagnostics are bitwise identical.
Documented the units of variables as they actually appear in subroutine calls to various equation of state or density integral routines, eliminating the potentially confusing lists of alternative units in comments. Only comments are changed, and all answers are bitwise identical.
Revised the calculation of the drho_dT and drho_dS diagnostics to use dimensional consistency testing, along with the newer interface to calculate_density that takes dimensionally rescaled arguments. With this change, the units of most the variables in this section of code match their descriptions in comments, although there is still the local re-use of some 3-d arrays as temporaries with units that do not match. All answers and output are bitwise identical.
Refactored MOM_density_integrals to use the newer calculate_density_1d() and calculated_stanley_density_1d() interfaces to the equation of state routines, and to thereby shift all related dimensional rescaling into MOM_EOS.F90. Also revised the comments describing the arguments to a number of the equation of state routines to eliminate confusing options and clearly indicate the units of each input and output variable. As a part of this change, the units of the rho_ref argument to calculate_stanley_density_1d were changed from [kg m-3] to [R ~> kg m-3] to match the equivalent routine calculate_density_1d(). Because this does not appear to have been used previously, this should not be a problem, and answers will not change unless a dimensional consistency test is underway. All answers are bitwise identical, but there is one minor change to the rescaled units of one apparently unused optional argument.
Modified the comments describing the units of the arguments to int_density_dz_wright, int_spec_vol_dp_wright int_density_dz_linear and int_spec_vol_dp_linear so that they reflect the units as they are used in practice where they are called from analytic_int_density_dz or analytic_int_specific_vol_dp. All answers are bitwise identical.
This commit has several interface changes to some little-called equation of state routines to follow the patterns set by the most commonly used equation of state routines. All answers in test cases are bitwise identical. - Added calculate_TFreeze_1d to the overloaded interface calculate_TFreeze, with dimensional rescaling of its arguments taken from its EOS_type argument and an optional two-element domain, rather than two mandatory integer arguments used with calculate_TFreeze_array. The older interface is also retained within the overloaded interface to calculate_TFreeze. - Modified calculate_density_scalar and calculate_stanley_density_scalar to use units of [R ~> kg m-3] for its rho_ref optional argument, following the pattern from calculate_density_1d. These arguments were not previously used. - Renamed the internally visible routine calculate_density_second_derivs_array to calculate_density_second_derivs_1d and changed its argument list to take an optional two-element domain, rather than two mandatory integer arguments, to follow the pattern set by calculate_density_derivs_1d. Because this routine was only being called in two places the older interface is not being preserved in the overloaded interface calculate_density_second_derivs. - Renamed the internally visible routine calculate_compress_array to calculate_compress_1d and changed its argument list to take an optional two-element domain, rather than two mandatory integer arguments, to follow the pattern set by calculate_density_derivs_1d. Because this routine was only being called in one place the older interface is not being preserved in the overloaded interface calculate_compress. - Eliminated some unnecessary local variables (mostly p_scale) for brevity and code clarity. - Modified two calls to calculate_density_second_derivs in thickness_diffuse_full to use its revised interface. - Modified one call to calculate_compress in build_slight_column to use its revised interface.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #118 +/- ##
============================================
+ Coverage 28.78% 28.81% +0.02%
============================================
Files 249 249
Lines 72991 72934 -57
============================================
+ Hits 21010 21015 +5
+ Misses 51981 51919 -62
Continue to review full report at Codecov.
|
Corrected a bug in the calculation of drho_dS_dP and drho_dT_dP in the calculate_density_second_derivs routines, where the inverse of the correct rescaling was being used. However, these routines are only called in a very few places and these particular output fields are not being used, so this bug does not alter any existing MOM6 solutions.
|
This PR is the starting point for PR #122, so please use a git merge instead of a rebase to reconcile this with any changes in its base branch, to avoid having duplicated commits between the two PRs. Each of the commits here are potentially significant for future git bisections, so please do not squash this commit. |
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/15535 ✔️ |
Since this hasn't been merged yet, do you just want to rebase #122 onto this one? |
|
Nevermind, I'll just do the regular merge. Though perhaps we should discourage PRs which build on other PRs like this. |
marshallward
approved these changes
May 19, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes a series of commits that standardize some of the less-used
interfaces or arguments to the MOM6 equation of state routines and how they are
used in a few places. There is a particular emphasis on the consistency of
which routines use rescaled variables or MKS variables, so that essentially all
of the dimensional rescaling related to the equation of state now occurs inside
of the routines in MOM_EOS.F90. By design, there is not expected to be any
noticeable performance impact. The vast majority of calls to the equation of
state routines are unaltered. All answers in test cases are bitwise identical,
but there are a few (probably untested) places where the rescaled units of some
arguments are corrected.
Changes of note include:
Refactored MOM_density_integrals to use the newer calculate_density_1d() and
calculated_stanley_density_1d() interfaces to the equation of state routines,
and to thereby shift all related dimensional rescaling into MOM_EOS.F90.
Revised the calculation of the drho_dT and drho_dS diagnostics to use
dimensional consistency testing, along with the newer interface to
calculate_density_1d
Added calculate_TFreeze_1d to the overloaded interface calculate_TFreeze,
with dimensional rescaling of its arguments taken from its EOS_type argument
and an optional two-element domain, rather than two mandatory integer
arguments used with calculate_TFreeze_array. The older interface is also
retained within the overloaded interface to calculate_TFreeze.
Modified calculate_density_scalar and calculate_stanley_density_scalar to use
units of [R ~> kg m-3] for its rho_ref optional argument, following the
pattern from calculate_density_1d. These arguments were not previously used.
Renamed the internally visible routine calculate_density_second_derivs_array
to calculate_density_second_derivs_1d and changed its argument list to take
an optional two-element domain, rather than two mandatory integer arguments,
to follow the pattern set by calculate_density_derivs_1d. Because this
routine was only being called in two places the older interface is not being
preserved in the overloaded interface calculate_density_second_derivs.
Renamed the internally visible routine calculate_compress_array
to calculate_compress_1d and changed its argument list to take
an optional two-element domain, rather than two mandatory integer arguments,
to follow the pattern set by calculate_density_derivs_1d. Because this
routine was only being called in one place the older interface is not being
preserved in the overloaded interface calculate_compress.
Eliminated some unnecessary local variables (mostly p_scale) for brevity and
code clarity.
Modified two calls to calculate_density_second_derivs in
thickness_diffuse_full to use its revised interface.
Modified one call to calculate_compress in build_slight_column to use
its revised interface.
Made the description of units more precise in numerous comments describing
the equation of state calls.
Eliminated numerous unused module use statements for EOS routines
The commits in this commit include: