forked from mom-ocean/MOM6
-
Notifications
You must be signed in to change notification settings - Fork 65
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
+(*)Make unit_scale_type arguments non-optional #29
Merged
marshallward
merged 4 commits into
NOAA-GFDL:dev/gfdl
from
Hallberg-NOAA:mandatory_US_args
Dec 12, 2021
Merged
+(*)Make unit_scale_type arguments non-optional #29
marshallward
merged 4 commits into
NOAA-GFDL:dev/gfdl
from
Hallberg-NOAA:mandatory_US_args
Dec 12, 2021
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
Added the public interface unit_no_scaling_init() to the MOM_unit_scaling module to initialize a non-scaling unit_scale_type without requiring a param_file_type argument. This should be useful for certain trivial unit tests, and it makes it more plausible to make the unit_scale_type arguments mandatory. As a part of this change, the new internal subroutine set_unit_scaling_combos() was carved out of unit_scaling_init to avoid code duplication. Also added comments describing the effective units of the various scaling factors with the standard MOM6 notation. All answers and output are bitwise identical.
Provide optional US arguments to 4 existing calls to set_grid_metrics() and MOM_initialize_topography(), enabling these arguments to become mandatory in the next commit. In some cases this requires a call to unit_no_scaling_init() or the removal of a call to rescale_dyn_horgrid_bathymetry(). In addition, a mis-scaled value was corrected in the initialization of the ODA control structures private thickness array; this latter issue is not a problem for Boussinesq models without dimensional rescaling (i.e. the tested cases), but could change answers in other cases with data assimilation. All answers in the MOM6-examples or TC test cases are bitwise identical.
Made the unit_scale_type arguments non-optional for 28 routines. These arguments had been optional in the first place to manage the coordination between the MOM6 and SIS2 repositories, but SIS2 has been using these optional arguments for several years now, and they can be made mandatory without imposing any disruptions. This change simplifies and clarifies the code. All answers and output are bitwise identical.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #29 +/- ##
============================================
- Coverage 29.05% 29.00% -0.05%
============================================
Files 240 240
Lines 71293 71235 -58
============================================
- Hits 20714 20665 -49
+ Misses 50579 50570 -9
Continue to review full report at Codecov.
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/14331 ✔️ |
marshallward
approved these changes
Dec 12, 2021
This was referenced Jan 29, 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 is a series of commits that make the previously optional
unit_scale_type arguments to 28 routines mandatory, mostly related to
the initialization of the bathymetry or the grid metrics. This change realizes
something that was envisioned when these arguments were originally introduced,
but coordination with SIS2 means that they had to be optional when added to
routines that are also used by SIS2. Now that SIS2 is using all of these
arguments, they can be made non-optional. The resulting code should be much
easier to understand. Because of a specific ODA initialization bug that was
corrected in these commits, answers might change slightly in nearly massless
layers for non-Boussinesq models with ODA enabled using the src/ocean_data_assim
code, but no such case exists in any of our test cases. All answers and output
in the MOM6-examples test suite are bitwise identical.
The commits in this PR include: