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

dev/master candidate 2019-08-30 #988

Merged
merged 351 commits into from
Sep 13, 2019
Merged

Conversation

adcroft
Copy link
Collaborator

@adcroft adcroft commented Aug 30, 2019

This PR submits everything on dev/gfdl as of 2019/08-30 to dev/master. As per usual feedback from @gustavo-marques, @awallcraft, @kshedstrom and @jiandewang is asked for before we accept this PR.

Main features:

  • .testing: Unbeknownst to most of you, the tests run on Travis-CI have been managed by the contents of the .testing directory for a while. We have now removed the inverse dependency on MOM6-examples and have defined some rudimentary test configurations (TCs) within the .testing. These are not usable configurations for science but sufficient for some code tests. If you have connected Travis-CI to your GitHub fork then with each push you will now invoke tests for 1) restarts, 2) decomposition, 3) dimension scaling, 4) symmetric/non-symmetric memory, 5) uninitialized data, and 6) optimization tolerance. A pull request also conducts a regression test against the parent branch. If Travis-CI is showing you a red X then you can now run the tests interactively. See the .testing/README.md for pointers. @marshallward has made all this work and will be expanding the coverage of these tests on the dev/gfdl branch over the coming months.
  • Dimensional scaling: @Hallberg-NOAA has touched a lot of code to test that expressions are dimensionally consistent and to allow us to continue to test expressions (this is the reason for the number of commits in this PR).
  • OBC fixes: @kshedstrom and @MJHarrison-GFDL have been fixing bugs in the open boundary code that does not affect most configurations but since the code is beginning to be used in regional configurations beware.
  • Answer changes: Numerous fixes have been implemented in diagnostics and forward code. Where the latter changed answers, we made the changes run-time conditional which has introduced numerous flags. By default, all these flags are set to not change answer but subsequent to this PR we will be rolling forward all the GFDL configurations to give new (corrected) answers. We recommend evaluating each of the new flags to see if you can move to the newer code so that in the future when we eventually remove the old code you are not adversely affected. The flags are:
    • EPBL_2018_ANSWERS
    • HOR_VISC_2018_ANSWERS
    • OPTICS_2018_ANSWERS
    • SET_DIFF_2018_ANSWERS
    • SET_VISC_2018_ANSWERS
    • SURFACE_FORCING_2018_ANSWERS
    • TIDAL_MIXING_2018_ANSWERS
    • WIND_GYRES_2018_ANSWERS

There are no answer changes implied for the MOM6-examples configurations but because of new parameters a branch with the same name has been pushed to https://github.com/NOAA-GFDL/MOM6-examples/tree/dev-master-candidate-2019-08-30.

adcroft and others added 30 commits July 11, 2019 19:48
  Removed commented out controlled_forcing calls and other code from the
ice_solo_driver, mct_driver, and nuopc_driver code.  This capability has never
been completed, so until it is working, it is only being retained (in commented
out form) in the coupled_driver code.  All answers are bitwise identical.
  Rescaled the units of pbce to L2 H-1 T-2 from m2 H-1 s-2 for enhanced
dimensional consistency testing.  All answers are bitwise identical.
  Rescaled the units of fluxes%buoy to L2 T-3 from m2 s-3 for enhanced
dimensional consistency testing.  This required the addition of unit_scale_type
arguments to several initialization or buoyancy flux routines. All answers are
bitwise identical.
  Rescaled the units of buoyancyFlux returned by the calculateBouyancyFlux
routines and the units of the buoyFlux arguments to KPP_calculate and
KPP_compute_BLD, all of which are now in [L2 T-3] for dimensional consistency
testing.  All answers are bitwise identical.
  Rescaled the internal representation of diagnostics in entrainment_diffusive.
All answers are bitwise identical.
  Rescaled the internal representation of commented out diagnostics and added
some clarifying comments in find_TKE_to_Kd. All answers are bitwise identical.
  Rescaled the internal representation of commented out diagnostics in
applyBoundaryFluxesInOut. All answers are bitwise identical.
  Explicitly set the sizes of optics-related arguments to mixedlayer_convection,
mechanical_entrainment, and sumSWoverBands so that these arrays do not alwasy
have to start at 1, thereby facilitating global indexing.  This involves adding
a new integer argument to sumSWoverBands.  All answers are bitwise identical.
  Added a new runtime paramter, KELVIN_WAVE_2018_ANSWERS, to select code that
changes to expressions that are rotationally symmetric and avoids problems that
could arise from spatially varying coefficients that are not being recalculated
within apatial loops.  By default all answers are bitwise identical, but the
MOM_parameter_doc files have a new entry if USE_KELVIN_WAVE_OBC is true.
  Replaced GV%g_Earth with the fully dimensionally rescaled GV%LZT_g_Earth, the
MKS variable GV%mks_g_Earth, or other combinations of variables, like GV%Z_to_H
and GV%H_to_Pa, throughout the MOM6 code.  All answes are bitwise identical.
  Renamed GV%LZT_g_Earth to GV%g_Earth and eliminated the separate GV%g_Earth,
which is no longer in use anywhere in the MOM6 code.  All answers are bitwise
identical.
  Changed the dimensions of the variables used to calculate a bulk Richardson
number in set_viscous_ML to use units of L2 T-2 for velocities squared.  All
answers are bitwise identical.
  There are two versions of vert_fill_TS in MOM_isopycnal_slopes.F90 and
MOM_thickness_diffuse.F90.  This commit changes how they handle massless layers
and brings the two versions closer together, including combining the diffusivity
and timestep arguments into a single argument of their product and adding a new
optional logical argument to cause the isopycnal_slopes version to reproduce the
answers from the thickness_diffuse version.  Also added a the existing runtime
parameter KD_SMOOTH to MOM_set_diffusivity for use when SET_DIFF_2018_ANSWERS is
false, but this does not change the MOM_parameter_doc files.  All answers are
bitwise identical in the existing MOM6-examples test cases, but there could be
answer changes when there are zero thickness layers.
  Removed the version of vert_fill_TS from MOM_thickness_diffuse.F90 because
identical functionality can be obtained via MOM_isopycnal_slopes, provided that
the optioanl argument larger_h_denom=.true. is used.  All answers are bitwise
identical, but there has been a relocation to a new module and a slight change
in one of the public interfaces.
The MEKE criteria for using the FrictWorkMax diagnostic was not
sufficient, since it is possible to use MEKE while still unable to
compute some of the terms required for FrictWorkMax.

We resolve this by adding the MEKE_type struct as an argument for
hor_visc_init, and use the same information when computing
FrictWorkMax to determine whether to register FrictWorkMax.

This changes the API by adding MEKE to most of the timestep
initializations, but should not affect answers.
  Added the runtime parameter HOR_VISC_2018_ANSWERS to permit the elimination of
a dimensional constant without changing answers. Rescaled the units of several
time variables in MOM_hor_visc.  Also added comments indicating issues with the
GME options and suggests for how to correct them.  All answers are bitwise
identical, but there is a new entry in the MOM_parameter_doc files.
  Changed the units of diffu and diffv as returned by MOM_hor_visc to [m s-1
T-1] for dimensional consistency testing.  Additional unit changes will come
automatically after the units of horizontal viscosities are rescaled.  All
answers are bitwise identical, but the units of some arguments to public
functions and elements of types have been rescaled.
  Changed the units of the 6 str_xx and str_xy variables in MOM_hor_visc to
[H m2 s-1 T-1] for dimensional consistency testing.  All answers are bitwise
identical.
  Changed the units of lateral viscosities to [m2 T-1] and the units of
biharmonic viscosities to [m4 T-1] in MOM_hor_visc.F90 for expanded dimensional
consistency testing.  All answers are bitwise identical.
  Changed the units of the timestep to [T} in hor_visc_init and of vert_vort_mag
to [T-1 m-1] in horizontal_viscosity.  Also added a variant of the
grad_vel_mag_h calculation with parentheses for rotational symmetry when
answers_2018 = False.  Changed the marks around suggestions for correcting
issues with the recently added GME code to #GME# to help in finding them.  All
answers are bitwise identical.
Stronger conditional registration of FrictWorkMax
  Changed the units of MEKE%Ku and MEKE%Au to [m2 T-1], including adding code
to allow for the dimensional scaling to change across restarts and moving the
halo updates on any MEKE variables read from restart files to the end of
MEKE_init.  Also change the units of GME_coeff in horizontal_viscosity to
[m2 T-1].  This also required adding a unit_scale_type argument to MEKE_init.
All answers are bitwise identical, but the units for some variables in a
publicly visible type have changed.
  Combined halo updates inside of the MEKE code into group passes to reduce
latency.  Also made del2MEKE into a local variable and removed it from the MEKE
control structure.  All answers are bitwise identical.
  Eliminated an unnecessary halo update in horizontal_viscosity.  All answers
are bitwise identical.
  Added a new runtime parameter, VERY_SMALL_FREQUENCY, to control how close to
zero some frequencies that appear in the denominator of some expressions for the
resolutoin functions can get.  Also added some comments and rearranged some code
addressing problems in calc_QG_Leith_viscosity.  By default, all answers in the
MOM6-examples test cases are bitwise identical, but there is a new entry in the
MOM_parameter_doc.all files.
This patch remaps the opacity diagnostic to a tanh function, i.e.

    op -> 1/L * tanh(op * L)

where L is arbitrarily set to 10^-10 (1 Angstrom).  For op << 1/L, the
diagnostic is nearly equivalent to the model opacity.  For values
comparable and larger than L, the diagnostic approaches 1/L, a
sufficiently large value to reproduce the effects of a divergent
opacity.

This allows us to safely manipulate and store the opacity while also
avoiding infinite values and floating point overflow.

This change will modify the opacity diagnostic, but should not affect
the dynamic state.
  Rescaled the units of the f2_dx2_... and beta_dx2_... elements of VarMix_CS.
These particular arrays are only used in calc_resoln_function, and because these
are raised to arbitrary powers they have to be rescaled back to mks units in
some cases.  All answers are bitwise identical in the MOM6-examples test cases.
adcroft and others added 23 commits August 28, 2019 18:05
This PR updates the README to include instructions for the test suite
Makefile.  It also fixes a bug in the travis config file which may have
been preventing code coverage updates.
The variables used for calculating internal heat in MOM_geothermal.F90
relied on zero-initialization due to the conditional do_i() check inside
of the loops, which was producing NaN warnings in our test suite.

This patch resolves this by conditionally initializing the fields based
on diagnostic registration.

It also introduces two new logical variables to control the
initialization and evaluation of the arrays.
ocean.stats and chksum_diag output is now tagged as PRECIOUS, and is
retained after a run.  This helps to identify an issue after a fail.

.gitignore was also restructured to move the test ignore rules into a
subdirectory.

Finally, we no longer rely on (-) to ignore commands in Makefile, since
it was causing odd issues after PRECIOUS was introduced.  We now just
explicitly ignore any commands by piping nonzero commands to `true`.
Initialize geothermal internal heat diagnostics
f90nml is no longer used to modify the input.nml files for restart runs,
since it was adding an unnecessary dependency.

Code coverage is now conditionally enabled for the symmetric build, and
controlled by the REPORT_COVERAGE flag, since it can significantly
increase the build and run times.
The z-interpolated uhml and vhml diagnostics gave inconsistent answers
across layouts when using the general mixed layer restratification
scheme, because the value of h had changed but its halos had not been
updated.

This had been previously fixed in the BML restratification but not the
general stratification method.

This patch updates the value of h by conditionally updating the halos if
this diagnostic is required.
This PR enables coverage cleanup and CodeCov upload for test runs.

It also introduces an optional configuration file, config.mk which can
be used for user-configured settings, such as templates or mpirun
commands.
Path rules for cleaning up old code coverage files (*.gcda) and for
running the CodeCov.io upload script have been fixed.
- The OBC tracer reservoirs were being updated in MOM_tracer_advect -
  twice each! Update them separately after tracer advection.
- The OBC tracer lengthscale was being cubed to get the volume. Change
  that to a lengthscale times the face area where the advection is
  happening.
- Changes answers if the tracer lengthscales were not set to zero.
- Could make it more verbose to let the user know which data are
  missing.
Travis: config.mk user config; enable coverage
*Add new update_segment_tracer_reservoirs routine
(*) Bugfix: [uv]hml z-diags in general restrat
- As agreed on the MOM6 dev call, we renamed MOM_surface_forcing.F90
  to  help distinguish it from the other caps. It is now called
  MOM_surface_forcing_gfdl.F90.
@kshedstrom
Copy link
Collaborator

I approve this PR.

@gustavo-marques
Copy link
Collaborator

We approve this PR.

@awallcraft
Copy link
Collaborator

I approve this PR.

@jiandewang
Copy link
Collaborator

jiandewang commented Sep 5, 2019 via email

@adcroft adcroft merged commit c7d2a71 into dev/master Sep 13, 2019
@adcroft adcroft deleted the dev-master-candidate-2019-08-30 branch December 3, 2019 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants