-
Notifications
You must be signed in to change notification settings - Fork 146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
miscellaneous changes from NRL #798
Conversation
@@ -66,6 +66,20 @@ foreach(typedef_module ${TYPEDEFS}) | |||
list(APPEND MODULES_F90 ${CMAKE_CURRENT_BINARY_DIR}/${typedef_module}) | |||
endforeach() | |||
|
|||
# This is a hack because I don't know how to get the pre_build to add these |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
specific to NRL (all current changes in CMakeLists.txt)
@@ -15,7 +15,8 @@ module GFS_rad_time_vary | |||
!> \section arg_table_GFS_rad_time_vary_timestep_init Argument Table | |||
!! \htmlinclude GFS_rad_time_vary_timestep_init.html | |||
!! | |||
subroutine GFS_rad_time_vary_timestep_init ( & | |||
subroutine GFS_rad_time_vary_timestep_init (nthrds, blksz, lrseeds, & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All changes in this file seem specific to NRL to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these changes make the random number generator _not specific to FV3. The random number generation has always been grid specific and this generalizes it.
@@ -7,6 +7,34 @@ | |||
[ccpp-arg-table] | |||
name = GFS_rad_time_vary_timestep_init | |||
type = scheme | |||
[nthrds] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All changes in this file correspond to NRL-specific changes in the corresponding Fortran.
@@ -16,7 +16,7 @@ end subroutine GFS_rrtmg_pre_init | |||
!! | |||
! Attention - the output arguments lm, im, lmk, lmp must not be set | |||
! in the CCPP version - they are defined in the interstitial_create routine | |||
subroutine GFS_rrtmg_pre_run (im, levs, lm, lmk, lmp, n_var_lndp, & | |||
subroutine GFS_rrtmg_pre_run (im, levs, lm, lmk, lmp, lextop, ltp, n_var_lndp, & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the changes in this file related to a bugfix or are they specific to NRL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these may be NRL/NEPTUNE-specific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think they are bug fixes when using lextop=.true. @matusmartini can comment more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember those, yes. It would be really good to get the lextop
bugfixes in. The UFS doesn't use the capability and there are no regression tests that cover it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we use this capabilty with success for several suites and maintain it so it does not break our warm restart tests. I added a capability to have lextop as a namelist control flag so it can be turned on/off.
@@ -46,7 +44,7 @@ subroutine GFS_rrtmg_setup_init ( & | |||
si, levr, ictm, isol, ico2, iaer, ntcw, & | |||
num_p3d, npdf3d, ntoz, iovr, isubc_sw, isubc_lw, & | |||
icliq_sw, crick_proof, ccnorm, & | |||
imp_physics, & | |||
imp_physics, ltp, & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the changes in this file related to a bugfix or are they specific to NRL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am checking with our developers. They appear to be NRL-specific, but I'll let you know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are related to using lextop=.true.
@@ -209,6 +209,7 @@ subroutine gcycle (me, nthrds, nx, ny, isc, jsc, nsst, tile_num, nlunit, | |||
! | |||
enddo | |||
! | |||
#if 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
specific to NRL (all changes in this file)
@@ -57,7 +57,7 @@ module module_iounitdef ! | |||
integer, parameter :: NISIGI2 = 12 | |||
integer, parameter :: NISFCI = 14 | |||
integer, parameter :: NICO2TR = 15 | |||
integer, parameter :: NICO2CN = 102 | |||
integer, parameter :: NICO2CN = 113 ! CCE (Cray) forbids 100-102 20211112 JM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all changes in this file specific to NRL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change at line 60 is not NRL-specific. You will run into this problem with Fortran unit numbers when you compile with the Cray CCE compiler.
@@ -950,7 +950,7 @@ SUBROUTINE mym_length ( & | |||
alp2 = 0.65 | |||
alp3 = 3.0 | |||
alp4 = 20. | |||
alp5 = 0.4 | |||
alp5 = 1.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all changes in this file specific to NRL or should they be merged? @michalakes @joeolson42
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change on line 1066 is a bug fix. Communicated this to @joeolson42 and he concurred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like an older version of the MYNN. Almost all of these parameters have been changed since we spoke about this. Also, some free-atmospheric testing against LES has happened since then and the results show improvements with alp5 decreased, not increased. Here's an example:
We may need to add a new mixing length option if you really want these values added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we want a different option in there for parameters (however, it would be nice if these were controlled through a namelist since they will be model dependent). The only thing that I was making a point about was the bug on line 1066 that was communicated via email on 19 Feb 2021 and response on 22 Feb confirming that it was a bug.
@@ -1318,7 +1318,7 @@ SUBROUTINE mp_gt_driver(qv, qc, qr, qi, qs, qg, ni, nr, nc, & | |||
m = RSHIFT(ABS(rand_perturb_on),1) | |||
if (MOD(m,2) .ne. 0) rand2 = rand_pert(i,1,j)*2. | |||
m = RSHIFT(ABS(rand_perturb_on),2) | |||
if (MOD(m,2) .ne. 0) rand3 = 0.25*(rand_pert(i,1,j)+ABS(min_rand)) | |||
if (MOD(m,2) .ne. 0) rand3 = 0.25*(rand_pert(i,1,j)+ABS(min_rand)) !TODO min_rand used before set 2021112 JM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commenting out write statements specific to NRL?
@@ -35,7 +35,7 @@ | |||
!! constants for GCM models. | |||
module physcons | |||
! | |||
use machine, only: kind_phys, kind_dyn | |||
use machine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really want to remove specificity of what we need from the machine module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember that the only cluse was causing a compile-time issue, but I would need to go back to recover the details.
@@ -1616,6 +1616,13 @@ subroutine samfdeepcnv_run (im,km,itc,ntc,cliq,cp,cvap, & | |||
ktcon1(i) = k | |||
flg(i) = .false. | |||
endif | |||
!NRL MNM: Limit overshooting not to be deeper than the actual cloud |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes in this file are specific to NRL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As explained for sascnvn.F in my comment below, this is to prevent potential negative humidity in rare instances.
@@ -1008,6 +1008,13 @@ subroutine sascnvn_run( | |||
ktcon1(i) = k | |||
flg(i) = .false. | |||
endif | |||
!NRL MNM: Limit overshooting not to be deeper than the actual cloud |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes in this file are specific to NRL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a while back but if I remember correctly, it was to prevent negative QV resulting from overly deep overshooting mostly for relatively shallow updrafts with extremely deep overshoots. This change was before the parameter aafac was reduced from 10% to 5%, which helped but did not eliminate all of the negative values.
Digging through my notes with examples of updrafts with negative QV concentrations:
Cloud base at 1.4 km above ground, level of neutral buoyancy ~3.5 km, with overshooting top is at 18 km!
More points found with cloud base at ~0.6 km, level of neutral buoyancy ~2.5 km, with overshooting top at ~16 km!
The reason for the negative QV’s because of the approximations made in SAS:
First, the moisture content of the rising parcel if it was saturated (qrch) is approximated as the sum of saturated moisture (qeso) and a term that is directly proportional to buoyancy. Generally, overshooting layers are negatively buoyant, and so the sum of the two can < 0 if large buoyancies are present.
Second, the convective overshooting level is estimated as the level where (0.1 * A_ktcon) < 0, where A_ktcon is the cloud work function at the level of neutral buoyancy (ktcon). The 0.1 is a tunable parameter (aafac) independent of the first approximation.
Moreover, there is no check that would stop this from happening in the overshooting layers. There is only a check on negative cloud mass flux (eta) in imfdeepcnv=2, but it only scans levels below the level of neutral buoyancy. There should strictly be a check for negative qrch also in overshooting layers in both options of imfdeepcnv 1 and 2. After playing around with both options 1 and 2 of SAS convection, I see several tuning knobs that can result in increased number of updrafts with negative qrch (all of them near the overshooting cloud top). The most important tuning parameter is the critical thickness of the calculated convection (cthk), which determines whether the convection is deep enough, otherwise it is inhibited. In option 1, this threshold is set to 150 hPa. In option 2, this threshold is increased to 200 hPa. Doing an experiment with the cthk increased to 200 hPa in option 1, eliminates most of the negatively moist parcels. And vice versa, decreasing cthk to 150 hPa in option 2, introduces a few negatively moist parcels. Similar result can be achieved by tuning the aafac parameter.
@@ -55,24 +60,50 @@ subroutine GFS_rad_time_vary_timestep_init ( | |||
|
|||
if (lsswr .or. lslwr) then | |||
|
|||
!--- call to GFS_radupdate_timestep_init is now in GFS_rrtmg_setup_timestep_init | |||
nblks = size(blksz) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This update and those below that allow the seeds to be supplied by the host model appear to be NRL/NEPTUNE-specific. These for NEPTUNE, but are there any issues for other codes? (I'm not sure how to determine that other than asking other groups to review.)
@@ -37,11 +37,10 @@ subroutine GFS_rrtmg_pre_run (im, levs, lm, lmk, lmp, n_var_lndp, & | |||
clouds9, cldsa, cldfra, faersw1, faersw2, faersw3, faerlw1, faerlw2, & | |||
faerlw3, alpha, errmsg, errflg) | |||
|
|||
use machine, only: kind_phys | |||
use machine, only: kind_phys, r8=>kind_dbl_prec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the definition of r8. Not used in this routine. Was probably added during debugging and forgot to remove.
@@ -16,7 +16,7 @@ end subroutine GFS_rrtmg_pre_init | |||
!! | |||
! Attention - the output arguments lm, im, lmk, lmp must not be set | |||
! in the CCPP version - they are defined in the interstitial_create routine | |||
subroutine GFS_rrtmg_pre_run (im, levs, lm, lmk, lmp, n_var_lndp, & | |||
subroutine GFS_rrtmg_pre_run (im, levs, lm, lmk, lmp, lextop, ltp, n_var_lndp, & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these may be NRL/NEPTUNE-specific.
@@ -46,7 +44,7 @@ subroutine GFS_rrtmg_setup_init ( & | |||
si, levr, ictm, isol, ico2, iaer, ntcw, & | |||
num_p3d, npdf3d, ntoz, iovr, isubc_sw, isubc_lw, & | |||
icliq_sw, crick_proof, ccnorm, & | |||
imp_physics, & | |||
imp_physics, ltp, & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am checking with our developers. They appear to be NRL-specific, but I'll let you know.
@@ -57,7 +57,7 @@ module module_iounitdef ! | |||
integer, parameter :: NISIGI2 = 12 | |||
integer, parameter :: NISFCI = 14 | |||
integer, parameter :: NICO2TR = 15 | |||
integer, parameter :: NICO2CN = 102 | |||
integer, parameter :: NICO2CN = 113 ! CCE (Cray) forbids 100-102 20211112 JM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change at line 60 is not NRL-specific. You will run into this problem with Fortran unit numbers when you compile with the Cray CCE compiler.
@@ -35,7 +35,7 @@ | |||
!! constants for GCM models. | |||
module physcons | |||
! | |||
use machine, only: kind_phys, kind_dyn | |||
use machine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember that the only cluse was causing a compile-time issue, but I would need to go back to recover the details.
1cebb6b
to
06708b4
Compare
This is closed in favor of an up-to-date version (#952) that only has the changes not already merged and/or still needed |
DRAFT
TODO: Which of these changes should get merged into main?