Candidate for main from GFDL 2020-09-18 #1211
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
Corrected halo size in density derivative calculations in smoothed_dRdT_dRdS This fixes an i-parallelization problem that was recently introduced (as a part of MOM6 PR#1089) when VERTEX_SHEAR is True, and closes MOM6 issue #1146. All answers in the existing MOM6-examples test suite are bitwise identical, but this does change (correct) answers when VERTEX_SHEAR is true.
- 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.
(*)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
|
I approve this PR. |
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
|
Following @gustavo-marques realization yesterday, I pushed the corresponding branch "main-candidate-2020-09-18" to MOM6-examples. |
|
EMC will follow the changing of those default values, works fine in EMC coupled system |
|
@gustavo-marques What's your status on this PR? |
|
@alperaltuntas and I are working on it. Cheyenne was down for maintenance over the last two days. We should be able to finish handling it within the next day or two. |
| @@ -1095,7 +1095,7 @@ subroutine surface_forcing_init(Time, G, US, param_file, diag, CS, restore_salt, | |||
| call get_param(param_file, mdl, "USE_NET_FW_ADJUSTMENT_SIGN_BUG", & | |||
| CS%use_net_FW_adjustment_sign_bug, & | |||
| "If true, use the wrong sign for the adjustment to "//& | |||
| "the net fresh-water.", default=.false.) | |||
| "the net fresh-water.", default=.true.) | |||
There was a problem hiding this comment.
The default for this parameter did not have to be changed. We (NCAR) had already set this to False and, unintentionally, @Hallberg-NOAA set it back to True in NOAA-GFDL#1127
There was a problem hiding this comment.
Addressed in 3871073
| @@ -1094,7 +1094,7 @@ subroutine surface_forcing_init(Time, G, US, param_file, diag, CS, restore_salt, | |||
| call get_param(param_file, mdl, "USE_NET_FW_ADJUSTMENT_SIGN_BUG", & | |||
| CS%use_net_FW_adjustment_sign_bug, & | |||
| "If true, use the wrong sign for the adjustment to "//& | |||
| "the net fresh-water.", default=.false.) | |||
| "the net fresh-water.", default=.true.) | |||
There was a problem hiding this comment.
Same here. The default should be False.
There was a problem hiding this comment.
Addressed in 3871073
|
We were able to recover previous answers after manually setting |
|
@gustavo-marques Thanks for the above. I'll update this branch shortly. |
- 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.
marshallward
referenced
this pull request
Oct 21, 2020
- 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.
|
Thanks for making these changes. We now approve this PR. |
|
I approve this PR. |
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 is a PR onto our collective main branch. Note we now use the terminology "main" where we once used "dev/master". Instructions on renaming your branches can be found at https://github.com/NOAA-GFDL/MOM6/wiki/Renaming-the-main-branch-in-MOM6.
This PR has some potentially major-impact changes which are detailed below. Each in isolation made sense but unfortunately we let them stack up so may take some work to sort through - our apologies!
pkg/has been replaced by the use of null modules inconfig_src/external. This means the search path for code at compile time must be updated.I've classified the remaining PRs below which we expect should not affect you adversely.
As usual, please can @jiandewang, @awallcraft, @kshedstrom, and @gustavo-marques review the PR.
Bug fixes:
Testing:
Enhancements: