+(*)Get the offline tracer mode working again #37
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
Corrected the comments describing the optional arguments to advect_tracer and fixed a few spelling errors in comments. All answers are bitwise identical.
Corrected bugs in the offline tracer code that were preventing it from reproducing across processor counts (and perhaps working sensibly at all). All answers and output in the MOM6-examples regression suite are bitwise identical. Although answers with the offline tracer code will change because of the bug fix, because of some bugs that were fixed in another recent commit, it was previously impossible to have run the offline tracer cases at all.
Substantially refactored the offline tracer code. This refactoring includes adding grid and unit_scale_type arguments to several routines related to the offline tracer code. An offline tracer advection test case is now consistent across processor layouts and pass the dimensional rescaling tests (including the chksums in debug mode), and there are comments describing all the real variables and their dimensions in the offline tracer routines. All answers and output are bitwise identical.
Minorly revised the algorithms used for offline tracer advection for rotational consistency, and for exact reproducibility across PE layouts by using reproducing sums to detect convergence. This also includes some changes to use roundoff to detect convergence instead of fixed values. Also replaced some divisions with multiplication by a reciprocal. In addition, some of the optional arguments to advect_tracer that are only used for offline tracer advection were renamed or revised for clarity and reordered for the convenience of the non-offline-tracer code. Although answers with the offline tracer code will change slightly because of this refactoring, because of some bugs that were fixed in another recent commit, it was previously impossible to have run the offline tracer cases at all. All answers and output in the MOM6-examples regression suite are bitwise identical.
|
Each of the commits in this PR has been carefully tested to work, and may be a significant way-point should there need to be any future debugging of these capabilities, so please do not do a squash-merge on this PR. |
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #37 +/- ##
============================================
- Coverage 28.91% 28.91% -0.01%
============================================
Files 242 242
Lines 71252 71260 +8
============================================
+ Hits 20605 20606 +1
- Misses 50647 50654 +7
Continue to review full report at Codecov.
|
Corrected the reported unit conversions in two comments describing variables in MOM_offline_aux.F90. All answers and output are bitwise identical.
ashao
suggested changes
Dec 16, 2021
There was a problem hiding this comment.
Thanks @Hallberg-NOAA for cleaning up this portion of the code. I especially like the changes made to the advect_tracer API. Once this PR is in, I'll go ahead and set aside some time to excise this code from the MOM.F90 and make a separate offline driver (now that I'm older and wiser).
Commented out the problematic lines that Andrew Shao flagged in his review of MOM6 dev/gfdl PR mom-ocean#37. The model runs perfectly well in short offline-tracer test runs, and even gives bitwise identical output, perhaps because no layers were being abruptly flooded to 10^13 times their previous values. These omitted lines could change answers in some cases, so the lines in question have been retained in case the offline tracer code needs to be debugged layer and these mysterious (and seemingly unhelpful) lines turn out to have been necessary. All answers in the non-offline-tracer runs are bitwise identical.
|
I've looked over the changes again and this PR looks good to me. I haven't had the chance to test the Baltic_offline test case yet. |
Deleted four lines in the offline tracer code that had recently been commented out, along with the comments describing them. Further conversations had led to a consensus that these lines served no useful purpose, and are not worth keeping in the code, even in comments. Several excess spaces were also eliminated in MOM_offline_aux.F90. All answers and output are bitwise identical.
|
This PR has been favorably reviewed by Andrew Shao, and all code conflicts have been resolved. I believe that this PR is now ready for acceptance, once TC and pipeline testing are complete. |
marshallward
approved these changes
Dec 29, 2021
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/14453 ✔️ This PR cannot be rebased, so will include merge commits. |
This was referenced Jan 29, 2022
Hallberg-NOAA
added
answer-changing
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 includes a series of targeted changes to get the offline tracer mode
to work once again, and equally importantly to get it to reproduce across
processor layouts and with dimensional rescaling. (MOM6 would have encountered
a segmentation fault in this mode since about July 3, 2019, and may not have
worked before this.) It also includes a substantial refactoring of the offline
tracer code, including the addition of new arguments to a number of routines,
revisions to improve some of the algorithms, comments documenting each real
variable and robustly tested rescaling of the offline tracer variables for
dimensional consistency testing.
There is also a corresponding PR to MOM6-examples (NOAA-GFDL/MOM6-examples#345)
that updates some of the parameter input and diag_table files that were there to test the
offline-tracer capabilities in MOM6-examples/ice_ocean_SIS2/Baltic_ALE_z_offline_tracers.
All answers and output are bitwise identical in any cases that worked before,
and the offline tracer mode is once more working and reproducing answers when
dimensional consistency test are active. The commits in this PR include: