-
Notifications
You must be signed in to change notification settings - Fork 159
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
New Thompson cloud fraction (updated subroutine cal_cldfra3) #432
New Thompson cloud fraction (updated subroutine cal_cldfra3) #432
Conversation
ccpp/driver/GFS_diagnostics.F90
Outdated
@@ -215,6 +287,46 @@ subroutine GFS_externaldiag_populate (ExtDiag, Model, Statein, Stateout, Sfcprop | |||
ExtDiag(idx)%data(nb)%var2 => IntDiag(nb)%ulwsfc(:) | |||
enddo | |||
|
|||
idx = idx + 1 | |||
ExtDiag(idx)%axes = 2 | |||
ExtDiag(idx)%name = 'rad_swdnt' |
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.
We are using "DSWRFI" for 'instantaneous surface downward shortwave flux' and "DSWRFtoa" for the averaged 'top of atmos downward shortwave flux'. Can we use "DSWRFItoa" as the field name for consistency?
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.
@gthompsnWRF wanted to use these names, but I agree that it would be more consistent to use the UFS-GFS names and not the WRF names. I'll change this.
ccpp/driver/GFS_diagnostics.F90
Outdated
|
||
idx = idx + 1 | ||
ExtDiag(idx)%axes = 2 | ||
ExtDiag(idx)%name = 'rad_swupt' |
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.
suggest to use "USWRFItoa"
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.
Sure.
ccpp/driver/GFS_diagnostics.F90
Outdated
|
||
idx = idx + 1 | ||
ExtDiag(idx)%axes = 2 | ||
ExtDiag(idx)%name = 'rad_lwupt' |
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.
suggest to use: "ULWRFItoa"
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.
Sure.
…ver/GFS_diagnostics.F90
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.
Looks good.
Someday in the "ideal future," someone should really do a job of unifying names to easier-to-find variables names for groups of variables in some logical manner. No naming schema is ever perfect, but the proliferation of shortened variable names is just so frustrating to anyone outside of the small group who are familiar with the 'legacy' names used. While we don't need 60 characters for most variables, our arcane variable names are too mismatched and disjointed. Feel free to move ahead how others wish. I just wanted to record the comment. If I was asked to review the PR, I would approve. |
Yes, absolutely. The CCPP standard names or another set of standard names would be better than these cryptic names (be it WRF or UFS names) that could all well serve as passwords ... I didn't ask you to review because I thought you were still on leave. |
Submodule pointer for ccpp-physics is correct (fb752d4), and |
Includes code changes described in NCAR/ccpp-physics#795 and NOAA-EMC/fv3atm#432: the addition of a new cloud fraction scheme for Thompson MP.
Description
This PR supports the changes in NCAR/ccpp-physics#781, which add a new method to calculate cloud fraction for Thompson MP. A few diagnostic variables are added for use by the cloud fraction schemes in CCPP, and the
icloud==3
switch is repurposed from currently being unused to switch between the originalprogcld6
and the newprogcld_thompson
scheme in CCPP.Issue(s) addressed
n/a
Testing
See ufs-community/ufs-weather-model#929
Dependencies