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

+(*)Fix USE_QG_LEITH_VISC = True reproducibility #604

Merged
merged 2 commits into from
May 13, 2024

Conversation

Hallberg-NOAA
Copy link
Member

This commit includes a series of changes that get MOM6 to reproduce across processor counts, layouts and restarts when USE_QG_LEITH_VISC = True and also to implement related changes to the QG Leith code to be consistent with not making the Boussinesq approximation when in non-Boussinesq mode. These changes include:

  • Adding a thermo_var_ptrs and a timestep argument to to the interfaces to horizontal_viscosity(), initialize_dyn_split_RK2() and initialize_dyn_split_RK2b()

  • Correcting a bug in the powers of H_subroundoff in the denominators of some expressions for the effective thicknesses associated with interfaces, thereby correcting some dimensionally inconsistencies in these expressions

  • Adding the new publicly visible routine calc_QG_slopes()

  • Adding the arguments slope_x and slope_y to calc_QG_Leith_viscosity() and avoiding the use of the VarMix%slope_[xy] elements in that routine

  • Using thickness_to_dz() to consistently convert layer thicknesses to the vertical distances across layers and provide this as an argument to calc_QG_Leith_viscosity()

  • Determining vertical distances in calculating the vertical derivatives of the slopes consistently with avoiding the Boussinesq approximation when in non-Boussinesq mode

  • Increasing some loop extents in calc_QG_Leith_viscosity()

Note that several of the expansions of the halo sizes in updates or in some of the calculations are not necessary if an extra halo update is included for slope_x and slope_y and dz after the calls to calc_QG_slopes() and thickness_to_dz() in horizontal_viscosity(). This extra halo update has also been included in a comment for later reference.

This commit includes several changes to publicly visible interfaces, and it will change answers with USE_QG_LEITH_VISC = True (which had previously triggered a fatal error), but answers and output are bitwise identical in other cases.

@Hallberg-NOAA Hallberg-NOAA added bug Something isn't working enhancement New feature or request labels Apr 18, 2024
Copy link

codecov bot commented Apr 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.07%. Comparing base (1829b7f) to head (c1ab0db).

❗ Current head c1ab0db differs from pull request most recent head 2562a90. Consider uploading reports for the commit 2562a90 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##           dev/gfdl     #604      +/-   ##
============================================
+ Coverage     37.01%   37.07%   +0.05%     
============================================
  Files           271      271              
  Lines         81134    80992     -142     
  Branches      15139    15123      -16     
============================================
- Hits          30034    30027       -7     
+ Misses        45475    45343     -132     
+ Partials       5625     5622       -3     

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

  This commit includes a series of changes that get MOM6 to reproduce across
processor counts, layouts and restarts when USE_QG_LEITH_VISC = True and also to
implement related changes to the QG Leith code to be consistent with not making
the Boussinesq approximation when in non-Boussinesq mode.  These changes
include:

  - Adding a thermo_var_ptrs and a timestep argument to to the interfaces to
    horizontal_viscosity, initialize_dyn_split_RK2 and initialize_dyn_split_RK2b

  - Correcting a bug in the powers of H_subroundoff in the denominators of some
    expressions for the effective thicknesses associated with interfaces,
    thereby correcting some dimensionally inconsistencies in these expressions

  - Adding the new publicly visible routine calc_QG_slopes

  - Adding the arguments slope_x and slope_y to calc_QG_Leith_viscosity and
    avoiding the use of the VarMix%slope_[xy] elements in that routine

  - Using thickness_to_dz to consistently convert layer thicknesses to the
    vertical distances across layers and provide this as an argument to
    calc_QG_Leith_viscosity

  - Determining vertical distances in calculating the vertical derivatives of
    the slopes consistently with avoiding the Boussinesq approximation when in
    non-Boussinesq mode

  - Increasing some loop extents in calc_QG_Leith_viscosity

  Note that several of the expansions of the halo sizes in updates or in some of
the calculations are not necessary if an extra halo update is included for
slope_x and slope_y and dz after the calls to calc_QG_slopes and thickness_to_dz
in horizontal_viscosity.  This extra halo update has also been included in a
comment for later reference.

  This commit includes several changes to publicly visible interfaces, and it
will change answers with USE_QG_LEITH_VISC = True (which had previously
triggered a fatal error), but answers and output are bitwise identical in other
cases.
@gustavo-marques
Copy link
Member

@Hallberg-NOAA, thanks for these fixes!

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.

@marshallward
Copy link
Member

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

@marshallward marshallward merged commit 93d9bb8 into NOAA-GFDL:dev/gfdl May 13, 2024
10 checks passed
@Hallberg-NOAA Hallberg-NOAA deleted the NonBous_Leith_QG_visc branch May 24, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants