Metadata bug fixes, cleanup of active_gases_array use in RRTMGP, cleanup of Ferrier-Aligo variables, remove redundant consistency checks #749
Conversation
…g-array-alloc-ccpp-caps
…g-array-alloc-ccpp-caps
|
@dustinswales @SMoorthi-emc please review this PR if you have time - it should be all straightforward, but it contains quite some cleanup and simplification, in particular on the RRTMGP side. Thanks! |
|
@mzhangw Please review my code changes for Ferrier-Aligo. I stripped out unnecessary variables from the argument list that were used and can be declared locally without affecting the scheme. Thanks! |
| character(len=*), intent(out) :: errmsg | ||
| integer, intent(out) :: errflg | ||
|
|
||
| ! Initialize CCPP error handling variables | ||
| errmsg = '' | ||
| errflg = 0 | ||
|
|
||
| ! Consistency check that the number of albedo components in array |
There was a problem hiding this comment.
Consistency checks are no longer required - the CCPP framework does this automatically in the auto-generated caps when compiled in DEBUG mode.
|
@tanyasmirnova please review the change in |
There was a problem hiding this comment.
@climbfuji
All of the GP changes look fantastic to me. Thanks for cleaning up my mess
|
@climbfuji |
| @@ -54,10 +52,6 @@ subroutine mp_fer_hires_init(ncol, nlev, dtp, imp_physics, & | |||
| logical, intent(in) :: restart | |||
| character(len=*), intent(out) :: errmsg | |||
| integer, intent(out) :: errflg | |||
| real(kind_phys), intent(out) :: f_ice(:,:) | |||
There was a problem hiding this comment.
I doubt if we should omit cwm and "fractions" in FA scheme, since these variables have not been fully tested in UFS yet. Based on our email thread with @ericaligo-NOAA @ligiabernardet and HWRF team on Aug 19, 2019, these variables are essential in operational HWRF model, which uses FA scheme. @ligiabernardet confirmed that we should proceed with also outputting total condensate (cwm) and fractional arrays.
There was a problem hiding this comment.
The point is that these arrays are used nowhere in the host model (FV3) at the moment. Further, they were part of the GFS_interstitial derived data type, which doesn't make sense because this data is not persistent. In other words, the fields couldn't be used in FV3 or the dycore, they couldn't be written to disk, etc. Lastly, because they were part of the GFS_interstitial DDT, there standard names have not been updated yet.
If we decide in the future that these arrays are needed in the host model, then we can put them back in and use the correct DDTs in FV3, and standard names that are derived from the rules for creating standard names.
The |
There was a problem hiding this comment.
Lots of nice simplification in here. I didn't take too much effort to understand the RRTMGP changes (since @dustinswales already signed off), but it looks good to me.
| @@ -32,9 +32,7 @@ module mp_fer_hires | |||
| !! \htmlinclude mp_fer_hires_init.html | |||
| !! | |||
| subroutine mp_fer_hires_init(ncol, nlev, dtp, imp_physics, & | |||
| imp_physics_fer_hires, & | |||
| restart, & | |||
| f_ice,f_rain,f_rimef, & | |||
There was a problem hiding this comment.
From my knowledge of Ferrier microphysics that I used to work with about a decade ago, these variables are needed in the radiation_clouds.
May be this has changed now.
This PR contains the following changes:
active_gases_arrayin RRTMGPdtendarray, and two arrays in RAS)inGFS_radiation_surfaceandGFS_rrtmg_setup(no longer required due to the new debugging features in the auto-generated caps, see ccpp-framework PR Rename --debug flag to --verbose, add new debugging capability to detect array size mismatches, pretty-print auto-generated caps ccpp-framework#404)Associated PRs:
#749
NCAR/ccpp-framework#404
https://github.com/NOAA-EMC/fv3atm/pull/407/files
NOAA-PSL/stochastic_physics#48
ufs-community/ufs-weather-model#850
For regression testing, see
ufs-community/ufs-weather-model#850