+(*)Refactor and fix bugs in diabatic_driver #1250
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
Rearranged diagnostics in MOM_diabatic_driver to precede the calls to the tracer column function routines, and explicitly set zeros arrays for diagnostics that are not actually available and should be elminated. Also eliminated some unused arrays. All answers are bitwise identical.
Reused the variables ea_s and eb_s for passive tracer mixing in ALE mode, which simplifies the logic around the calls to call_tracer_column_fns(). This also corrects a bug using an uninitialized array when double diffusion is not used and MIX_BOUNDARY_TRACERS is explicitly set to false, which could change answers for some passive tracers, or it could cause runs to fail altogether, but there are no known cases that exercises this combination of settings. The new runtime parameter MIX_BOUNDARY_TRACER_ALE has been added allow the boundary mixing for passive to be both set and used, correcting another bug. All answers are bitwise for all MOM6-examples test cases, but there are new entries in some MOM_parameter_doc files.
Replaced the various layer entrainment rates with interface coupling coefficients in the diabatic driver when used in ALE mode, both for efficiency and for greater code clarity. Also explicitly set several diagnostics that area always zero to use zeros_h arrays, to indicate that they are always zero and are candidates for removal. All answers are bitwise identical.
Removed unused variables in MOM_diabatic_driver.F90, and added a comment flagging a probable bug with USE_LEGACY_DIABATIC_DRIVER=False and DOUBLE_DIFFUSION=True. All answers are bitwise identical.
Modified diabatic_ALE to avoid triple counting double diffusion. This would change answers with DOUBLE_DIFFUSION=True, USE_LEGACY_DIABATIC_DRIVER=False, USE_CVMIX_DDIFF=False, and BOUNDARY_MIX_TRACERS=False, but I am unaware of any MOM6 test cases that use. Also renamed and corrected the comments describing a variable. All answers in the MOM6-examples test suite are bitwise identical, but there would be answer changes for some configurations that probably do not exist.
Reuse Kd_heat for Kd_int in diabatic_ALE to avoid unnecessary copies of 3-d arrays and reduce the model's memory footprint. All answers are bitwise identical.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #1250 +/- ##
==========================================
Coverage 46.02% 46.03%
==========================================
Files 224 224
Lines 71064 70921 -143
==========================================
- Hits 32707 32648 -59
+ Misses 38357 38273 -84
Continue to review full report at Codecov.
|
Corrected the symmetric memory array size declarations for zeros_v, which is used to write out diagnostics that are always 0 and should probably be eliminated. All answers are bitwise identical.
adcroft
reviewed
Nov 18, 2020
| eatr(:,:,:) = 0.0 ; ebtr(:,:,:) = 0.0 | ||
| call find_uv_at_h(u, v, h, u_h, v_h, G, GV, US, eatr, ebtr) | ||
| ! Note that ent_s(:,:,:) = 0.0 | ||
| call find_uv_at_h(u, v, h, u_h, v_h, G, GV, US, ent_s(:,:,1:nz), ent_s(:,:,1:nz)) |
There was a problem hiding this comment.
I'm fairly sure this forces copies of arrays?
|
The potential copy raised by @adcroft will be addressed by @Hallberg-NOAA in a later PR. For now we will check it in. The reported regression is due to a re-ordering of diagnostics, which we currently cannot distinguish. We will address this in a later PR. Those issues aside, this is ready to be merged, pending Gaea regression testing. |
|
https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/11567 ✔️ 🟡 Correct up to a parameter change (possibly from the last merge) |
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.
Refactor MOM_diabatic_driver.F90 and corrected several bugs that could change
answers for some apparently unused combinations of parameters. This includes
changes that substantially simplify the code, eliminate unused variables and
the calculation of diagnostics that are always zero, and remove unnecessary
halo updates. There is one new runtime parameter, so some MOM_parameter_doc
files have an added entry. The commits in this PR include: