Sync from NCAR/main + Thompson params #309
Conversation
the end of subroutines, remove duplicate print statements
statements. More cleanup
"can not" to cannot.
errflg directly. On branch return-on-error
ufs-dev PR#287
…hu@noaa.gov, and Kwun Y. Fung).
equation of GFS PBL scheme. (from @WeiguoWang-NOAA)
ufs-dev PR279
…k that w3emc is using the expected precision
…k that w3emc is using the expected precision
UFS-dev PR#195
grantfirl
requested review from
rhaesung,
Qingfu-Liu,
dustinswales,
tanyasmirnova,
barlage,
cenlinhe,
HelinWei-NOAA,
JongilHan66,
joeolson42,
ZhanZhang-NOAA,
mjiacono,
gthompsnWRF,
RuiyuSun,
AndersJensen-NOAA,
MicroTed,
AnningCheng-NOAA,
andrewgettelman,
mdtoyNOAA,
BoYang-NOAA and
mzhangw
as code owners
| @@ -126,6 +133,22 @@ subroutine mp_thompson_init(ncol, nlev, con_pi, con_t0c, con_rv, & | |||
| lvap0 = con_hvap | |||
| lfus = con_hfus | |||
|
|
|||
| if (present(con_nt_c_l)) nt_c_l = con_nt_c_l | |||
| if (present(con_nt_c_o)) nt_c_o = con_nt_c_o | |||
| if (present(con_av_i)) then | |||
There was a problem hiding this comment.
Where is this con_av_i defined, in GFS_typedefs.F90? Is it initialized as 0 ?
There was a problem hiding this comment.
Ruiyu,
Since it is an optional, It is used only when larger than 0. However, I think it is not used at this moment.
There was a problem hiding this comment.
@RuiyuSun It is passed in as an optional argument. It is defined in GFS_typedefs as -999 and is set by namelist, although no namelists are set up to change it. For all UFS configurations, right now, con_av_i is present, but less than 0, so the functional form is used below. For NEPTUNE, they will pass in a value > 0 and therefore, it can be used as a constant, which is what they need. A couple of commits back, I tried to just have it as an optional argument without defining it in GFS_typedefs.F90, but apparently, the CCPP framework won't allow this, hence the need to define it, but set it as a nonsense/negative value.
There was a problem hiding this comment.
@dustinswales @climbfuji Optional argument experts: is it correct to say that for variables that are optional at the scheme level at least need to be declared and have metadata in every host? For example, I originally had the av_i argument in this PR as an optional argument with metadata in Thompson MP, but since I knew that UFS-based hosts wouldn't need it, I didn't even add a declaration or metadata in *_typedefs. The CCPP framework errored out because it expected the host to at least have these for cross-checking. Should the framework be able to skip these variables (that are optional in the scheme but not present in the host)?
There was a problem hiding this comment.
See d856677 for how I originally coded it.
There was a problem hiding this comment.
That's an interesting question. I agree that the idea of "if it's an optional argument and the host doesn't know, then simply don't provide it - i.e. never present" sounds good. One question would then be if that opens the door to unintended errors. But we need to first remind ourselves what capgen does in this case. Does it create the optional variable? Or does it ignore optional variables when it generates interstitial variables in the caps? @dustinswales do you know?
Description of Changes:
Changes from:
The issue: Some subroutines do not have return statements after an error occurs. Instead two variables errflg and errmsg are reset or re-initialized in subsequent calls, and if an error occurs it is not captured and the program continues without stopping. Please note that some calls already have if(errflg/=0) return in place.
This PR fixes an issue to return right after an error occurs. Subroutines that use errflg and errmsg should be checked for errflg and abort if necessary. Insert if(errflg/=0) return after such calls.
In addition the following is added:
Remove unnecessary return statements at the end of subroutines. In one case, there were actually two return statements right after each other.
Improve error messages and remove duplicate print statements if they are the same as the errmsg.
Change “can not” to “cannot” and other minor additions.
UPDATE: Thanks to reviewers' feedback, this PR also includes cleanup of GFS_phys_time_vary.fv3.F90 to remove copy_error calls from one-thread region (accessing errmsg and errflg directly).
NCAR#1152 description (issue):
The way w3emc is used in GFS_time_vary_pre is not safe, since there is no protection against argument mismatches in case the wrong version of the library (e.g., _4 instead of _d) is used. This came up during the debugging of a w3emc build issue with Intel oneAPI ifx.
A detailed description can be found here: JCSDA/spack-stack#1716
The issue also provides a code snippet that protects against using a wrong kind for the integer and real arguments in the w3difdat routines.
reviewer changes from UFS-dev PR#253 NCAR/ccpp-physics#1154 (parameterized types in canopy_utils_mod)
reviewer changes from UFS-dev PR#195 NCAR/ccpp-physics#1153 (remove dependence on MPI in mp_tempo_post, other minor naming changes)
bugfix for NSSL MP: bugfix: NaNs in module_mp_nssl_2mom.F90 NCAR/ccpp-physics#1157
Make some Thompson MP tuning parameters visible/tunable from host (needed by NRL NEPTUNE)
Tests Conducted:
UFS RTs on Ursa show no changes to baselines. Some testing was done at NRL with the NEPTUNE model that necessitated reversing some UFS-specific tuning commits. See NCAR#1158
Dependencies:
None
Documentation:
None
Issue (optional):
Fixes NCAR#1155
Contributors (optional):
@matusmartini @climbfuji