Make sure EBT structure is computed when desired #263
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
This addition makes sure that CS%ebt_struct is filled/computed if the user specifies KHTH_USE_EBT_STRUCT = True. Before this addition, an array for CS%ebt_struct got allocated and initialized to 0, if KHTH_USE_EBT_STRUCT = True. But this array never got filled (with values other than 0) unless the user selected any of the following options too: 1) KHTH_USE_FGNV_STREAMFUNCTION = True 2) USE_MEKE = True 3) KhTr_passivity_coeff > 0 4) MLE_front_length > 0 5) Output the diagnostic Rd_dx 6) Use resolution scaling for the a) Laplacian viscosity, b) interface height diffusivity, c) tracer diffusivity, OR d) the MEKE viscosity contribution.
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #263 +/- ##
============================================
- Coverage 37.13% 37.12% -0.01%
============================================
Files 263 263
Lines 73444 73444
Branches 13675 13675
============================================
- Hits 27271 27269 -2
- Misses 41148 41149 +1
- Partials 5025 5026 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Hallberg-NOAA
approved these changes
Dec 13, 2022
There was a problem hiding this comment.
If there were an award for the most thorough explanation for a PR per line of code change, this PR would be the winner hands down! The explanation is incredibly clear and convincing, and the code change is as parsimonious as we could possibly hope for! I am very happy to approve this PR.
|
Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17690 ✔️ |
This was referenced Apr 6, 2023
marshallward
added a commit
that referenced
this pull request
Apr 9, 2024
* Enhancements for adding land block elimination to NUOPC cap: - Add sum_across_PEs_int4_2d to the sum_across_PEs interface - Allow mask_table file to be placed in run directory (now, the first dir that is looked at). * Enhance NUOPC cap to support MOM_mask_table. - Determine masked blocks. - Evenly distribute eliminated cells. - Fill ESMF gindex array accordingly. - During Export phase, set fields of eliminated cells to zero. * set %label in register_netcdf_field and register_netcdf_axis * first working version of an automated mask table generator * While determining masked blocks, take reentrancy and tripolar stitch into account * apply tripolar stitch fix in auto mask_table generation * add AUTO_IO_LAYOUT_FAC parameter to control IO_LOAYUT when AUTO_MASKTABLE is on * Miscellaneous auto masking fixes to address reviews: - Dimensionalize topographic depth variables used to determine cell masks in auto masktable routine. - Raise error if the user provided PE layout is inconsistent with auto masktable generation. - Save the masktable parameter description to a string variable to avoid repetition. - Fix typos, whitespaces, use modern array syntax. * Disable FPEs in MacOS testing Due to poor handling of floating point in HDF5 1.14.3, it is currently not possible to use floating point exceptions (FPEs) whenever this version is present. The GitHub Actions CI nodes would randomly select either 1.14.2 or 1.14.3, and would raise an FPE error if 1.14.3 was selected. Additionally, the homebrew installation does not provide a clean method for selecting a different version of HDF5. Thus, for now we disable FPEs in the MacOS testing, and hope to catch any legitimate FP errors in the Ubuntu version. We will restore these tests as soon as this has been fixed in an easily-accessible version of HDF5. As part of this PR, I have also moved the FCFLAGS configuration to the platform specific Actions files, allowing for independent compiler configuration for each platform. --------- Co-authored-by: Marshall Ward <[email protected]>
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.
Issue
I ran two (supposedly) identical MOM6 experiments, both with the following parameters in
MOM_input:In the first experiment (left), MOM6 imposed the desired EBT structure for the interface height diffusivity, diagnosed via
KHTH_t. In the second experiment (right), MOM6 set the interface height diffusivity to zero, except for in the uppermost layer.What happened?
In the first case, I was lucky, and somewhat unrelatedly asked for the
Rd_dxdiagnostic in thediag_table. In the second case, I did not ask for theRd_dxdiagnostic. That's what made the difference.More generally and for future reference, here is what happened in the code before this PR:
KHTH_USE_EBT_STRUCT= True, an array forCS%ebt_structgot allocated and initialized to 0.KHTH_USE_FGNV_STREAMFUNCTION= TrueUSE_MEKE= TrueKhTr_passivity_coeff> 0MLE_front_length> 0Rd_dx(I guess, OM4 uses a few of these options, which is why this problem has not been detected before?)
The one-line fix by this PR makes sure that the allocated array always gets filled if
KHTH_USE_EBT_STRUCT= True.