Merge dev master candidate 2019 08 30 #122
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
(*) Remap opacity diagnostic to tanh()
…/MOM6 into Hallberg-NOAA-code_units_cleanup
When the surface state went out of user-specified bounds we reported an error such as: ``` WARNING from PE 130: Extreme surface sfc_state detected: i= 18 j= 18 x= -60.625 y= -72.075 D= 1.9385E+01 SSH=-1.1945E+00 SST=-2.5183E+00 SSS= 3.2605E+01 U-= 0.0000E+00 U+=-8.9452E-03 V-= 0.0000E+00 V+= 0.0000E+00 ``` The i,j here are the on-core local i,j and the x,y are the geographic location (so you can find the location on a map). Neither of these are particularly useful when looking at actual model output unless you are adept on porjections. This commit changes the message to: ``` WARNING from PE 130: Extreme surface sfc_state detected: i= 958 j= 89 lon= -60.625 lat= -72.075 x= -60.042 y= -72.075 D= 1.9385E+01 SSH=-1.1945E+00 SST=-2.5183E+00 SSS= 3.2605E+01 U-= 0.0000E+00 U+=-8.9452E-0 3 V-= 0.0000E+00 V+= 0.0000E+00 ``` which allows you to look at model output using either indices or coordinates and still find the location on a map. - Changes the reported i,j-location to global index - Adds the diagnostic grid-lon,lat to report
When defining a parameter with get_param() we can indicate that the parameter is for debugging purposes with the optional argument `debuggingParam=.true.`. This had been implemented for scalar reals but not for a vector of reals.
More useful message when detecting bad surface state
…ging-param Allows vector-of-reals debugging parameters
The KE_adv diagnostic is a sum of values multiplied by -1, which will assign a -0.0 value for zero-initialized states. This can lead to reproducibility problems for symmetric and nonsymmetric grids, since many intermediate calculations rely on masking of the u field and do not apply masks to subsequent steps. This can occur when a MPP domain is bordered by land, where calculations on the S and W boundaries of a symmetric grids are computed as if they are unmasked, and would be assigned a -0.0 value. For nonsymmetric grids, these values were never computed and would retain a +0.0 value. We resolve this by re-initalizing the KE_u and KE_v fields, since they are re-used as buffers for several diagnostics, and exclude masked points from the calculation. This ensures +0.0 values in any land boundaries across symmetric grids. If the masking is applied to other fields using `KE_u` and `KE_v`, then we may be able to remove the re-initialization step. While +/-0.0 are arithmetically identical in all cases, this fix will preserve bitwise reproducibility and is a step towards phasing out the `abs()` operation in the checksums.
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.
These need to be cleaned before submitting a PR to dev/ncar
|
tx0.66v1 tests are bit for bit but gx1v6 answers change. Is it expected? |
|
I suspect this is because we still have USE_MEKE = True in this configuration. Did we have the same problem in a previous PR? |
|
Right. Any idea why travis is failing? |
|
No. Test tc0 in symmetric mode is the one failing... |
* We deleted these terms previously but forgot to remove posting the diagnostics. Thanks to Travis and .testing/tc0 we were able to identidy this mistake.
Codecov Report
@@ Coverage Diff @@
## dev/ncar #122 +/- ##
===========================================
Coverage ? 43.17%
===========================================
Files ? 213
Lines ? 62163
Branches ? 0
===========================================
Hits ? 26836
Misses ? 35327
Partials ? 0
Continue to review full report at Codecov.
|
|
Finally, it passed all the tests. |
alperaltuntas
approved these changes
Sep 17, 2019
gustavo-marques
pushed a commit
that referenced
this pull request
Aug 5, 2022
Add dimensional rescaling of temperature and salinity
alperaltuntas
pushed a commit
that referenced
this pull request
Jul 7, 2023
Added a missing scale factor in the DENSE_WATER_EAST_SPONGE_SALT get_param call in dense_water_initialize_sponges, and added comments describing the local variables (and their units) throughout the dense_water_initialization module. The variable set by DENSE_WATER_SILL_HEIGHT was unused and it probably was always intended to be DENSE_WATER_SILL_DEPTH, which it now is. Units arguments were also added to two of the unlogged get_param calls in this module. Without this change, this test case would not reproduce with dimensional rescaling due to a scale factor that was omitted when salinity was being rescaled on May 3, 2022, which became a part of PR #122 to dev/gfdl, but answers should not change when dimensional rescaling of salinities is not used. All answers and output in the MOM6-examples test suite are bitwise identical.
alperaltuntas
pushed a commit
that referenced
this pull request
Feb 9, 2024
…1031 update MOM6 to its main repo 20231025 (NCAR candidate) and 20231031(GMAO FMS_cap) updating
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have already checked that these commits do not change answers for NCAR but it would be good to check this again. I had to manually merge the latest dev/ncar due to a conflict in MOM_thickness_diffuse.F90.