-
Notifications
You must be signed in to change notification settings - Fork 62
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
+Remapping consolidation, separation and cleanup #224
+Remapping consolidation, separation and cleanup #224
Conversation
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #224 +/- ##
============================================
- Coverage 37.18% 37.13% -0.06%
============================================
Files 263 263
Lines 73035 73060 +25
Branches 13608 13605 -3
============================================
- Hits 27161 27130 -31
- Misses 40859 40919 +60
+ Partials 5015 5011 -4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
3b88da4
to
9b2a721
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good cleanup. Tested at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17220
Created the new module remapping_attic to hold older versions of remapping code that are no longer used by MOM6. The subroutines is PosSumErrSignificant, remapByProjection, remapByDeltaZ and integrateReconOnInterval were moved to remapping_attic, where they can be tested by calling remapping_attic_unit_tests. The hard-coded old_algorithm logical module variable and the code it wraps were also eliminated. Also added a schematic description of the units of the real variables in the various routines in MOM_remapping and corrected some spelling errors. All answers are bitwise identical.
Moved interpolate_column and reintegrate_column (without changing anything) from MOM_diag_vkernels.F90 to MOM_remapping.F90 and incorporated the tests that had been in diag_vkernels_unit_tests into remapping_unit_tests. The entire MOM_diag_vkernels.F90 file was then removed. All answers are bitwise identical, although the module for two public routines was changed and a third was eliminated.
Remove missing_value arguments to interpolate_column and reintegrate_column, instead using 0 for the values in vanished cells. This change helps to address github.com/mom-ocean/issues/769. Also added comments schematically describing some of the argument units. Because 0 was already being used for the missing value (except in unit tests), all solutions are bitwise identical.
Added the new subroutine check_remapped_values with the duplicative error checking code in remapping_core_h and remapping_core_w, both to reduce code volume and promote code coverage, and to make the substance of these two routines easier to follow. All answers are bitwise identical.
9b2a721
to
f9dbf76
Compare
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17220 ✔️ |
…-main-2022-08-10 Merge GFDL to main (2022-08-10)
This PR consists of a series of commits that separate out older, unused
remapping code into a new, separate module (remapping_attic.F90), and collects
the remapping code that actually is used by MOM6 into MOM_remapping.F90. It
also includes changes to improve the documentation of the remapping code or to
stream-line it. The missing_value argument was eliminated in several places, in
response to github.com/mom-ocean/issues/769, and to reflect how the code is
actually being used. This PR is a prelude to other changes in a subsequent PR
that will make the remapping code practical for application more widely in the
MOM6 code, but which will unfortunately change some diagnostics by avoiding some
inflexible or inefficient constructs in the code as it is. Despite the addition
of several routines, MOM_remapping is 10% shorter than it was before. Although
several modules have changed, been added or removed, all answers are bitwise
identical, and all of the unit tests that were used before are still being used.
The commits in this PR include: