Minor modification and bug fix in GFS cumulus convection schemes #885
Conversation
| @@ -1993,6 +1993,9 @@ module GFS_typedefs | |||
| real (kind=kind_phys), pointer :: tsnowpb(:) => null() !< accumulated surface snowfall in bucket (m) | |||
| real (kind=kind_phys), pointer :: rhonewsn1(:) => null() !< precipitation ice density outside RUC LSM (kg/m3) | |||
|
|
|||
| !--- TKE used by convection schemes | |||
| real (kind=kind_phys), pointer :: tkeh(:,:) => null() !< vertical turbulent kinetic energy (m2/s2) at the model layer interfaces | |||
There was a problem hiding this comment.
@rhaesung I'm wondering why the Diag DDT was chosen to house this variable. Isn't it basically a variable that needs to get passed from PBL to deep convection and not retained from one timestep to the next? If so, it probably belongs in the CCPP_typedefs/Interstitial DDT. I doubt that this is causing the reproducibility problem, but all variables held in the Diag datatype are subject to clearing on the bucket interval (nszero). This isn't necessary because it is defined as intent(out) in satmedmfvdifq.F90 and overwritten every timestep anyway.
There was a problem hiding this comment.
@JongilHan66 Could you help address this?
| standard_name = vertical_turbulent_kinetic_energy_at_interface | ||
| long_name = vertical turbulent kinetic energy at model layer interfaces | ||
| units = m2 s-2 | ||
| dimensions = (horizontal_loop_extent,vertical_layer_dimension) |
There was a problem hiding this comment.
@rhaesung There is a mismatch between the standard name and the vertical dimension. Variables defined on the interfaces between model layers should use the standard name of vertical_interface_dimension.
| @@ -7952,6 +7955,9 @@ subroutine diag_create (Diag, Model) | |||
| allocate (Diag%refl_10cm(IM,Model%levs)) | |||
| allocate (Diag%max_hail_diam_sfc(IM)) | |||
|
|
|||
| !--- Vertical turbulent kinetic energy at model layer interfaces | |||
| allocate (Diag%tkeh(IM,Model%levs)) | |||
There was a problem hiding this comment.
Shouldn't this be Model%levs+1 if it is truly defined on the interfaces?
There was a problem hiding this comment.
@grantfirl @rhaesung Although tkeh is tke at model interfaces, I think it can has vertical dimension of levs rather than levs+1 because it's vertical dimension is defined as "vertical_layer_dimension" consistently in all relevant meta files (i.e., GFS_typedefs.meta, satmedmfvdifq.meta, samfdeepcnv.meta, samfshalcnv.meta). Am I wrong? If so, Rhaesung, please change "vertical_layer_dimension" to "vertical_interface_dimension" for tkeh in all the meta files and also change "Diag%tkeh(IM,Model%levs)" to "Diag%tkeh(IM,Model%levs+1)" in GFS_typedefs.F90. Once the prognostic tke is updated every time step, then tkeh is computed as tkeh=0.5*(tke(k)+tke(k+1)) up to k=levs-1 every time step in satmedmfvdifq.F for use in samfdeepcnv.f and samfshalcnv.f.
There was a problem hiding this comment.
@JongilHan66 You're right that the consistency of naming across all files is correct, but I think that it is at the very least confusing to have a variable intended for use on interfaces to be given the vertical dimension corresponding to the model layer centers. If you look at other variables defined on interfaces, they're given the vertical dimension of vertical_interface_dimension. I'm just looking at the changes in this PR to try to help debug the unreproducible regression tests, and I think that uninitialized sections of arrays have the tendency to create unreproducible code. When tkeh is calculated in satmedmfvdifq.F, I think that it would make sense to make sure that the entire array is given a value, not just up to k=levs-1, just in case that the array is ever accessed outside levs-1 in subsequent schemes. Even setting the entire tkeh array to 0 before doing the loop from k=1 to levs-1 would fix the potential problem.
There was a problem hiding this comment.
@grantfirl @rhaesung Grant, tkeh is initialized to be zero in GFS_typedefs.F90 as "Diag%tkeh = zero". Do we still need to initialize in satmedmfvdifq.F?
There was a problem hiding this comment.
@rhaesung Rhaesung, could you also set tkeh = 0. before doing the loop from k=1 to levs-1 in satmedmfvdifq.F and test it if it can fix the problem?
There was a problem hiding this comment.
Yes, because it is intent(out) in satmedmfvdifq. Any time a variable is given intent(out) in a scheme, it is (potentially) given garbage values. Plus, variables in the Diag DDT are cleared out and reinitialized every nszero timesteps, which is unnecessary. Another "fix" would be to just make tkeh intent(inout). That way, the initialization within diag_phys_zero would at least be kept, although I'd argue it's still being held in the wrong DDT for this application. It should be in CCPP_typedefs.F90/Interstitial DDT that get reinitialized every physics timestep.
There was a problem hiding this comment.
@grantfirl @rhaesung Thanks, Grant for the explanation!!
Rhaesung, before you try to change from "vertical_layer_dimension" to "vertical_interface_dimension",
please do these tests in order as:
- make tkeh intent(inout) in all the 4 meta files
- set tkeh=0 before doing the loop from k=1 to levs-1 in satmedmfvdifq.F
There was a problem hiding this comment.
@JongilHan66 Sure, I'll do that and report back to you.
There was a problem hiding this comment.
@JongilHan66 @grantfirl Unfortunately, neither of them is working to resolve the issue. I’ll determine if the issue is actually related to tkeh and try to follow Grant's suggestion.
…nto conv_hr5update
Description
This PR mainly involves changes in ccpp-physics, and the code was provided by @JongilHan66.
Modified prognostic updraft fraction (sigmab) calculation in 'progsigma_calc.f90' which is physically more sound:
a) moisture convergence calculation: integrate from the convection source level rather than from the cloud base
b) 2D advection of sigmab: sigmab advection averaged over the cloud layers rather than taking a maximum sigmab advection from k=2 to the model top
c) To suppress unrealistically large reflectivity in the model first time step, minimum sigmab at the first time step is set to zero
Fix in missing vertical transport of turbulent kinetic energy (TKE) when aerosol transport is turned on (samfdeepcnv.f & samfshalcnv.f)
Introduce TKE at model layer interfaces (tkeh) for use in convection schemes (GFS_typedefs.F90, GFS_typedefs.meta, satmedmfvdifq.F, satmedmfvdifq.meta, samfdeepcnv.f, samfdeepcnv.meta, samfshalcnv.f, and samfshalcnv.meta)
Vertical transports of hydrometeor variables are currently not allowed in the convection schemes. But vertical transports of number concentrations of only cloud water and ice are mistakenly allowed, which is fixed in this update (CCPP_typedefs.F90)
All the modifications and bug fixes above had neutral impacts on the GFS forecasts
Dependencies
If testing this branch requires non-default branches in other repositories, list them.
Those branches should have matching names (ideally)
Do PRs in upstream repositories need to be merged first?
If so add the "waiting for other repos" label and list the upstream PRs