GFDL to main candidate branch (2022-05-16) #1569
Closed
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
remove conflict with dev/emc
further resolve conflict
put id_sppt_wts, etc back.
* remove white space and fix comment * Update MOM_oda_incupd.F90 remove unused index bounds, and fix sum_h2 loop. Co-authored-by: pjpegion <[email protected]> Co-authored-by: Marshall Ward <[email protected]>
- Pointing to OBC wiki file from the lateral parameterizations doc. - Using the MOM6 verbosity to control the time_interp verbosity. - Making the check for negative water depths more informative.
This patch removes any unused variables detected by -Wunused-variables in GCC. This may include variables which are referenced in commented "zombie code". In some cases, the variables were preserved in comments, but not in all cases. If the zombie code is restored, it should be possible to reconstruct the original declaration. (All known cases were simple scalars, such as loop indices). In one case, a variable was conditionally used within a preprocessor `#ifdef __DO_SAFETY_CHECKS__` block. This patch moves the declaration into another `#ifdef` block, but perhaps it is preferable to remove the block. Certain documentation variables such as `mdl`, which typically contain the function name, have been removed if unused. The strongest motivation for this patch is to allow us to enable the `-Wall` flag, which includes unused variables, and will strengthen our ability to detect potential errors in the codebase.
Added the new run-time parameter ODA_2018_ANSWERS to recover the answers from the previous version of the code, which did not supply properly dimensional rescaled minimum thicknesses for the remapping calls in the ODA driver. When this is set to True, all answers are bitwise identical.
Remove unused variables
Added a call to set thickness_units, which are used in the units description for the diagnostic deta_dt, which was recently added by #99, but with an uninitialized variable being used for the units description. This changes contents of one entry in the MOM_available_diags, which will once again reproduce across runs and compilers. All solutions are bitwise identical.
…omain_rescaling_fix
+Fix some rescaling issues smoked out by the regional Bering domain
Removed unused module use statements for EOS_type, calculate_density or calculate_density_derivs in 20 files. All answers are bitwise identical.
Added comments documenting the units of the variables in diagnoseMLDbyEnergy and slightly refactored this routine to clean up its call to calculate_density and eliminated a redundant array of interface depths. Also fixed several spelling errors in comments. All answers and diagnostics are bitwise identical.
Documented the units of variables as they actually appear in subroutine calls to various equation of state or density integral routines, eliminating the potentially confusing lists of alternative units in comments. Only comments are changed, and all answers are bitwise identical.
Revised the calculation of the drho_dT and drho_dS diagnostics to use dimensional consistency testing, along with the newer interface to calculate_density that takes dimensionally rescaled arguments. With this change, the units of most the variables in this section of code match their descriptions in comments, although there is still the local re-use of some 3-d arrays as temporaries with units that do not match. All answers and output are bitwise identical.
Refactored MOM_density_integrals to use the newer calculate_density_1d() and calculated_stanley_density_1d() interfaces to the equation of state routines, and to thereby shift all related dimensional rescaling into MOM_EOS.F90. Also revised the comments describing the arguments to a number of the equation of state routines to eliminate confusing options and clearly indicate the units of each input and output variable. As a part of this change, the units of the rho_ref argument to calculate_stanley_density_1d were changed from [kg m-3] to [R ~> kg m-3] to match the equivalent routine calculate_density_1d(). Because this does not appear to have been used previously, this should not be a problem, and answers will not change unless a dimensional consistency test is underway. All answers are bitwise identical, but there is one minor change to the rescaled units of one apparently unused optional argument.
Modified the comments describing the units of the arguments to int_density_dz_wright, int_spec_vol_dp_wright int_density_dz_linear and int_spec_vol_dp_linear so that they reflect the units as they are used in practice where they are called from analytic_int_density_dz or analytic_int_specific_vol_dp. All answers are bitwise identical.
This commit has several interface changes to some little-called equation of state routines to follow the patterns set by the most commonly used equation of state routines. All answers in test cases are bitwise identical. - Added calculate_TFreeze_1d to the overloaded interface calculate_TFreeze, with dimensional rescaling of its arguments taken from its EOS_type argument and an optional two-element domain, rather than two mandatory integer arguments used with calculate_TFreeze_array. The older interface is also retained within the overloaded interface to calculate_TFreeze. - Modified calculate_density_scalar and calculate_stanley_density_scalar to use units of [R ~> kg m-3] for its rho_ref optional argument, following the pattern from calculate_density_1d. These arguments were not previously used. - Renamed the internally visible routine calculate_density_second_derivs_array to calculate_density_second_derivs_1d and changed its argument list to take an optional two-element domain, rather than two mandatory integer arguments, to follow the pattern set by calculate_density_derivs_1d. Because this routine was only being called in two places the older interface is not being preserved in the overloaded interface calculate_density_second_derivs. - Renamed the internally visible routine calculate_compress_array to calculate_compress_1d and changed its argument list to take an optional two-element domain, rather than two mandatory integer arguments, to follow the pattern set by calculate_density_derivs_1d. Because this routine was only being called in one place the older interface is not being preserved in the overloaded interface calculate_compress. - Eliminated some unnecessary local variables (mostly p_scale) for brevity and code clarity. - Modified two calls to calculate_density_second_derivs in thickness_diffuse_full to use its revised interface. - Modified one call to calculate_compress in build_slight_column to use its revised interface.
Corrected a bug in the calculation of drho_dS_dP and drho_dT_dP in the calculate_density_second_derivs routines, where the inverse of the correct rescaling was being used. However, these routines are only called in a very few places and these particular output fields are not being used, so this bug does not alter any existing MOM6 solutions.
Note that most of these commits are from previously squashed pull requests, and this PR is restoring them. - 6360dbb Merge branch 'main' into main_to_dev - bac8031 Merge pull request mom-ocean#1566 from jiandewang/EMC-FMS-mixed-mode-20220411 - e532d86 Merge pull request #88 from marshallward/missing_attrib_with_class_bugfix - d380f1d An alternate fix to class(*) issues with FMS 2022-01 - 8ecf333 Merge pull request #87 from jiandewang/feature/update-to-main-20220317 - ba37f94 Merge remote-tracking branch 'FSU/main' into feature/update-to-main-20220317 this is corresponding to MOM6 main 20220317 commit (hash # 399a7db) - 44313d9 Merge pull request #85 from jiandewang/feature/update-to-main-20220217 - 966707f Merge remote-tracking branch 'GFDL/main' into feature/update-to-main-20220217 this is corresponding to MOM6 main branch 20220217 commit (hash # 6f6d4d6), which originally based on GFDL-candidate-20220129 - 32c0e1e Merge pull request #81 from jiandewang/feature/update-to-main-20211220 - 9642b1d delete external/OCEAN_stochastic_phyiscs directory as Phil re-coded in external/stochastic_physics directory - e7c9ada solve minor conflict in mom_cap.F90 mom_ocean_model_nuopc.F90 and MOM_energetic_PBL.F90, add two new files: src/parameterizations/stochastic/MOM_stochastics.F90 and config_src/external/stochastic_physics/stochastic_physics.F90 - 90d5961 Merge pull request #78 from jiandewang/feature/update-to-GFDL-20211019 - fd02017 Merge remote-tracking branch 'GFDL/main' into feature/update-to-GFDL-20211019 - 36f17eb Merge pull request #72 from pjpegion/ocn_stoch_july2021 - a9a957e return a more accurate error message in MOM_stochasics - 56bb41e Merge branch 'ocn_stoch_july2021' of https://github.com/pjpegion/MOM6 into ocn_stoch_july2021 - ca2ae1c update to dev/emc - 14ca4a1 Merge pull request #76 from jiandewang/feature/update-to-GFDL-20210914 - 29016c2 Merge remote-tracking branch 'GFDL/main' into feature/update-to-GFDL-20210914 merge GFDL main 20210914 commit (hash # c09e199) - a8577df Merge branch 'NOAA-EMC:dev/emc' into ocn_stoch_july2021 - f8a8e4c update to gfdl 20210806 (#74) - 16e6af0 update to dev/emc - 237a510 add comments - 1b4273d revert logic wrt increments - 5b2040e add logic to remove incrments from restart if outside IAU window - c5f2b72 add write_stoch_restart_ocn to MOM_stochastics - bdf2dc7 doxygen cleanup - 8bc4acc move stochastics to external directory - a3fa3a1 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch_july2021 - e4bc007 stochastic physics re-write - 202cbd4 update to dev/emc - 61717ee Merge remote-tracking branch 'origin/dev/emc' into ocn_stoch - 565e0bb remove debug statements - a4c0411 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch - 689a73f remove PE_here from mom_ocean_model_nuopc.F90 - 8afe969 clean up of mom_ocean_model_nuopc.F90 - 25ed4fc revert MOM_domains.F90 - b8d9888 place stochastic array in fluxes container and make SPPT specific arrays allocatable - d984a7e remove stochastics container - eb88219 clean up of code for MOM6 coding standards - 6e3ea1b correct coupled_driver/ocean_model_MOM.F90 and other cleanup - 0b99c1f make stochastics optional - 85023f8 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch - 80f9f44 clean up MOM_domains - 5443f8e remove blank link in MOM_diagnostics - 1727d9a re-write of stochastic code to remove CPP directives - 600ebf9 Merge remote-tracking branch 'upstream/dev/emc' into ocn_stoch - 6bb9d0b fix non stochastic ePBL calculation - 1d7ffa3 clean up code - 040e1f1 Merge pull request #13 from NOAA-EMC/dev/emc - 2cba995 Merge branch 'dev/emc' into ocn_stoch - 1dc0f4f Merge remote-tracking branch 'upstream/dev/emc' into dev/emc - 4bd9b9e clean up debug statements - 25ed5ef additions for stochy restarts - a2a374b add stochy_restart writing to mom_cap - 0c15f4c Update MOM_diabatic_driver.F90 - 167a62e Merge pull request #12 from pjpegion/dev/emc - bd477a9 Update MOM_diabatic_driver.F90 - 7212400 Update MOM_diabatic_driver.F90 - 7de295c cleanup of code and enhancement of ePBL perts - cd06356 Merge pull request #11 from NOAA-EMC/dev/emc - 9896d61 Merge pull request #9 from pjpegion/dev/emc_merge - 0a62737 Merge branch 'ocn_stoch' into dev/emc_merge - 3cad1ba Merge pull request #8 from NOAA-EMC/dev/emc - c2aa2a8 updates from dev/emc - 182ef34 additions for stochastic physics and ePBL perts - 671c714 Merge pull request #1 from NOAA-EMC/dev/emc
Candidate branch main->dev/gfdl
Revised the stochastics-related code to bring it into closer alignment with the MOM6 style guide at https://github.com/mom-ocean/MOM6/wiki/Code-style-guide, and splitting config_src/external/stochastic_physics/stochastic_physics.F90 into two files (one of which is the new file get_stochy_pattern.F90) to respect the MOM6 convention that there is only one module per file. Also renamed two stochastics-related optional arguments to ePBL_column to better reflect what they do, and corrected a number of spelling errors in comments in the files that were being modified. All answers and output are bitwise identical, but there are some minor internal interface changes.
Revised the external/drifters code to bring it into closer alignment with the MOM6 style guide at https://github.com/mom-ocean/MOM6/wiki/Code-style-guide. This includes using standard syntax to document variable units, the addition of 'implicit none ; private', explicit public statements, and a licensing statement to the files that were missing one, and modifications to follow the MOM6 2-point indenting convention. All answers are bitwise identical.
Revised the external/ODA_hooks code to bring it into closer alignment with the MOM6 style guide at https://github.com/mom-ocean/MOM6/wiki/Code-style-guide. This includes using standard syntax to document variable units, the addition of 'implicit none ; private', and modifications to follow the MOM6 2-point indenting convention. All answers are bitwise identical.
Corrected unbalanced indentation levels and impose MOM6-standard 2-point indentation in the config_src/driver codes, including both the FMS and mct caps and the solo_driver. The nuopc_cap is excluded from this commit because of the huge number (over 1000) of lines there that use irregular indentation. All answers are bitwise identical, and only white space changes are included.
Corrected unbalanced indentation levels and impose MOM6-standard 2-point indentation in the config_src/driver/nuopc_cap code. There were over 1000 lines that used non-standard or unbalanced indentation, so when reviewing this commit the use of the "ignore whitespace" option might be very useful. All answers are bitwise identical, and only white space changes are included.
Avoids using hard-coded dimensional tolerances in horizontal_regridding and improves the documentation of the variables and their units in this file. Without this change, calls to horiz_interp_and_extrap_tracer with a non-unity conversion factor will exhibit unexpected changes in answers, and may have wildly inappropriate amounts of smoothing of the interpolated values in arrays that are initialized from z-space file. However, the defaults are carefully chosen to avoid changing answers in any cases that do not use rescaling of tracer concentrations. The specific changes include: - Add a new optional argument tr_iter_tol to the horiz_interp_and_extrap_tracer routines to set an appropriate dimensional threshold for when the post-fill smoothing of tracers should be considered to be adequate. - Make the dimensional crit argument to fill_miss_2d non-optional, and fill it with appropriate values in calls from the horiz_interp_and_extrap_tracer routines when tr_iter_tol is not present. There should never be a hard-coded non-zero default for a dimensional variable! - Add a new optional scale argument to myStats - Eliminate the unused and unnecessary smooth logical argument to fill_miss_2d; the same functionality is obtained by setting the num_pass argument to 0. - Added to comments to document the units or unitlessness of the real variables in MOM_horizontal_regridding.F90. - Made minor changes to make the duplicated portions of the two horiz_interp_and_extrap_tracer similar. By default, all answers are bitwise identical, but this corrects a subtle bug with the use of the conversion factor argument in horiz_interp_and_extrap_tracer calls. There are new optional arguments to publicly visible routines.
Codecov Report
@@ Coverage Diff @@
## main #1569 +/- ##
==========================================
- Coverage 28.93% 28.81% -0.13%
==========================================
Files 242 249 +7
Lines 71606 72934 +1328
==========================================
+ Hits 20720 21015 +295
- Misses 50886 51919 +1033
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
kshedstrom
approved these changes
May 16, 2022
abozec
approved these changes
May 19, 2022
+Revise equation of state interfaces for consistency
|
@marshallward usually you will have a "...candidate" branch for us to test, but this time the PR is directly pointed to dev/gfdl and you just updated dev/gfdl several minutes ago. Shall we freeze to today's hash # in our testing or wait for further updating ? |
|
Oh wow, you are right. Thanks for noticing this @jiandewang. I will fix it. |
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.
This patch includes several new features, as well as the usual crop of bugfixes
and aggressive refactoring.
fluxes%shelf_sfc_mass_flux)New parameters
VELOCITY_REMAPPING_SCHEMEPARTIAL_CELL_VELOCITY_REMAPREMAP_VEL_MASK_BBL_THICKREMAP_VEL_MASK_H_THINDATA_OVERRIDE_SHELF_FLUXESTHICKNESS_TOLERANCEINTERNAL_TIDE_RESIDUAL_DRAGGME_NUM_SMOOTHINGSTRANS_FILEKH_ETA_CONSTKH_ETA_VEL_SCALEUSE_HYBGEN_UNMIXHYBGEN_MIN_THICKNESSHYBGEN_N_SIGMAHYBGEN_COORD_FILEHYBGEN_DEEP_DZ_PR0FILEHYBGEN_SHALLOW_DZ_PR0FILEHYBGEN_DEEP_DZ_VARHYBGEN_SHALLOW_DZ_VARHYBGEN_TGT_DENSITY_VARHYBGEN_ISOPYCNAL_DZ_MINHYBGEN_MIN_ISO_DEPTHHYBGEN_RELAX_PERIODHYBGEN_BBL_THICKNESSHYBGEN_REMAP_DENSITY_MATCHHYBGEN_REMAP_MAX_ZSTAR_DILATEODA_2018_ANSWERSNew diagnostics
deta_dtd[uv]d[xy]_btsfc_mass_fluxITide_tot_residual_losstransresidualDue to the removal of unused variables and whitespace cleanup, this PR is
expected to contain a very large number of changes in total.
Note that the large number of commits (~180) is due to re-introduction of
incorrecltly squashed commits from previous PRs. Hopefully this will get
everything back on track.
Features
Bugfixes and calculation improvements
Refactoring and testing
Contributors: