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

Add dimensional rescaling of temperature and salinity #122

Merged
merged 13 commits into from
May 22, 2022

Conversation

Hallberg-NOAA
Copy link
Member

@Hallberg-NOAA Hallberg-NOAA commented May 4, 2022

This PR includes a series of commits that systematically add the ability to do
dimensional consistency testing for temperatures and salinities, as specified
with the new runtime parameters C_RESCALE_POWER and S_RESCALE_POWER. These
changes have been tested, and they give identical answers with this rescaling
for almost all of the test cases, with the adjustment_2d/z test case having
issues with on hard-coded tracer concentration in the PPM vertical remapping
code. All answers are bitwise identical, but there are new or corrected entries
in the MOM_parameter_doc files. The commits in this PR include:

  • 1c6696427 Rescaled dumbbell_initialize_thickness salinities
  • 079fd3e44 +Add cons_temp_to_pot_temp & abs_saln_to_prac_saln
  • b3c41b105 +Rescale vars in MOM_temp_salt_initialize_from_Z
  • 30d3e70b6 Merge branch 'EOS_interface_cleanup' into EOS_rescale_prep
  • 31d411776 Correct a few more temperature and salin units
  • 6d78d2b0a +Add rescaling for temperature and salinity (2)
  • 5a89a9d0c +Add rescaling for temperature and salinity (1)
  • 44a786178 +Add scale argument to set_up_ALE_sponge
  • 47f13924c Modify more units in temperature and saln comments
  • 14a222ea3 Use simpler calculate_TFreeze interfaces
  • b8e599034 +Temperature and salinity rescaling in MOM_EOS.F90
  • 5d88f2e47 Modify units in temperature and salinity comments

  Modified comments in 20 files to prepare for the addition of dimensional
rescaling of temperature and salinity.  All answers are bitwise identical.
  Added rescaling conversion factors for temperature and salinity to the
EOS_type and added code to all of the EOS routines that work with dimensionally
rescaled arguments to handle these new rescaling factors.  Also added new
optional arguments to int_density_dz_wright and int_spec_vol_dp_wright to handle
rescaling temperature and salinity.  There are also many places in MOM_EOS.F90
where comments are altered to reflect the new rescaled units. However, for now
these new rescaling factors are hard-coded to 1, so there is no new rescaling
yet, and all answers are bitwise identical.  There are, however, new optional
arguments in two public interfaces.
  Use the new, clearer interfaces for calculate_TFreeze in MOM6.F90 and
MOM_diabatic_aux.F90, and use tv%C_p instead of fluxes%C_p in several places.
tv%C_p is not used outside of the code under the MOM6 src directory, whereas
fluxes%C_p is, so it is preferable to use tv%C_p to permit clean rescaling of
the temperature-related variables without touching anything outside of the src
directories.  All answers are bitwise identical.
  Modified comments in 5 more files to prepare for the addition of dimensional
rescaling of temperature and salinity.  All answers are bitwise identical.
  Added optional scaling arguments to the set_up_ALE_sponge routines, to allow
input fields to be rescaled before use.  This change is necessary to permit the
dimensional rescaling of temperature, salinity, and other tracers because of the
way that some versions repeatedly read new values from files as the runs
progress.  All answers are bitwise identical, but there are new optional
arguments to public interfaces.
  Added dimensional rescaling for temperature and salinity, as determined by the
new runtime parameters C_RESCALE_POWER and S_RESCALE_POWER.  With this change
there are 4 new elements in the transparent unit_scale_type, and these are
widely used in the code.  In addition, ### other files were added that had
checksum calls or diagnostics rescaled by these new factors, and where comments
were changed, but were otherwise unaltered as a result of the new dimensional
rescaling.  There will be another commit very shortly that completes the changes
and leads to fully functional dimensional rescaling for temperatures and
salinities, but these will involve more extensive code or interface changes, but
this commit will be useful for any possible git-bisection of any potential
changes that do not involve dimensional rescaling.  All solutions in existing
test cases are bitwise identical.
  This commit completes the dimensional rescaling for temperature and salinity,
and it has been confirmed that the solutions for the existing test cases pass
these tests.  There are new unit_scale_type arguments to several publicly
visible interfaces, mostly related to temperature initialization.  There is also
a new optional argument, conc_scale to the register_tracer calls to specify the
conversion that should be done to tracer concentrations during output.
Additionally, there are new entries in the incorrect units on some runtime
parameters for user code were corrected in the MOM_parameter_doc files for some
test cases.  All answers are bitwise identical in the MOM6 regression suite,
including when the temperature and salinity rescaling are enabled.
@codecov
Copy link

codecov bot commented May 4, 2022

Codecov Report

Merging #122 (4767046) into dev/gfdl (0b05686) will decrease coverage by 0.02%.
The diff coverage is 28.43%.

@@             Coverage Diff              @@
##           dev/gfdl     #122      +/-   ##
============================================
- Coverage     33.51%   33.48%   -0.03%     
============================================
  Files           262      262              
  Lines         71175    71362     +187     
  Branches      13284    13310      +26     
============================================
+ Hits          23856    23898      +42     
- Misses        42847    42992     +145     
  Partials       4472     4472              
Impacted Files Coverage Δ
src/ALE/MOM_hybgen_regrid.F90 0.00% <0.00%> (ø)
src/ALE/MOM_hybgen_unmix.F90 0.00% <0.00%> (ø)
src/ALE/MOM_regridding.F90 23.00% <0.00%> (ø)
src/ALE/coord_adapt.F90 0.00% <0.00%> (ø)
src/ALE/coord_hycom.F90 0.00% <ø> (ø)
src/ALE/coord_rho.F90 12.17% <0.00%> (ø)
src/ALE/coord_slight.F90 0.00% <0.00%> (ø)
src/core/MOM_checksum_packages.F90 24.80% <0.00%> (-0.20%) ⬇️
src/core/MOM_open_boundary.F90 23.01% <0.00%> (-0.06%) ⬇️
src/diagnostics/MOM_PointAccel.F90 3.01% <0.00%> (-0.02%) ⬇️
... and 70 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 0b05686...4767046. Read the comment docs.

@Hallberg-NOAA
Copy link
Member Author

Please note that this PR actually has fewer commits than it appears, because it is built on top of PR #118. Also, each of the commits here are potentially significant for future git bisections, so please do not squash this commit.

@Hallberg-NOAA Hallberg-NOAA added documentation Improvements or additions to documentation enhancement New feature or request labels May 7, 2022
Hallberg-NOAA and others added 6 commits May 7, 2022 21:50
  Corrected the units in comments describing some temperature and salinity
variables that had been accidentally omitted from the previous commits in this
sequence.  Also rescaled some local temperature and salinity variables used in
seamount_initialize_thickness and added missing unit conversion factors for
several diagnostics in MOM_oda_incupd.  All answers are bitwise identical.
  Works with rescaled temperatures and salinities for the internal calculations
in MOM_temp_salt_initialize_from_Z, taking advantage of the recently corrected
rescaling capabilities in horiz_interp_and_extrap_tracer.  This commit also
includes these other closely related changes.

 - Modified convert_temp_salt_for_TEOS10 to work with rescaled temperature and
   salinity units

 - Eliminated unused land_fill argument from determine_temperature

 - Slightly refactored tracer_z_init_array to avoid needing an extra set of do
   loop through the 3-d array when a scale argument is present

All answers are bitwise identical, but there are changes in the arguments to
publicly visible interfaces.
  This commit adds new functionality to the MOM_EOS module to support the
dimensional rescaling of temperatures and salinities.

 - Added the new routines cons_temp_to_pot_temp and abs_saln_to_prac_saln to
   convert between forms of temperature and salinity variables, respectively.
   These work on arrays of rescaled variables.

 - Added the new optional argument scale_from_EOS to calculate_TFreeze_scalar,
   to indicate that this routine should use the unit scaling stored in their
   EOS_type arguments.

 - Also corrected some comments throughout MOM_EOS.F90.

All answers are bitwise identical, but there are new public interfaces.
  Rescaled the units of some salinities in dumbbell_initialize_thickness and
added comments or corrected the unit descriptions in comments describing several
variables.  All answers are bitwise identical.
@marshallward
Copy link
Member

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/jobs/78209 ✔️ 🟡

New parameters:

  • C_RESCALE_POWER
  • S_RESCALE_POWER

@marshallward marshallward merged commit a965005 into NOAA-GFDL:dev/gfdl May 22, 2022
Hallberg-NOAA added a commit to Hallberg-NOAA/MOM6 that referenced this pull request May 29, 2022
  Corrected two bugs in the code handling the unscaling of tracers in east-west
open boundary segments.  Both changes bring the east-west code into (closer?)
agreement with the north-south code, and it might explain some recent reports of
strange behavior.  However, the fact that the existing MOM6-examples pipeline
tests do not detect this bug reveals a clear shortcoming in the suite of test
cases with OBCs that are currently being testing with MOM6 code changes.  These
bugs were introduced to dev/gfdl on May 22, 2022 (one week before this fix) as a
part of PR# 122 (NOAA-GFDL#122).
marshallward pushed a commit that referenced this pull request May 29, 2022
  Corrected two bugs in the code handling the unscaling of tracers in east-west
open boundary segments.  Both changes bring the east-west code into (closer?)
agreement with the north-south code, and it might explain some recent reports of
strange behavior.  However, the fact that the existing MOM6-examples pipeline
tests do not detect this bug reveals a clear shortcoming in the suite of test
cases with OBCs that are currently being testing with MOM6 code changes.  These
bugs were introduced to dev/gfdl on May 22, 2022 (one week before this fix) as a
part of PR# 122 (#122).
@Hallberg-NOAA Hallberg-NOAA deleted the EOS_rescale_prep branch July 16, 2022 09:42
marshallward pushed a commit that referenced this pull request Nov 23, 2022
  Added a missing scale factor in the DENSE_WATER_EAST_SPONGE_SALT get_param
call in dense_water_initialize_sponges, and added comments describing the local
variables (and their units) throughout the dense_water_initialization module.
The variable set by DENSE_WATER_SILL_HEIGHT was unused and it probably was
always intended to be DENSE_WATER_SILL_DEPTH, which it now is.  Units arguments
were also added to two of the unlogged get_param calls in this module.  Without
this change, this test case would not reproduce with dimensional rescaling due
to a scale factor that was omitted when salinity was being rescaled on May 3,
2022, which became a part of PR #122 to dev/gfdl, but answers should not change
when dimensional rescaling of salinities is not used.  All answers and output in
the MOM6-examples test suite are bitwise identical.
marshallward pushed a commit that referenced this pull request Feb 28, 2024
…1031

update MOM6 to its main repo 20231025 (NCAR candidate) and 20231031(GMAO FMS_cap) updating
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants