GFDL to main candidate branch (2022-05-16) #1570
Conversation
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.
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.
|
This time with the right branch (Thanks @jiandewang) Apologies to everyone, those who approved will need to re-approve it. |
Codecov Report
@@ Coverage Diff @@
## main #1570 +/- ##
==========================================
- Coverage 28.93% 28.78% -0.16%
==========================================
Files 242 249 +7
Lines 71606 72991 +1385
==========================================
+ Hits 20720 21010 +290
- Misses 50886 51981 +1095
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
|
This candidate is changing answers for some of our tests. We will need some time to investigate this further. The candidate merged to dev/ncar can be found here. There were a lot of conflicts that had to be solved manually. |
|
@gustavo-marques, if find that you are having trouble tracking down why the answers are changing, I may be able to help. If you could point us to a git repository with any simple test cases (i.e., not requiring access to huge data volumes or specific access to NCAR machines) where the answers are changing, I might have some time this week to help you track them down. Moreover, I periodically carry out additional tests beyond those in the standard GFDL pipeline tests, especially before committing some of the more widespread code changes. I would be happy to consider other test cases to add to this list. |
|
no answer change in UFS and N. Atlantic region setting. I am holding on approval at this moment as @gustavo-marques reported there is answer change in dev/ncar |
|
Thanks, @Hallberg-NOAA. Unfortunately, we do not have any simple test cases that work with the MCT or NUOPC caps. However, we could put together an ocean-only global_ALE version with the NCAR physics enabled. This is not ideal but could help in identifying changes in answers related to the choice of physics. |
|
@gustavo-marques, unless you have one ready, I have a tx0.66v1 standalone experiment in the following cheyenne directory. I haven't updated it in the last year or so, so you might want to update MOM_input and MOM_override to be able to reproduce the answer change. /glade/work/altuntas/mom6.standalone.runs/cesm/c.t061.common And here are the inputs: |
|
@abozec : following is one of the netcdf file header used by OBC run, it works fine in this PR code |
|
I was able to track down where answers start to diverge. It's in the checksum call for In addition to changing the ocean state, this PR also changes Both changes are correct and should be retained. However, I will check with the rest of the NCAR team on how to proceed before approving this PR. I will report back here before Friday EOD. |
|
We approve this PR. |
|
@sanAkel Are you or Matt able to review this PR? |
For some reason, I didn't get notified of this PR until now. Will get back in a few hours. Thanks! |
|
Everyone has approved this, and it seems like any answer changes are understood and there is a plan to resolve them, so I will now merge this. Thanks as always to everyone. Time to update our branches! |
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: