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

MOM6: +Expand dimensional consistency testing #1019

Merged
merged 111 commits into from
Nov 13, 2019

Conversation

Hallberg-NOAA
Copy link
Collaborator

This PR includes a series of changes that extends the dimensional consistency
testing, particularly of variables related to density. Some bugs have been
noted with comments but not corrected yet. All answers are bitwise identical,
and there are no changes to MOM_parameter_doc or other output files. MOM6
commits with this PR include:

  Rescaled the timestep arguments to set_viscous_ML, vertvisc, vertvisc_coef,
vertvisc_remnant, write_u_accel and write_v_accel to use units of [T].  All
answers are bitwise identical, but the units of some public arguments have
been rescaled.
  Added factors for power-of-2 rescaling of density to the unit_scale_type,
along with the new run-time parameter R_RESCALE_POWER.  All answers are
bitwise identical, but there is a new runtime parameter, some new elements in
a transparent public type, and a new optional variable in the MOM restart
files.  This adds a new entry to the MOM_parameter_doc.debugging files.
  Added two new dimensional conversion factors, H_to_RZ and RZ_to_H, to the
MOM6 vertical grid, in preparation for adding testing of dimensional rescaling
of density to the MOM6 code.  All answers are bitwise identical, but a
transparent type has two new elements.
  Rescaled density units in MOM_bulk_mixedlayer for dimensional consistency
testing.  All answers are bitwise identical.
  Added a new optional scale argument to calculate_density, calculate_spec_vel
calculate_density_derivs, calculate_density_second_derivs, and
calculate_specific_vol_derivs, to rescale the densities or related variables.
All answers are bitwise identical, but there are new optional arguments to
public interfaces.
  Rescale bulkmixedlayer densities and their derivatives via the calls to
calculate_density and calculate_density_derivs.  All answers are bitwise
identical.
  Rescaled density units in MOM_entrain_diffusive for dimensional consistency
testing.  All answers are bitwise identical.
  Changed the units of GV%Rlay from [kg m-3] to [R] for dimensional consistency
testing.  This required the addition of unit_scale_type arguments to several
interfaces.  All answers are bitwise identical, but new arguments have been
added to several public interfaces.
  Moved rescaling of Rlay to [R] into the various set_coord routines.  This
required the addition of unit_scale_type arguments to two interfaces.  All
answers are bitwise identical, but new arguments have been added to two
public interfaces.
  Changed the units of GV%Rho0 from [kg m-3] to [R] for dimensional consistency
testing.  This required the addition of unit_scale_type arguments to several
interfaces.  All answers are bitwise identical, but new arguments have been
added to several public interfaces and the units of an element in a public
type have changed.
  Rescaled density units in MOM_regularize_layers for dimensional consistency
testing.  All answers are bitwise identical.
  Rescaled density units in MOM_set_viscosity for dimensional consistency
testing.  All answers are bitwise identical.
  Rescaled density units in MOM_set_diffusivity for dimensional consistency
testing.  All answers are bitwise identical.
  Rescaled density units in MOM_kappa_shear for dimensional consistency
testing.  All answers are bitwise identical.
  Rescaled density units in MOM_internal_tide_input for dimensional consistency
testing.  All answers are bitwise identical.
  Rescaled density units in diagnoseMLDbyDensityDifference in MOM_diabatic_aux
for dimensional consistency testing.  All answers are bitwise identical.
  Rescaled density units in MOM_tidal_mixing for dimensional consistency
testing.  All answers are bitwise identical.
  Rescaled density units in MOM_geothermal for dimensional consistency
testing.  This required adding a unit_scale_type argument to geothermal_init.
All answers are bitwise identical, but a public interface has a new argument.
  Corrected dimensions in comments.  All answers are bitwise identical.
  Partially rescaled the units of itide%TKE_itidal_input for dimensional
consistency testing.  All answers are bitwise identical, but the units of an
element of a transparent public type have changed.
  Rescaled density units in MOM_energetic_PBL for dimensional consistency
testing.  All answers are bitwise identical.
  Rescaled the density units of the cTKE or TKE_forced variables passed to
energetic_PBL and applyBoundaryFluxesInOut for dimensional consistency testing.
All answers are bitwise identical, but the units of an argument to two public
interfaces have changed.
  Rescaled the specific volume (density) units of the dSV_dT and dSV_dS
variables passed to energetic_PBL, applyBoundaryFluxesInOut, and
absorbRemainingSW for dimensional consistency testing.  Also rescaled the
dimensions of TKE returned from absorbRemainingSW.  All answers are bitwise
identical, but the units of a arguments to 3 public interfaces have changed.
  Used GV%H_to_RZ to simplify rescalings in applyBoundaryFluxesInOut and
absorbRemainingSW.  All answer are bitwise identical.
  Rescaled the units of the mixed layer densities passed to apply_sponge and
set_up_sponge_ML_density to [R] for dimensional consistency testing.  This
required adding a unit_scale_type argument to RGC_initalize_sponges.  All
answers are bitwise identical, but the units of two arguments to public
interfaces have changed.
  Rescaled units of dRhodT in applyBoundaryFluxesInOut for dimensional
consistency testing.  All answers are bitwise identical.
  Rescaled density units in MOM_thickness_diffuse for dimensional consistency
testing.  All answers are bitwise identical.
  Rescaled units of diagnostic FrictWork variables in MOM_hor_visc.F90 for
dimensional consistency testing.  All answers are bitwise identical.
  Rescaled density units in MOM_mixed_layer_restrat for dimensional consistency
testing.  All answers are bitwise identical.
This adds dimensional scaling to the vprec diagnostic, and resolves some
variable name changes and additions to the OpenMP directives.  This
fixes some of the tests in GitHub PR 1019.
  Renamed internal variables dt to dt_in_s and dt_in_T to dt in the
MOM_dynamics_... files in preparation for passing in timesteps in units of [T].
  Pass timesteps to apply_ALE_sponge, apply_sponge, geothermal and
regularize_layers in units of [T], and also store the sponge restoring rates
internally in units of T-1.  This required passing new vertcalGrid_type and
unit_scale_type arguments to init_sponge_diags.  Also renamed internal variables
dt to dt_in_s and dt_in_T to dt in MOM_diabatic_driver.F90 and rescaled the
units of the vertical advective and diffusive heat and salt fluxes.  All answers
are bitwise identical, but public interfaces have changed.
  Renamed internal variables dt_in_T to dt in multiple files.  All answers are
bitwise identical.
  Rescaled the time units of advective and diffusive tracer diagnostics.  Also
renamed the internal variable dt to dt_in_s and dt_in_T to dt in
MOM_tracer_hor_diff.F90.  All answers are bitwise identical.
  Modified comments to clarify the options for the units of arguments to
tracer_vertdiff.  Only comments have changed, and all answers are bitwise
identical.
  Pass timesteps to step_MOM_dyn_split, step_MOM_dyn_split_RK2,
step_MOM_dyn_unsplit, diabatic, advect_tracer and tracer_hordiff in units of
[T].  Also corrected some comments.  All answers are bitwise identical, but the
units of some arguments have been rescaled.
  Corrected the pointer being passed to the restart registration call for
m_to_L.  This could fix the ability to change the dimensional rescaling between
restarts.  All answers in the test cases are bitwise identical.
  Pass the timestep to the tracer_vertdiff calls for temperature and salinity in
units of [T}.  This argument is not used in these cases, so this is really just
code cleanup.  All answers in the test cases are bitwise identical.
@Hallberg-NOAA
Copy link
Collaborator Author

This updated PR addresses some concerns from @marshallward and @adcroft about unnecessarily renaming variables (especially dt to dt_in_T). There should now be fewer code differences with the previous code (but some more differences in comments), while also having slightly more complete dimensional rescaling of the timesteps.

  Corrected the dimensional rescaling in MOM_MEKE.F90; this scaling was not
properly automatically merged from dev/gfdl.  All answers are bitwise identical
in the MOM6-examples test cases.
  Added enable_averages, a new interface for enabling diagnostic averages using
a time interval specified in [T].  All answers are bitwise identical, but there
is a new public interface.
  Enabled averaging diagnostics via calls to enable_averages in
MOM_diabatic_driver and the three MOM_dynamics modules.  All answers are bitwise
identical.
  Rescaled various internal timesteps in MOM.F90 for code simplification and
expanded dimensional consistency testing, including replacing enable_averaging
calls with calls to enable_averages, eliminating all use of dt_in_s and renaming
dt_in_T back to dt.  All answers are bitwise identical.
  Fixed dimensional rescaling unit conversion factors for 7 diagnostics and pass
the timestep to neutral_diffusion in [T] for diagnostic purposes.  All answers
are bitwise identical.
  Rescaled the timesteps passed to calculate_diagnostic_fields,
post_surface_thermo_diags and post_transport_diagnostics in units of [T] for
more complete dimensional consistency testing.  Also added unit_scale_type
argument to register_surface_diags.  All answers and diagnostics are bitwise
identical.
@Hallberg-NOAA
Copy link
Collaborator Author

This PR is being tested (again) with https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/9358 after correcting some diagnostic rescaling and merging in the latest version of dev/gfdl.

@@ -237,9 +237,11 @@ subroutine convert_IOB_to_fluxes(IOB, fluxes, index_bounds, Time, G, US, CS, &
!! is present, or false (no restoring) otherwise.
logical :: restore_sst !< local copy of the argument restore_temp, if it
!! is present, or false (no restoring) otherwise.
real :: delta_sss !< temporary storage for sss diff from restoring value
real :: delta_sss !< temporary storage for sss diff from restoring value2
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this "2" intentional? Just curious...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, that "2" at the end of a comment was a mistake. Sorry about that. Although I tried to be as careful as possible, it would be helpful if someone from NCAR (@gustavo-marques ?) would please evaluate whether the few necessary changes to the mct_driver and nuopc_driver code are as benign as they were intended to be.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Hallberg-NOAA Do you want to wait for feedback from MCT/NUOPC devs before merging?

Copy link
Collaborator

@adcroft adcroft left a comment

Choose a reason for hiding this comment

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

92 files!

@marshallward
Copy link
Collaborator

Scratch my previous comment, but noting here that this does make a parameter change (R_RESCALE_POWER) and needs a MOM6-examples update.

@marshallward marshallward merged commit 840881d into mom-ocean:dev/gfdl Nov 13, 2019
@Hallberg-NOAA Hallberg-NOAA deleted the density_rescale branch July 30, 2021 18:56
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.

4 participants