(*)Fix bug in h_neglect used to remap diagnostics to density space #572
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 explicit names to the h_neglect and h_neglect_edge arguments in a call to build_rho_column in diag_remap_update. These were problematic because these arguments were previously in the wrong positions and hence were being used for the wrong variables inside of build_rho_column. In Boussinesq mode, the results will be the same as long as there is no dimensional rescaling being applied to thicknesses, and it this were being tested it would have resulted in a dimensional consistency test failure. In non-Boussinesq mode, the values of H_subroundoff as used for this coordinate will be off by a factor of RHO_KV_CONVERT (usually 1035), but these thicknesses are so small that they do not matter unless ANGSTROM is explicitly set to zero (when no dimensional rescaling is being used). To summarize, this fixes a bug that has been around since 2017, and while this fix could slightly change some diagnostics that are remapped to density, it only does so in circumstances that are unlikely to have been realized. No answer changes were detected in the MOM6-examples regression suite, and almost certainly none ever would be in any existing use of MOM6.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/gfdl #572 +/- ##
=========================================
Coverage 37.20% 37.20%
=========================================
Files 271 271
Lines 80475 80475
Branches 15008 15008
=========================================
Hits 29942 29942
Misses 44961 44961
Partials 5572 5572 ☔ View full report in Codecov by Sentry. |
marshallward
approved these changes
Feb 28, 2024
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/22481 ✔️ |
This was referenced May 16, 2024
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.
Added explicit names to the
h_neglectandh_neglect_edgearguments in a call tobuild_rho_column()indiag_remap_update(). These were problematic because these arguments were previously in the wrong positions and hence were being used for the wrong (unused) variables inside ofbuild_rho_column(). In Boussinesq mode, the results will be the same as long as there is no dimensional rescaling being applied to thicknesses, and it this were being tested it would have resulted in a dimensional consistency test failure. In non-Boussinesq mode, the values ofH_subroundoffas used for this coordinate will be off by a factor of RHO_KV_CONVERT (usually 1035), but these thicknesses are so small that they do not matter unless ANGSTROM is explicitly set to zero (when no dimensional rescaling is being used). To summarize, this fixes a bug that has been around since 2017, and while this fix could slightly change some diagnostics that are remapped to density, it only does so in circumstances that are unlikely to have been realized. No answer changes were detected in the MOM6-examples regression suite, and almost certainly none ever would be in any existing use of MOM6.