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

+*Add and revise step_MOM_dyn_split_RK2b #534

Merged
merged 5 commits into from
Dec 22, 2023

Conversation

Hallberg-NOAA
Copy link
Member

@Hallberg-NOAA Hallberg-NOAA commented Dec 14, 2023

This pull request consists of two commits that add the new module MOM_dynamics_split_RK2b and then revise it to give a new split time-stepping scheme. This new scheme is enabled by setting the new runtime parameter SPLIT_RK2B = True.

With this new scheme, the velocity that is passed back and forth to the driver is averaged over the phases of the barotropic stepping, rather than being an instantaneous velocity. The instantaneous velocities can be regenerated from the phase-averaged velocities using a stored barotopic velocity increment. It is hoped that this new scheme will be slightly more robust than before, although this will be more expensive than the other (previous) split time stepping scheme, with two calls to horizontal_viscosity per baroclinic dynamics time step (instead of one) and four calls to continuity per step (instead of 3). Some of this added cost could be recovered by unwinding the sub-calls within continuity to reuse the PPM thickness reconstructions in one of the two directions.

This new scheme has been tested on all of the examples in the MOM6-examples test suite, where it gives very similar answers. It has also been confirmed that the new scheme reproduces across processor layout and restart files.

When this new scheme is used there are substantial differences in the contents of the restart files, with 7 fewer 3-d fields, but two additional 2-D velocity increments. Moreover, when this scheme is used in ALE mode, there is only a single pair of velocity components that need to be remapped.

Answers will change with SPLIT_RK2B = True. There is a new publicly visible module with several new public interfaces. In addition, there are changes to the contents of the MOM_parameter_doc.all files even when the new scheme is not being exercised.

The commits in this PR include:

  • 9feaa3bb7 *+Revise algorithm in step_MOM_dyn_split_RK2b
  • c075fb84a +Add MOM_dynamics_split_RK2b enabled by SPLIT_RK2B

@Hallberg-NOAA Hallberg-NOAA added enhancement New feature or request Parameter change Input parameter changes (addition, removal, or description) labels Dec 14, 2023
@Hallberg-NOAA Hallberg-NOAA requested review from adcroft and removed request for adcroft December 14, 2023 23:08
Copy link

codecov bot commented Dec 14, 2023

Codecov Report

Attention: 613 lines in your changes are missing coverage. Please review.

Comparison is base (eac3bb3) 37.46% compared to head (e46c5c8) 37.17%.

Files Patch % Lines
src/core/MOM_dynamics_split_RK2b.F90 0.00% 602 Missing ⚠️
src/core/MOM.F90 26.66% 7 Missing and 4 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           dev/gfdl     #534      +/-   ##
============================================
- Coverage     37.46%   37.17%   -0.29%     
============================================
  Files           270      271       +1     
  Lines         79695    80308     +613     
  Branches      14830    14979     +149     
============================================
+ Hits          29854    29855       +1     
- Misses        44291    44899     +608     
- Partials       5550     5554       +4     

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

  Add the new module MOM_dynamics_split_RK2b and calls to the routines in this
module to MOM.F90.  These calls are exercised when the run time parameter
SPLIT_RK2B is true.  For now, all answers are bitwise identical when this new
code is being exercised, but there is a new module with multiple public
interfaces and a new entry in many MOM_parameter_doc files.
  Revised step_MOM_dyn_split_RK2b to use the time-filtered velocities as
arguments and reconstruct the instantaneous velocities with the proper phase
from the barotropic solver from the saved increments du_av_inst and dv_av_inst.
As a part of this change, the continuity solver is used to find the thickness
fluxes used in the predictor step Coriolis terms, and the horizontal viscous
accelerations are calculated for both the predictor and corrector steps, thereby
avoiding the need to vertically remap any 3-d fields apart from a single pair of
velocity components.

  The run-time option STORE_CORIOLIS_ACCEL is no longer used when SPLIT_RK2B =
True.  FPMIX was also disabled in this mode due to unresolved dimensional
inconsistency errors in that code.

  Additionally, the time-filtered velocities are now used instead of the
instantaneous velocities in the second call to radiation_open_bdry_conds(),
which should improve the performance of several of the Orlanski-type open
boundary conditions.

  The 2-d fields du_av_inst and dv_av_inst were added to the restart file, while
the 3-d fields u2, v2, diffu and diffv and either CAu and CAv or uh, vh and h2
are all removed from the restart files.  Remap_dyn_split_RK2b_aux_vars() now has
nothing to do, and it should probably be eliminated altogether in a subsequent
commit.

  All answers are changed when SPLIT_RK2B = True, and there are changes to the
contents of the restart files.
  This commit includes further revisions to MOM_dynamics_split_RK2b that avoid
some unnecessary calculations and group some of the halo updates into fewer
group passes.  All answers are bitwise identical.
  Corrected the description of the runtime parameter SPLIT_RK2B that will appear
in the MOM_parameter_doc files and in the doxygen descriptions of the MOM module
to better reflect what was ultimately being done with this new scheme.  All
answers are bitwise identical, but there are changes to the (newly added)
contents of some MOM_parameter_doc files.
@Hallberg-NOAA
Copy link
Member Author

This PR has been revised to correct the comments describing the new runtime parameter SPLIT_RK2B, and there are no other known corrections that are needed.

@marshallward
Copy link
Member

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

Copy link
Member

@marshallward marshallward left a comment

Choose a reason for hiding this comment

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

We can look into merging these two RK2-based methods once the new scheme has been road-tested.

@marshallward marshallward merged commit 5137442 into NOAA-GFDL:dev/gfdl Dec 22, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Parameter change Input parameter changes (addition, removal, or description)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants