Skip to content
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

Make sure EBT structure is computed when desired #263

Merged
merged 2 commits into from
Dec 14, 2022

Conversation

NoraLoose
Copy link

@NoraLoose NoraLoose commented Dec 3, 2022

Issue

I ran two (supposedly) identical MOM6 experiments, both with the following parameters in MOM_input:

THICKNESSDIFFUSE = True
KHTH = 800.0
USE_VARIABLE_MIXING = True
KHTH_USE_EBT_STRUCT = True

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_dx diagnostic in the diag_table. In the second case, I did not ask for the Rd_dx diagnostic. That's what made the difference.

More generally and for future reference, here is what happened in the code before this PR:

  • If KHTH_USE_EBT_STRUCT = True, an array for CS%ebt_struct got allocated and initialized to 0.
  • 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.

(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.

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
Copy link

codecov bot commented Dec 3, 2022

Codecov Report

Merging #263 (039aa46) into dev/gfdl (490bfca) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@             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     
Impacted Files Coverage Δ
...eterizations/lateral/MOM_lateral_mixing_coeffs.F90 42.74% <100.00%> (ø)
src/framework/MOM_document.F90 74.10% <0.00%> (-0.45%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@marshallward
Copy link
Member

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17690 ✔️

@marshallward marshallward merged commit c80db67 into NOAA-GFDL:dev/gfdl Dec 14, 2022
@NoraLoose NoraLoose deleted the fix-ebtstr-comp-cases branch December 14, 2022 17:23
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 <marshall.ward@noaa.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants