Feature/update to gfdl 20201022 #40
Conversation
- FMS2020 has newer cpu affinity work. These are mostly to fix the
issues with thread placing and hyperthreadng under slurm on gaea.
But it also works on Orion.
- The new affinity module simplifies the thread-placing calls in the
component models.
- The name of some functions has changed, that's the reason for crashes
like:
FATAL: input domain does not have an io_domain.
- This update fixes those issues.
- openmp runs with 1 and 2 threads gives the same answers as non-openmp
- NOTE: I don't rememer why we put the thread placing calls in MOM_domains.F90
They look as unnecessary and the whole #ifndef NOT_SET_AFFINITY block
can probably be removed. ocean_nthreads is either set in coupler
or solo_driver.
…_halo (*)Corrected halo size in EOS call if VERTEX_SHEAR=T
FMS affinity operations, often used in OpenMP directives, require explicit CPU affinities such that the number of available CPUs matches the number of requested PEs. To accommodate this, we explicit configure OpenMP to use cpu=0 for our ARM tests.
Removed old debugging code in blocks of code surrounded by #ifdef statements and removed unnecessary #ifdef around other blocks of debugging code. MOM6 standards discourage the use of CPP macros except for a limited set of uses related to memory where this is unavoidable, so this commit is bringing MOM6 closer to its stated standards. All answers and output are identical.
Modified doc_module so that new lines are added only when modules are documented, and are added in all parameter_doc files in which modules are documented. All answers and output are identical, but there are white space changes in MOM_parameter_doc and SIS_parameter_doc files.
Added the new optional argument like_default to the log_param and doc_param routines to help control where the documentation appears. This new argument is used for logging EPBL_USTAR_MIN, the diagnosed output value of MAXIMUM_DEPTH when the input value is negative, and the diagnosed number of columns where sponges occur with MOM_ALE_sponge. An '!' was also added to the logging of EPBL_USTAR_MIN. All answers are bitwise identical but there are minor changes in the contents of some MOM_parameter_doc.short files.
…nto add_dens_deriv_diag
Added diagnostics for partial derivatives of density
Removed #ifdef debugging blocks
Added code to determine whether all parameters in the MOM_grid, MOM_restart, MOM_write_cputime and MOM_tracer_registry modules are being used with their default settings, and added all_default arguments to the log_version calls for these modules. All answers and output are identical, but there are white space changes in MOM_parameter_doc.short and SIS_parameter_doc.short files.
Testing: Explicit OpenMP CPU affinity
- Members of a type cannot be individually labelled as shared/private - One variable was converted to shared since it was defiend in a non-OMP section and then labelled as private which meant it was uninitialized.
…A/MOM6 into Hallberg-NOAA-module_doc_newlines
Pointed out by @raphaeldussin
Typo fix
Autoconf-based build framework
Fix typo in README link
Markdown formatting fixes.
GM-Brankart update from NCAR
The COVERAGE variable was incorrectly quoted, and was causing incorrect quotes in the compile flag arguments. This patch removes the redundant quotes.
Testing: Remove quotes in COVERAGE
- The default for USE_NET_FW_ADJUSTMENT_SIGN_BUG had been changed to False but during the merge of main onto dev/gfdl the default was inadvertently flipped. @gustavo-marques pointed this out when reviewing NOAA-GFDL/MOM6#1211. - This affects EMC and NCAR versions of drivers and is not tested at GFDL.
…update-to-GFDL-20201022 this is corresponding to GFDL main 20201022 commit main-candidate-2020-09-18
|
Log from orion.pdf |
|
Instead of posting the log files, can you just point to the UFS-weather-model PR? I'm assuming there is a corresponding one? |
|
@JessicaMeixner-NOAA Are we doing 2 PRs on ufs-weather too? Given the slow commit queue on ufs-weather, couldn't the update to ufs-weather be done only once (after the new default setings are used). The update here is for the code; jiande can show the code is b4b if we switch the input settings. The input setting updates would go w/ the code update to ufs-weather. |
|
DELL log.pdf |
|
It will only be 1 commit to MOM6 but I think there should be 2 commits to ufs-weather-model -- one with the parameters being explicitly set from to maintain current answers, one to update parameters. l I guess can look in the PR follow links and see that the parameters are explicitly changed are: I guess if the parameters that are changed are explicitly enough documented one commit will have to work, but if a slow queue is a reason to always make massive commits... that's not really a good place to be in either. |
|
Thinking about this more, what I think is actually most important is that we can retain restart repro w/ the updated MOM6 and the new defaults for all resolutions. Shan and I have been using a specific cpld_restart branch to test this; I've made a new branch which points to the updated MOM6: cpld_restart_updatemom. @jiandewang can you use this branch to test for restart repro with the new defaults and the updated MOM6? You'll need to add that branch to your own ufs-weather fork and then change the MOM templates for the three resolutions. The original restart-repro issue and the branch description are in this ufs-weather issue. To test, use:
Let me know if you have any questions. Thanks. |
|
@DeniseWorthen I can make that test but I think the root cause is related to a halo bug which is not in this commit yet. Can you help me generate "rt.cpldrestart.conf" file based on my current file at /work/noaa/marine/Jiande.Wang/UFS-weather/JW-M6-up-025-only/JW-b4b-025-only/tests/rt.conf-JW, I think all you need is to delete some test case |
|
The original cpld_restart branch that Shan and I were using contained the vertex shear halo bug fix. We got restart repro w/ both frac and non-frac grid in this branch (using some changes to fv3). What I want to make sure is that we retain restart repro for frac_grid and non-frac grid when we update MOM6. You need to just checkout the branch I pointed you to (which contains the updated MOM6), change the templates to the new default ones you are proposing and run the rt.cpldrestart.conf with the command I gave you. Then do a cmp between the restart and the control directories at each resolution. |
|
@DeniseWorthen got restart repro w/ both frac and non-frac grid in this new MOM code |
* (*)Fixed dimensional inconsistency in P3M_functions Corrected dimensionally inconsistent expressions in P3M_functions.F90, notably in P3M_limiter and monotonize_cubic and a complete rewrite and simplification of is_cubic_monotonic. Also added comments documenting the units of all real variables in this module, and changed the code to use logical variables in place of integer "booleans", including in the return value from is_cubic_monotonic. These changes will change (fix) the answers when remapping variables with small numerical values, but no answers change in the MOM6-examples test cases. * +Added REMAPPING_2018 runtime option Added a new runtime option, REMAPPING_2018, which if set to false triggers the use of new, more accurate expressions in various parts of the ALE remapping code. By default, the older expressions are used, and all answers are bitwise identical, but there are new optional arguments to various routines related to remapping to trigger the use of new mathematically equivalent expressions. By default all answers are bitwise identical, but there are new and reordered entries in the MOM6_parameter_doc files. * Corrected the formatting of a doxygen comment * Added conversion factors to forcing diagnostics 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 correct scaling factors to chksum calls 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. * +Unscales area before taking global sum 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. * (*)Correct dimensionally inconsistent advective CFL 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 sea level before averaging 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 arg to global_i_mean 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. * Expand consistency testing with i-mean sponges 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.
Description of changes
update EMC MOM6 to GFDL 20201022 commit
Specific notes
Issues Fixed (include github issue #):
MOM6 Issue #39
ufs-weather-model Issue 283 (ufs-community/ufs-weather-model#283)
Are changes expected to change answers?
When using this new MOM6 code which contains some default value changes, the original results can be retained for 0.25x0.25 degree setting by explicitly adding several parameters in MOM_input_025, detail can be seen at https://github.com/jiandewang/ufs-weather-model/blob/feature/update-MOM6-retain-b4b-025/tests/parm/MOM_input_template_025#L12. However there are 4 variables being removed from restart file MOM.res-2.nc, so will fail in cmp but the common variables are b4b identical.
Testing performed:
Hashes used for testing
NEMS: 251d70c3a
CMEPS: efb8d353
FV3: 4fdf7c6f
CICE: ac617cde
FV3: 4fdf7c6f1
WW3: 9c22b135
stochastic_physics: 6eac8455
MOM6: aff9c2c (https://github.com/jiandewang/MOM6/tree/feature/update-to-GFDL-20201022)