Tempo microphysics to CCPP #245
Conversation
… into feature/ccpp_implementation
Feature/ccpp implementation
… into feature/tempo_mp
AndersJensen-NOAA
requested review from
grantfirl,
rhaesung,
Qingfu-Liu,
dustinswales and
mzhangw
as code owners
| @@ -763,9 +789,19 @@ subroutine GFS_rrtmg_pre_run (im, levs, lm, lmk, lmp, n_var_lndp, lextop,& | |||
| qi_mp (i,k) = tracer1(i,k,ntiw)/(1.-qvs) | |||
| qs_mp (i,k) = tracer1(i,k,ntsw)/(1.-qvs) | |||
| if(nint(slmsk(i)) == 1) then | |||
| nc_mp (i,k) = Nt_c_l*orho(i,k) | |||
| if (imp_physics == imp_physics_thompson) then | |||
There was a problem hiding this comment.
I'm concerned about the addition of 'if' statements within the spatial loops for performance reasons. As an example for this section, couldn't one keep a Nt_c_l (generic) and set it from Nt_c_l_thmpsn/tempo as needed above the loop?
| nc_mp(i,k) = make_DropletNumber(qc_mp(i,k)*rho(i,k), nwfa(i,k)*rho(i,k)) * orho(i,k) | ||
| if ((ltaerosol .or. mraerosol) .and. qc_mp(i,k)>1.e-12 .and. nc_mp(i,k)<100.) then | ||
| if (imp_physics == imp_physics_thompson) then | ||
| nc_mp(i,k) = make_DropletNumber_thmpsn(qc_mp(i,k)*rho(i,k), nwfa(i,k)*rho(i,k)) * orho(i,k) |
There was a problem hiding this comment.
It's harder to make the same change when calling a subroutine like this, although you could still pull the 'if imp_physics' statement out and have multiple loop sections. There's already one 'if' statement being executed inside the loop, but still.
| use module_mp_thompson, only: calc_effectRad, Nt_c_l, Nt_c_o, re_qc_min, re_qc_max, & | ||
| re_qi_min, re_qi_max, re_qs_min, re_qs_max | ||
| use module_mp_thompson_make_number_concentrations, only: make_IceNumber, & | ||
| use module_mp_tempo_utils, only: calc_effectRad, make_IceNumber, & |
There was a problem hiding this comment.
@dustinswales Are you OK switching from Thompson's subroutines to TEMPO's permanently? There's no need to import both (Thompson and TEMPO) and use if statements in this scheme as done for RRTMG?
| @@ -225,16 +229,26 @@ subroutine GFS_suite_interstitial_4_run (im, levs, ltaerosol, tracers_total, ntr | |||
| qc_mp(i,k) = (clw(i,k,2)-save_qc(i,k)) / (one-spechum(i,k)) | |||
| !> - Convert number concentration from moist to dry | |||
| nc_mp(i,k) = gq0(i,k,ntlnc) / (one-spechum(i,k)) | |||
| nc_mp(i,k) = max(zero, nc_mp(i,k) + make_DropletNumber(qc_mp(i,k) * rho, nwfa(i,k)*rho) * orho) | |||
| if (imp_physics == imp_physics_thompson) then | |||
There was a problem hiding this comment.
Isn't this already within the 'if imp_physics == imp_physics_thompson' section?
| if (imp_physics == imp_physics_thompson) then | ||
| nc_mp(i,k) = max(zero, nc_mp(i,k) + make_DropletNumber_thmpsn(qc_mp(i,k) * rho, nwfa(i,k)*rho) * orho) | ||
| endif | ||
| if (imp_physics == imp_physics_tempo) then |
There was a problem hiding this comment.
This should never execute unless I'm wrong about the comment above.
| ni_mp(i,k) = gq0(i,k,ntinc) / (one-spechum(i,k)) | ||
| ni_mp(i,k) = max(zero, ni_mp(i,k) + make_IceNumber(qi_mp(i,k) * rho, save_tcp(i,k)) * orho) | ||
| ni_mp(i,k) = gq0(i,k,ntinc) / (one-spechum(i,k)) | ||
| if (imp_physics == imp_physics_thompson) then |
| @@ -250,13 +264,23 @@ subroutine GFS_suite_interstitial_4_run (im, levs, ltaerosol, tracers_total, ntr | |||
| !> - Update cloud water mixing ratio | |||
| qc_mp(i,k) = (clw(i,k,2)-save_qc(i,k)) | |||
| !> - Update cloud water number concentration | |||
| gq0(i,k,ntlnc) = max(zero, gq0(i,k,ntlnc) + make_DropletNumber(qc_mp(i,k) * rho, nwfa(i,k)*rho) * orho) | |||
| if (imp_physics == imp_physics_thompson) then | |||
There was a problem hiding this comment.
Same comment about adding more 'if' statements within the spatial loops. It's pretty inefficient unless compiler optimizers know how to "fix" it. I don't know if they do or not. Sometimes it's unavoidable, but in this case, it probably is.
| @@ -3315,7 +3315,7 @@ SUBROUTINE cal_cldfra3(CLDFRA, qv, qc, qi, qs, dz, & | |||
| & modify_qvapor, max_relh, & | |||
| & kts,kte, debug_flag) | |||
| ! | |||
| USE module_mp_thompson , ONLY : rsif, rslf | |||
| USE module_mp_tempo_utils , ONLY : rsif, rslf | |||
There was a problem hiding this comment.
Same question as to whether it is OK to switch to the TEMPO version here rather than add 'if' statements as necessary. @Qingfu-Liu as codeowner of this file, do you agree with this? Are there any plans for the TEMPO versions of these routines to differ from Thompson? Or, is this just preparing to replace Thompson with TEMPO?
|
@AndersJensen-NOAA @dustinswales I've reviewed the non-TEMPO files. Once my question regarding which of the TEMPO files to review (either the internal CCPP ones or the submodule ones), I'll do that. |
|
@AndersJensen-NOAA Please update the upstream repo PRs when you get a chance too (fv3atm, ufs-weather-model). The existing PRs are pointing to the old ccpp-physics PR. |
@dustinswales This updates the branch used for the TEMPO PR. #214 can be deleted.