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

Update from mom-ocean #1647 #801

Merged
merged 7 commits into from
Jan 13, 2025
Merged

Update from mom-ocean #1647 #801

merged 7 commits into from
Jan 13, 2025

Conversation

marshallward
Copy link
Member

Merge commit of mom-ocean#1647 from the main branch.

Includes updates which restore reproducibility from MEKE, as well as potential performance improvements.

This is a bookkeeping PR and should not be merged through GitHub.

The MEKE GM computation of `src` and `src_GM`, a diagnostic array, were
placed in a single loop.  The similar RHS of each expression made it
unfavorable to use FMAs on the `src` update.  Older production runs
depending on this FMA were seeing answer changes.

This patch restores the FMA loop update of `src` by separating `src` and
`src_GM` into separate loops.
This patch makes several adjustments to MOM_MEKE.F90 and
MOM_hor_visc.F90 to ensure that the Laplacian and biharmonic friction
coefficients are computed separately, and only if their respective terms
are enabled.

This resolves some subtle bugs where the default biharmonic value of -1
was applied to the Laplacian case, even when the biharmonic MEKE
friction was disabled.
The if-test inside of the FrictWork loops are likely to impede
performance.  Even if the total work is reduced, they are likely to
interrupt pipelines.  When EY24_EBT_BS is disabled, they will clearly
reduce performance.

This patch moves those tests outside of the if-block and applies them
separately.

(Calculation would be slightly improved if the meaning of the flag were
reversed, but I don't want to make additional changes.)
The damping MEKE loop also included updates to multiple diagnostics,
even if they were not registered.  This would presumably have a negative
impact on performance.

This patch moves each diagnostic into a separate loop.  It also
conditionally precomputes the damping and damp_rate parameters, which
are now stored as 2d arrays rather than in-loop scalars.

As before, the MEKE calculation is left unchanged in order to preserve
bit reproducibility.
The redefining of int_tide_CS control struct in set_diffusivity_init
caused errors in debug-mode for Intel compilers.  The issue appears to
be an internal function that expects a pointer rather than the type.

This patch reverts this back to a pointer.  We can revisit this if there
is a need to reduce reliance on pointers.
This patch updates the expression for FrictWork_bh (biharmonic
frictional work) when the FrictWork_bug flag is enabled.  The new form
is symmetric to rotations when FMA instructions are enabled.
@marshallward
Copy link
Member Author

marshallward commented Jan 13, 2025

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 31.37821% with 2141 lines in your changes missing coverage. Please review.

Project coverage is 39.17%. Comparing base (05d8cc3) to head (776be84).
Report is 162 commits behind head on dev/gfdl.

Files with missing lines Patch % Lines
src/ice_shelf/MOM_ice_shelf.F90 0.00% 464 Missing ⚠️
src/core/MOM_PressureForce_FV.F90 17.17% 369 Missing and 12 partials ⚠️
src/diagnostics/MOM_harmonic_analysis.F90 4.91% 227 Missing and 5 partials ⚠️
src/core/MOM_density_integrals.F90 23.59% 127 Missing and 9 partials ⚠️
src/parameterizations/lateral/MOM_MEKE.F90 32.05% 65 Missing and 41 partials ⚠️
src/diagnostics/MOM_spatial_means.F90 50.00% 36 Missing and 25 partials ⚠️
src/core/MOM_barotropic.F90 46.29% 50 Missing and 8 partials ⚠️
src/core/MOM_open_boundary.F90 36.98% 38 Missing and 8 partials ⚠️
src/ALE/MOM_regridding.F90 13.72% 41 Missing and 3 partials ⚠️
src/core/MOM_dynamics_split_RK2b.F90 0.00% 44 Missing ⚠️
... and 43 more
Additional details and impacted files
@@             Coverage Diff              @@
##           dev/gfdl     #801      +/-   ##
============================================
+ Coverage     37.05%   39.17%   +2.12%     
============================================
  Files           271      298      +27     
  Lines         81290    96153   +14863     
  Branches      15166    19122    +3956     
============================================
+ Hits          30124    37671    +7547     
- Misses        45526    52028    +6502     
- Partials       5640     6454     +814     
Flag Coverage Δ
?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marshallward
Copy link
Member Author

Regressions are due to the apparent-removal of MEKE diagnostics, which are now conditionally registered. These can be ignored.

@marshallward marshallward merged commit 7a45c61 into dev/gfdl Jan 13, 2025
18 of 20 checks passed
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.

1 participant