Update Tiedtke scheme with MPASv8.0.0 updates #1025
Conversation
matusmartini
requested review from
grantfirl,
Qingfu-Liu and
dustinswales
as code owners
|
@matusmartini Have these changes been tested and evaluated for scientific impact in any UFS application? |
|
@lisa-bengtsson This scheme is orphaned in CCPP, and it sounds as if there is an interest to try it with FV3? We tested with NEPTUNE at various configurations and found improved skill especially in lower troposphere. More info and discussion on this is in #1024 |
|
@matusmartini thank you for pointing me to the discussion. |
physics/cu_ntiedtke.F90
Outdated
| real(kind=kind_phys),private :: rovcp,r5alvcp,r5alscp,ralvdcp,ralsdcp,ralfdcp | ||
|
|
||
| real(kind=kind_phys),parameter:: t13 = 0.333333333 | ||
| real(kind=kind_phys),parameter:: tmelt = 273.16 |
There was a problem hiding this comment.
temperature_at_zero_celsius and triple_point_temperature_of_water are both defined as host-defined constants already. For consistency across schemes and hosts, tmelt should probably be an argument with one of the above variables.
There was a problem hiding this comment.
Good catch. Maybe this could be a separate PR?
physics/cu_ntiedtke.F90
Outdated
| @@ -14,27 +14,29 @@ module cu_ntiedtke | |||
| ! this also requires redefining derived constants in the | |||
| ! parameter section below | |||
| use physcons, only:rd=>con_rd, rv=>con_rv, g=>con_g, & | |||
There was a problem hiding this comment.
These should be passed in through the argument list (standard names that should be passed in are given below in parentheses):
con_rd=gas_constant_of_dry_aircon_rv=gas_constant_water_vaporcon_g=gravitational_accelerationcon_cp=specific_heat_of_dry_air_at_constant_pressurecon_hvap=latent_heat_of_vaporization_of_water_at_0Ccon_hfus=latent_heat_of_fusion_of_water_at_0C
There was a problem hiding this comment.
Good idea. Maybe this could be a separate PR?
physics/cu_ntiedtke.F90
Outdated
| logical :: lmfpen,lmfmid,lmfscv,lmfdd,lmfdudv | ||
| parameter(lmfpen=.true.,lmfmid=.true.,lmfscv=.true.,lmfdd=.true.,lmfdudv=.true.) | ||
| !-------------------- | ||
| logical,parameter:: lmfpen = .true. |
There was a problem hiding this comment.
Should any of these logicals be settable through namelist?
There was a problem hiding this comment.
That would be a nice feature.
physics/cu_ntiedtke.F90
Outdated
| real(kind=kind_phys), dimension( :, : ), intent(in ) :: pzz, prsi | ||
| !--- input arguments: | ||
| integer, intent(in):: lq,km,ktrac | ||
| integer,intent(in),dimension(lq):: lmask |
There was a problem hiding this comment.
Argument variable arrays should be assumed shape for the CCPP. See https://ccpp-techdoc.readthedocs.io/en/latest/CompliantPhysicsParams.html#input-output-variable-argument-rules.
physics/cu_ntiedtke.F90
Outdated
| ! end if | ||
| deallocate(pcen) | ||
| deallocate(ptenc) | ||
| errmsg = 'cu_ntiedtke_run OK' |
There was a problem hiding this comment.
It's not necessary to set errmsg since it isn't checked unless errflg /= 0 (at least in UFS-based models).
physics/cu_ntiedtke.F90
Outdated
|
|
||
| return | ||
| end subroutine cudlfsn | ||
| ike=klev-3 |
There was a problem hiding this comment.
These large sections where there are whitespace changes are really difficult to detect changes. I wonder if we could see a version without whitespace changes for review, then reinstate them after review?
|
According to the discussion in the 8/2/2023 CCPP Physics Code Mgmt meeting, we will delay the final review of this PR until Wei Wang (who did the Tiedtke updates for MPAS v8) is back in mid September 2023. |
Thank you @ligiabernardet for the discussion yesterday and @grantfirl for all of the above comments. The goal of my initial commit was to point out the differences between orphaned Tiedtke scheme in CCPP and seemingly cleaner version in MPAS with the scientific updates without imposing sense of urgency. These changes simply attempt to sync as much as possible with MPAS version while leaving MPAS-specifics out (for example units of latent and sensible heat, units of pressure, humidity). If the ultimate goal is to have ONE Tiedtke scheme across MPAS and CCPP then some of @grantfirl suggestions (such as the one about assumed shape of argument variable arrays) would have to propagate into MPAS, bridging the two... In addition, it would be nice to avoid duplicate efforts with MPAS shared physics driver @dustinswales going forward. In the interim, proceeding with these changes (after review) would bring the CCPP version closer to the MPAS version. Alternatively, I could just bring the very minimal set of updates and these could just be the scientific ones and bugfixes. Perhaps that would be a better start, when @weiwangncar is available, and would also make the review much easier if there is preference for that. @yangfanglin |
|
May I ask what the status of this PR is? |
|
Is there a status update on this PR? |
|
@climbfuji @areinecke I don't think there is anything "technical" holding up this PR and I'd like to see these changes propagate into the codebase. I think the initial holdup was testing, but since this scheme is not used/tested by any UFS/SCM applications, my inclination is to rely on the organization that has tested it (e.g NRL). In general, when an untested and orphaned scheme is adopted, what burden do we put on the contributor beyond testing/validating in their host? I view this as a great contribution and I don't think it makes sense for NRL to put together a SDF for the SCM/UFS testing. In the future, if a UWM application wants to use this scheme, they will need to build the infrastructure and validate at that time. @lisa-bengtsson @yangfanglin Do you have objections to moving forward? |
|
@dustinswales I don't have any objections about the code changes. I do know that the HAFS developers have done a lot of tests with the Tiedke scheme as they want diversity in their A and B configurations. It could be good to check with Zhan Zhang at EMC if there's any concern for their testing for HAVSv3. |
|
Thank you for following up on this. Before this is merged, I need to resolve the conflicts and add one more commit with improved comments (thanks to Michael Diaz from NRL). |
|
@dustinswales : HAFS will not use Tiedtke for the upcoming implementation. No objection to committing this PR. It would be desirable to set up a HAFS RT after the PR is completed ( @AndrewHazelton ) |
There was a problem hiding this comment.
These changes look fine to me.
@yangfanglin I opened an issue in the UWM to create a Tiedtke based RT for HAFS. ufs-community/ufs-weather-model#2542
The updates include: introducing scale dependency factors (Wang 2022, WAF), updated entrainment coefficients, vertical indexing fix, and horizontal wind tendency term (pgf_u, pg_v) calculation also for shallow convection not just deep and mid- convection, cleaner fields declarations.
NEW: Added comments from IFS documentation. Thanks to Michael Diaz from NRL for the updates and tuning experiments.(00d04d5)
Issue #1024