Merge dev master candidate 2019 12 17 #136
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
This patch consists of three changes to the test suite: - Tests are now run in dedicated work directories. This allows for parallel execution of tests inside of make. Unexpected improvements are a 3-4x speedup on test time, and the disappearance of several anomalous "negative zero" issues which were otherwise unreproducible outside of the test, suggesting that replacing the serial runs inside of the tc directories has resolved some otherwise unknown problems. - Makefile rules for running tests are now defined relative to the .testing directory, rather than an explicit base directory based on the path of the Makefile. Although this method is slightly less robust and slightly more prone to error, it makes it easier to build and run individual files within the project. Build paths are still absolute, and will be updated in a separate commit. - Test output is now stored in a `results` directory, rather than inside the tc* configuration directories.
Test run work and results dir; relative path rules
This patch addresses the inconsistency of signed zero in the minimum and maximum values used in checksum report. The behavior of the Fortran intrinsic min() and our MPI library's implementation of MPI_Reduce with MPI_MIN can give different results for different values of signed zero, e.g. min(0,-0) vs min(-0,0). Additionally, the MPI_Reduce result appears to not consistenty follow these rules in more complex MPI calculations. Due to these issues, we add the result to positive zero to ensure that any negative zero results are reported as positive.
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.
Added conversion factors to 4 mass-flux diagnostics and comments to 4 others on why no conversion factors are needed. All answers are bitwise identical.
Added scale arguments to 5 chksum calls and grouped another two chksum calls while also adding the right scaling argument. All answers are bitwise identical.
Undoes the dimensional scaling of the cell areas before taking their global sum, so that the reproducing sum does not overflow when there is dimensional rescaling. All answers are bitwise identical when there is no rescaling, but this eliminates a source of inadvertent overflows or underflows in the global sums, and there is a new optional argument to compute_global_grid_integrals.
Corrects the dimensionally inconsistent expressions for the CFL number in the tracer advection code, in which a negligible thickness had been added to the cell volume to avoid division by zero. This change does not alter the solutions in the MOM6-examples test cases, but now it permits dimensional rescaling of lengths over a much larger range, and it could change answers if the minimum layer thicknesses are small enough.
Unscale interface heights before taking a global average via a reproducing sum in non-Boussinesq mode global diagnostics to permit dimensional consistency testing over a larger range. All answers are bitwise identical.
Added an optional tmp_scale argument to global_i_mean and global_j_mean to specify an internal rescaling of variables being averaged before the reproducing sum. All answers are bitwise identical, but there are new optional arguments to two public interfaces.
Use tmp_scale when taking the i-mean interface heights for i-mean sponges, to give a greatly expanded range of dimensional consistency testing. All answers are bitwise identical.
MOM6: +(*)Dimensional consistency completion
Refactored how time-averaging of fluxes in forcing types that span multiple timesteps and flux diagnostics are handled, and rescaled the units of fluxes%dt_buoy_accum from [s] to [T]. This involved changing the arguments to fluxes_accumulate, forcing_accumulate, mech_forcing_diags and forcing_diagnostics, but because of the differing types of the arguments, an incompatible mix of code will not compile. Also changed the units of dt as passed to accumulate_net_input, and made a minor change to extractFluxes1d to avoid the possibilty of a division by zero. All answers are bitwise identical, but there are public interface changes, including changes that impact the mct and nuopc driver codes.
Added the new runtime parameters KAPPA_SHEAR_ITER_BUG and KD_TRUNC_KAPPA_SHEAR to permit correction of a dimensionally inconsistent expression in the Newton's method solver code of kappa_shear, and to allow the value of shear mixing that is neglected compared with the background mixing to be set at run-time instead of being hard-coded. By default, all answers are bitwise identical, but there are two new runtime parameters and the MOM_parameter_doc files change.
Added the new runtime parameter VERT_FRICTION_2018_ANSWERS that avoids the use of the hard-coded maximum viscous mixing length per timestep in the vertical viscosity code, and added h_neglect in the denominators of several terms in the viscosity code. All answers in the MOM6-examples test cases are bitwise identical, but the answers will change if ANGSTROM is set to 0, and there is a new entry in the MOM_parameter_doc files.
…AA/MOM6 into Hallberg-NOAA-revise_vert_friction
…OAA/MOM6 into Hallberg-NOAA-simplify_forcing_time
…candidate-2019-12-17
alperaltuntas
requested changes
Jan 3, 2020
There was a problem hiding this comment.
mct tests are failing due to an error in mct_driver/mom_ocean_model_mct.F90. @gustavo-marques see my below comment.
| @@ -520,10 +520,10 @@ subroutine update_ocean_model(Ice_ocean_boundary, OS, Ocean_sfc, & | |||
| endif | |||
| if (OS%icebergs_alter_ocean) then | |||
| if (do_dyn) & | |||
| call iceberg_forces(OS%grid, OS%forces, OS%use_ice_shelf, & | |||
| call iceberg_forces(OS%grid, OS%US, OS%forces, OS%use_ice_shelf, & | |||
There was a problem hiding this comment.
Looks like OS%US argument here was added inadvertently. It needs to be removed.
There was a problem hiding this comment.
@Hallberg-NOAA most likely added this to enable dimensional scaling testing. He probably ought to be involved in the conversation if it needs to be removed.
There was a problem hiding this comment.
The equivalent call in coupled_driver does not have this argument. We will wait for @Hallberg-NOAA's confirmation before deleting it.
There was a problem hiding this comment.
I think @Hallberg-NOAA meant to add the OS%US argument to iceberg_fluxes() calls, and NOT iceberg_forces() , which doesn't expect a unit scaling argument.
There was a problem hiding this comment.
Also, we need to add the OS%US argument to iceberg_fluxes() call in line 557. After making both changes, all CESM tests have passed. @gustavo-marques , once @Hallberg-NOAA confirms, I can push both changes and merge this.
There was a problem hiding this comment.
Sounds good, thanks @alperaltuntas.
There was a problem hiding this comment.
Yes, the correction that @alperaltuntas suggests is the right one. This OS%US argument should only have been added to iceberg_fluxes() and not to iceberg_forces(). Please go ahead and make the change to the mct coupler code as suggested.
Corrected arguments to iceberg_forces and iceberg_fluxes in the mct version of update_ocean_model. This should correct the recently introduced problems with compiling MOM6 with the mct coupler.
…candidate-2019-12-17
|
@alperaltuntas suggested corrections have been merged. Please evaluate this PR again. |
alperaltuntas
approved these changes
Jan 7, 2020
alperaltuntas
pushed a commit
to alperaltuntas/MOM6
that referenced
this pull request
Oct 17, 2024
…240531 update to main 20240729 commit (gfdl-to-main-2024-05-31)
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.
GFDL tests are b4b after this merge.