ufs-dev PR#337#1212
Conversation
This reverts commit 565d62c.
Update for Explicit Subcanopy Effects on Chemical Tracers in the PBL-SATMEDMF Scheme
scrasmussen
requested review from
climbfuji,
dustinswales,
grantfirl and
rhaesung
as code owners
|
I'll wait until #1211 is merged before I review this. |
grantfirl
left a comment
grantfirl
left a comment
There was a problem hiding this comment.
Changes are identical except for whitespace changes in GFS_rrtmgp_setup.F90
@climbfuji This is ready for review now. |
climbfuji
left a comment
climbfuji
left a comment
There was a problem hiding this comment.
Great cleanup. @matusmartini Do you want to take a look at the changes to radiation_clouds.f ?
|
Note. ufs-community/ufs-weather-model#3042 did not require new baselines, everything was b4b identical. |
| ! ! Parameters | ||
| ! real(kind_phys) :: & | ||
| ! lambda = 0.50 ! , & | ||
| ! P = 0.25 |
There was a problem hiding this comment.
| ! ! Parameters | |
| ! real(kind_phys) :: & | |
| ! lambda = 0.50 ! , & | |
| ! P = 0.25 |
There was a problem hiding this comment.
Are these comment lines needed?
There was a problem hiding this comment.
I don't think so. lambda is now a namelist parameter that is passed in. I think Qingfu decided to keep this here temporarily as a reminder? We could remove it if you feel strongly about it.
There was a problem hiding this comment.
P is effectively hard-coded because of the sqrt(sqrt()) in the code now, so I don't know why it is still commented here. Maybe as a reminder that P (presumably what it is called in the paper) can be tuned to something other than 0.25, in which case the code would need to be changed back to use X**p.
matusmartini
left a comment
matusmartini
left a comment
There was a problem hiding this comment.
Looks like a great cleanup. Thank you!
| ! ! Parameters | ||
| ! real(kind_phys) :: & | ||
| ! lambda = 0.50 ! , & | ||
| ! P = 0.25 |
There was a problem hiding this comment.
Are these comment lines needed?
| ! to be cleaned up. The convection schemes doing something different internally | ||
| ! here, because it fixes the convective transportable_tracers mess for | ||
| ! GFDL MP from GFS_suite_interstitial_3. This whole code around clw(:,:,2) | ||
| ! being set to -999 for GFDL-MP |
There was a problem hiding this comment.
This comment is confusing (and an incomplete sentence), but perhaps still trying to hint that the "code needs to be cleaned up". The clw(:,:,2) here is set to 0 if it is <= -999, not just for GFDL-MP.
I addition, it sounds like the whole loop below:
do k=1,levs
do i=1,im
if (clw(i,k,2) <= -999.0) clw(i,k,2) = 0.0
enddo
enddo
may not be necessary since it is not set to -999 anymore (no Zhao-Carr)?
There was a problem hiding this comment.
This is really old! I might have added this, but it looks like it got chopped off. I agree that we should check if -999 is still expected for GFDL-MP. But I would create an issue from this discussion and address it as a follow up.
There was a problem hiding this comment.
clw(:,:,2) is set to -999.9 in CCPP_typedefs.F90 before every physics timestep. Unfortunately, it looks like it is still being used kinda like a flag to denote whether cloud ice is present as a tracer in some deep convection schemes. As far as I can tell, it only really needs to be set back to 0 here for the MG micro scheme, which isn't widely used, AFAIK.
All of this to say, this could certainly be cleaned up in a followup PR. I don't think we need to bother reinstating the comment.
Co-authored-by: Matus Martini <matusmartini@users.noreply.github.com>
5 participants
Description of Changes:
Bringing in changes from the following PRs:
Tests Conducted:
Built with SCM, regression tests in NCAR/ccpp-scm#683. Also see ufs-community/ufs-weather-model#3042
Dependencies: