-
Notifications
You must be signed in to change notification settings - Fork 151
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
feature/transition-to-capgen-1: remove DDT dependency in radiation physics, correct units #506
feature/transition-to-capgen-1: remove DDT dependency in radiation physics, correct units #506
Conversation
…M), GFS_rrtmg_post
…into ddt_removal_radiation
rog, rocp, con_rd, xlat_d, xlat, xlon, coslat, sinlat, tsfc, slmsk, & | ||
prsi, prsl, prslk, tgrs, sfc_wts, phy_f3d_mg_cld, phy_f3d_reffr, & | ||
phy_f3d_cnvw, phy_f3d_cnvc, qgrs, aer_nm, & !inputs from here and above | ||
coszen, coszdg, phy_f3d_leffr, phy_f3d_ieffr, phy_f3d_seffr, & |
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.
@climbfuji Shall I fix the phy_f3d naming stuff and push a PR to your branch?
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.
It looks like only dropping phy_f3d from the names might be confusing for some variables. How about the following? It looks like these variables are mainly used to shuttle data into local copies for calculation, with the exception of the inout ones that are used directly. So, appending intents might make sense? I'm open to other/better suggestions too.
phy_f3d_mg_cld = mg_cld
phy_f3d_reffr = effrr_in
phy_f3d_cnvw = cnvw_in
phy_f3d_cnvc = cnvc_in
phy_f3d_leffr = effrl_inout
phy_f3d_ieffr = effri_inout
phy_f3d_seffr = effrs_inout
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 should have done that yesterday afternoon before running the tests, I forgot to follow up. Let's do the following: Merge as is, change the names when creating the PRs from transition-to-capgen-1 to the authoritative branches. Which names would you like me to use?
…
On Oct 7, 2020, at 9:56 AM, grantfirl @.***> wrote: @grantfirl commented on this pull request. In physics/GFS_rrtmg_pre.F90 <#506 (comment)>: > - clouds1, clouds2, clouds3, clouds4, clouds5, clouds6, & - clouds7, clouds8, clouds9, cldsa, cldfra, & - mtopa, mbota, de_lgth, alpha, alb1d, errmsg, errflg) + subroutine GFS_rrtmg_pre_run (im, levs, lm, lmk, lmp, n_var_lndp, & + imfdeepcnv, imfdeepcnv_gf, me, ncnd, ntrac, num_p3d, npdf3d, ncnvcld3d,& + ntqv, ntcw,ntiw, ntlnc, ntinc, ncld, ntrw, ntsw, ntgl, ntwa, ntoz, & + ntclamt, nleffr, nieffr, nseffr, lndp_type, kdt, imp_physics, & + imp_physics_thompson, imp_physics_gfdl, imp_physics_zhao_carr, & + imp_physics_zhao_carr_pdf, imp_physics_mg, imp_physics_wsm6, & + imp_physics_fer_hires, julian, yearlen, lndp_var_list, lsswr, lslwr, & + ltaerosol, lgfdlmprad, uni_cld, effr_in, do_mynnedmf, lmfshal, & + lmfdeep2, fhswr, fhlwr, solhr, sup, eps, epsm1, fvirt, & + rog, rocp, con_rd, xlat_d, xlat, xlon, coslat, sinlat, tsfc, slmsk, & + prsi, prsl, prslk, tgrs, sfc_wts, phy_f3d_mg_cld, phy_f3d_reffr, & + phy_f3d_cnvw, phy_f3d_cnvc, qgrs, aer_nm, & !inputs from here and above + coszen, coszdg, phy_f3d_leffr, phy_f3d_ieffr, phy_f3d_seffr, & @climbfuji https://github.com/climbfuji Shall I fix the phy_f3d naming stuff and push a PR to your branch? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#506 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5C2RLTFRMGEWIFCI2HLYLSJSFTVANCNFSM4SGRKGRA.I put some suggestions in the comment in the file. I'm not sure it's the best, but it shouldn't really matter a whole lot what they're called locally.
Thanks, I agree with your suggestion.
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 tedious DDT cleanup is amazing. Great job!
We should have done that yesterday afternoon before running the tests, I forgot to follow up. Let's do the following: Merge as is, change the names when creating the PRs from transition-to-capgen-1 to the authoritative branches. Which names would you like me to use?
… On Oct 7, 2020, at 9:56 AM, grantfirl ***@***.***> wrote:
@grantfirl commented on this pull request.
In physics/GFS_rrtmg_pre.F90 <#506 (comment)>:
> - clouds1, clouds2, clouds3, clouds4, clouds5, clouds6, &
- clouds7, clouds8, clouds9, cldsa, cldfra, &
- mtopa, mbota, de_lgth, alpha, alb1d, errmsg, errflg)
+ subroutine GFS_rrtmg_pre_run (im, levs, lm, lmk, lmp, n_var_lndp, &
+ imfdeepcnv, imfdeepcnv_gf, me, ncnd, ntrac, num_p3d, npdf3d, ncnvcld3d,&
+ ntqv, ntcw,ntiw, ntlnc, ntinc, ncld, ntrw, ntsw, ntgl, ntwa, ntoz, &
+ ntclamt, nleffr, nieffr, nseffr, lndp_type, kdt, imp_physics, &
+ imp_physics_thompson, imp_physics_gfdl, imp_physics_zhao_carr, &
+ imp_physics_zhao_carr_pdf, imp_physics_mg, imp_physics_wsm6, &
+ imp_physics_fer_hires, julian, yearlen, lndp_var_list, lsswr, lslwr, &
+ ltaerosol, lgfdlmprad, uni_cld, effr_in, do_mynnedmf, lmfshal, &
+ lmfdeep2, fhswr, fhlwr, solhr, sup, eps, epsm1, fvirt, &
+ rog, rocp, con_rd, xlat_d, xlat, xlon, coslat, sinlat, tsfc, slmsk, &
+ prsi, prsl, prslk, tgrs, sfc_wts, phy_f3d_mg_cld, phy_f3d_reffr, &
+ phy_f3d_cnvw, phy_f3d_cnvc, qgrs, aer_nm, & !inputs from here and above
+ coszen, coszdg, phy_f3d_leffr, phy_f3d_ieffr, phy_f3d_seffr, &
@climbfuji <https://github.com/climbfuji> Shall I fix the phy_f3d naming stuff and push a PR to your branch?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#506 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5C2RLTFRMGEWIFCI2HLYLSJSFTVANCNFSM4SGRKGRA>.
|
I put some suggestions in the comment in the file. I'm not sure it's the best, but it shouldn't really matter a whole lot what they're called locally. |
standard_name = in_number_concentration | ||
long_name = IN number concentration | ||
units = kg-1? | ||
standard_name = ice_nucleation_number |
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 that this is better, but it can be resolved in the CCPP standard name cleanup that Ligia and I are doing.
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. Thanks.
This PR contains
updated with the latest code in feature/transition-to-capgen-1
Associated PRs:
#506
NCAR/ccpp-framework#327
NCAR/fv3atm#64
NCAR/ufs-weather-model#65
For regression testing information, see NCAR/ufs-weather-model#65.