(*)Corrected the units of VarMix_CS%slope_x #314
Merged
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
VarMix_CS%slope_x was being set with units of [Z L-1 ~> nondim], but described in comments as though it was simply [nondim], and then used in the (apparently unused?) calculate_slopes=.false. branch in calc_slope_functions_using_just_e as though its units actually were [nondim]. This commit corrects this inconsistency, while also rescaling the internal slope variables in that routine to also have the proper units of [Z L-1 ~> nondim]. In so doing, several rescaling factors could be eliminated from the calculations. In addition, the slopes used in calc_QG_Leith_viscosity were also being rescaled with the wrong factor or had dimensionally incorrect tiny values in some denominators, and this has been corrected as well. In testing this rescaling fix, a number of other bugs were identified with USE_QG_LEITH_VISC=True (as described at github.com/mom-ocean/issues/1590), so a fatal error message was added if this option is enabled. All answers in the MOM6-examples test suite are bitwise identical, but the code will now give a fatal error if USE_QG_LEITH_VISC=.true.
Hallberg-NOAA
added
bug
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #314 +/- ##
============================================
- Coverage 37.17% 37.17% -0.01%
============================================
Files 265 265
Lines 74401 74402 +1
Branches 13821 13822 +1
============================================
- Hits 27661 27659 -2
- Misses 41655 41658 +3
Partials 5085 5085
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Incorporated a slope squared rescaling factor into the definition of the local N2min variable in wave_speed, thereby eliminating 4 points in the code where rescaling had been necessary and another local variable. All answers and output are bitwise identical.
|
Another small commit related to this pull request was added that eliminates some rescaling factors in |
marshallward
approved these changes
Jan 31, 2023
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/18103 ✔️ |
This was referenced Apr 6, 2023
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.
VarMix_CS%slope_x was being set with units of [Z L-1 ~> nondim], but described in comments as though it was simply [nondim], and then used in the (apparently unused?) calculate_slopes=.false. branch in calc_slope_functions_using_just_e as though its units actually were [nondim]. This commit corrects this inconsistency, while also rescaling the internal slope variables in that routine to also have the proper units of [Z L-1 ~> nondim]. In so doing, several rescaling factors could be eliminated from the calculations. In addition, the slopes used in calc_QG_Leith_viscosity were also being rescaled with the wrong factor or had dimensionally incorrect tiny values in some denominators, and this has been corrected as well. In testing this rescaling fix, a number of other bugs were identified with USE_QG_LEITH_VISC=True (as described at github.com/mom-ocean/issues/1590), so a fatal error message was added if this option is enabled. All answers in the MOM6-examples test suite are bitwise identical, but the code will now give a fatal error if USE_QG_LEITH_VISC=.true.