(*)Avoid array syntax math in MOM_wave_structure#167
Merged
marshallward merged 2 commits intoAug 2, 2022
Conversation
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #167 +/- ##
============================================
+ Coverage 37.41% 37.64% +0.22%
============================================
Files 259 259
Lines 71823 70994 -829
Branches 13440 13322 -118
============================================
- Hits 26871 26724 -147
+ Misses 40009 39354 -655
+ Partials 4943 4916 -27
Help us with your feedback. Take ten seconds to tell us how you rate us. |
raphaeldussin
left a comment
raphaeldussin
left a comment
There was a problem hiding this comment.
I think the code changes are accurate but found a minor sign error in a comment
Revised the MOM_wave_structure code to avoid using array syntax in calculations and subroutine arguments. While making these changes, several incorrect or missing variable unit descriptions in comments were identified and corrected. This set of changes is mathematically equivalent, but will result in changes at the level of roundoff, either because the order of sums was changed or because divisions are replaced by multiplication by a common reciprocal. Some debugging safety checks were moved inside of a logical test of the debug flag for efficiency. One dimensionally inconsistent expression (in vertical and horizontal distances) was corrected, so that it should now pass the dimensional consistency checks. This commit will change answers in any cases that depend on this code, but because we recently corrected (in MOM6 PR#154) a major bug in this code with a much larger impact without causing any disruptions, we can be confident that this code is not yet used in an production runs where minor answer changes could be problematic. This code change does not alter any answers or output in the MOM6-examples test suite.
Corrected a sign error in commented out code in wave_structure, as noted in a review by Raf Dussin of the previous PR. All answers are bitwise identical.
Hallberg-NOAA
force-pushed
the
wave_structure_code_cleanup
branch
from
f36c7c9 to
e0d8b01
Compare
raphaeldussin
approved these changes
Jul 31, 2022
Member
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/16255 ✔️ |
marshallward
approved these changes
Aug 2, 2022
Member
marshallward
left a comment
marshallward
left a comment
There was a problem hiding this comment.
Approved on behalf of @raphaeldussin
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Revised the MOM_wave_structure code to avoid using array syntax in
calculations and subroutine arguments. While making these changes, several
incorrect or missing variable unit descriptions in comments were identified and
corrected. This set of changes is mathematically equivalent, but will result in
changes at the level of roundoff, either because the order of sums was changed
or because divisions are replaced by multiplication by a common reciprocal.
Some debugging safety checks were moved inside of a logical test of the debug
flag for efficiency. One dimensionally inconsistent expression (in vertical and
horizontal distances) was corrected, so that it should now pass the dimensional
consistency checks.
This commit will change answers in any cases that depend on this code, but
because we recently corrected (in MOM6 PR#154) a major bug in this code with a
much larger impact without causing any disruptions, we can be confident that
this code is not yet used in an production runs where minor answer changes could
be problematic. This code change does not alter any answers or output in the
MOM6-examples test suite.