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

+*Non-Boussinesq revision of MOM_vertvisc.F90 #428

Merged
merged 2 commits into from
Aug 11, 2023

Conversation

Hallberg-NOAA
Copy link
Member

Revised the code in MOM_vertvisc.F90 to work internally with thickness-based units for the viscous coupling between layers, which eliminates any dependence on the value of GV%Rho_0 in non-Boussinesq mode. Various kinematic viscosities are replaced with dynamic viscosities in non-Boussinesq configurations, including revising the scaled units of the viscosities to [H Z T-1 ~> m2 s-1 or Pa s]. This commit also modifies the code to explicitly use vertical distances rather than thicknesses when calculating the vertical viscous coupling coefficients in vertvisc_coef and find_coupling_coef.

This commit involves changing the units of numerous variables to use thickness, vertical distance, dynamic viscosity or other related units, including:

  • 14 elements in the vertvisc_CS type
  • 6 arguments to the private routine find_coupling_coef
  • 2 arguments to the private routine find_coupling_coef_gl90
  • 1 arguments ("a") to the public routine write_u_accel
  • 1 arguments ("a") to the public routine write_v_accel
  • 1 internal variable in vertvisc
  • 1 internal variable in vertvisc_remnant
  • 23 internal variables in vertvisc_coef,
  • 7+4+4+3 internal variables in find_coupling_coef,
  • 1 internal variable in find_coupling_coef_gl90

Local variables that are no longer needed were eliminated in vertvisc and vertvisc_remnant, while 2 new local variables were added to find_coupling_coef and 6 new local variables were added to vertvisc_coef. In 6 places the Boussinesq reference density was replaced with GV%H_to_RZ, which is equivalent to the reference density in Boussinesq mode, but scales to 1 in non-Boussinesq mode.

The previous dimensional rescaling factor for KD_GL90 was incorrect (and inconsistent with the correct scaling factor used when reading in the analagous spatially varying kappa_gl90_2d); this has been corrected in this commit.

A total of 59 GV%H_to_Z or GV%Z_to_H unit conversion factors or references to GV%Rho_0 were eliminated with these changes, and in non-Boussinesq mode there is no longer any dependence on the Boussinesq reference density.

Replaced the forces argument to find_coupling_coef with an array of the friction velocities and use find_ustar to set them. When in non-Boussinesq mode, this has the effect of using forces%tau_mag and tv%SpV_avg instead of forces%ustar and GV%Rho0 when interpolating the friction velocity and stress magnitude in find_coupling_coef.

Revised units used to set the GL90 viscosities and rescale to convert diagnostics of the vertical viscosities in the MOM_vert_friction module, so that they do not depend on RHO_0 when in non-Boussinesq mode.

To accomodate this change in vertvisc, the units of the "a" arguments to write_u_accel and write_v_accel were also changed to use thickness-based arguments.

Because GV%Z_to_H is an exact power of 2 in Boussinesq mode, all answers are bitwise identical in that mode. In non-Boussinesq mode, the answers are changed by the replacement of the Boussinesq reference density by expressions using the layer-averaged specific volumes. This commit changes the units of 2 arguments in public (diagnostic) subroutine interfaces.

@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Merging #428 (a0bb33b) into dev/gfdl (597bbf1) will increase coverage by 0.00%.
The diff coverage is 66.41%.

❗ Current head a0bb33b differs from pull request most recent head 6956caf. Consider uploading reports for the commit 6956caf to get more accurate results

@@            Coverage Diff            @@
##           dev/gfdl     #428   +/-   ##
=========================================
  Coverage     38.05%   38.06%           
=========================================
  Files           269      269           
  Lines         77064    77133   +69     
  Branches      14225    14233    +8     
=========================================
+ Hits          29327    29358   +31     
- Misses        42419    42456   +37     
- Partials       5318     5319    +1     
Files Changed Coverage Δ
src/diagnostics/MOM_PointAccel.F90 3.85% <0.00%> (ø)
...c/parameterizations/vertical/MOM_vert_friction.F90 57.63% <67.42%> (-0.79%) ⬇️

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

@Hallberg-NOAA Hallberg-NOAA force-pushed the nonBous_vert_friction branch from 202a68e to a1f8cd7 Compare August 2, 2023 10:34
@Hallberg-NOAA Hallberg-NOAA added enhancement New feature or request answer-changing A change in results (actual or potential) labels Aug 2, 2023
@Hallberg-NOAA Hallberg-NOAA changed the title +*Work in thickness units in MOM_vertvisc.F90 +*nonBoussinesq revision of MOM_vertvisc.F90 Aug 2, 2023
@Hallberg-NOAA Hallberg-NOAA changed the title +*nonBoussinesq revision of MOM_vertvisc.F90 +*Non-Boussinesq revision of MOM_vertvisc.F90 Aug 2, 2023
@Hallberg-NOAA Hallberg-NOAA force-pushed the nonBous_vert_friction branch from a1f8cd7 to 1fefaf6 Compare August 2, 2023 10:58
  Revised the code in MOM_vertvisc.F90 to work internally with thickness-based
units for the viscous coupling between layers, which eliminates any dependence
on the value of GV%Rho_0 in non-Boussinesq mode.  Various kinematic viscosities
are replaced with dynamic viscosities in non-Boussinesq configurations,
including revising the scaled units of the viscosities to [H Z T-1 ~> m2 s-1 or
Pa s].  This commit also modifies the code to explicitly use vertical distances
rather than thicknesses when calculating the vertical viscous coupling
coefficients in vertvisc_coef and find_coupling_coef.

  This commit changes the units of numerous variables to use thickness, vertical
distance, dynamic viscosity or other related units, including:

 - 14 elements in the vertvisc_CS type
 - 6 arguments to the private routine find_coupling_coef
 - 2 arguments to the private routine find_coupling_coef_gl90
 - 1 arguments ("a") to the public routine write_u_accel
 - 1 arguments ("a") to the public routine write_v_accel
 - 1 internal variable in vertvisc
 - 1 internal variable in vertvisc_remnant
 - 23 internal variables in vertvisc_coef,
 - 7+4+4+3 internal variables in find_coupling_coef,
 - 1 internal variable in find_coupling_coef_gl90

  Local variables that are no longer needed were eliminated in vertvisc and
vertvisc_remnant, while 2 new local variables were added to find_coupling_coef
and 6 new local variables were added to vertvisc_coef. In 6 places the
Boussinesq reference density was replaced with GV%H_to_RZ, which is equivalent
to the reference density in Boussinesq mode, but scales to 1 in non-Boussinesq
mode.

  The previous dimensional rescaling factor for KD_GL90 was incorrect (and
inconsistent with the correct scaling factor used when reading in the analagous
spatially varying kappa_gl90_2d); this has been corrected in this commit.

  A total of 59 GV%H_to_Z or GV%Z_to_H unit conversion factors or references to
GV%Rho_0 were eliminated with these changes, and in non-Boussinesq mode there is
no longer any dependence on the Boussinesq reference density.

  Replaced the forces argument to find_coupling_coef with an array of the
friction velocities and use find_ustar to set them.  When in non-Boussinesq
mode, this has the effect of using forces%tau_mag and tv%SpV_avg instead of
forces%ustar and GV%Rho0 when interpolating the friction velocity and stress
magnitude in find_coupling_coef.

  Revised units used to set the GL90 viscosities and rescale to convert
diagnostics of the vertical viscosities in the MOM_vert_friction module, so that
they do not depend on RHO_0 when in non-Boussinesq mode.

  To accomodate this change in vertvisc, the units of the "a" arguments to
write_u_accel and write_v_accel were also changed to use thickness-based
arguments.

  Because GV%Z_to_H is an exact power of 2 in Boussinesq mode, all answers are
bitwise identical in that mode.  In non-Boussinesq mode, the answers are changed
by the replacement of the Boussinesq reference density by expressions using the
layer-averaged specific volumes.  This commit changes the units of 2 arguments
in public (diagnostic) subroutine interfaces.
@Hallberg-NOAA Hallberg-NOAA force-pushed the nonBous_vert_friction branch from 1fefaf6 to 48dcd50 Compare August 2, 2023 21:46
@Hallberg-NOAA
Copy link
Member Author

I just pushed a revised version that eliminates some of the extra logical branches that had been added to reproduce answers when fused-multiply-adds (FMAs) are added with one particular compiler, because these could not be guaranteed to be helpful with FMAs or other forms of aggressive optimization on other compilers, and they were just adding some ugly code complexity.

@MJHarrison-GFDL
Copy link

I approve this PR.

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/20230 ✔️

@marshallward marshallward merged commit 3ce1368 into NOAA-GFDL:dev/gfdl Aug 11, 2023
@Hallberg-NOAA Hallberg-NOAA deleted the nonBous_vert_friction branch September 27, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answer-changing A change in results (actual or potential) enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants