gmtb/develop: update from master, MYJ PBL/SFC, Noah MP #320
Conversation
climbfuji
commented
Sep 18, 2019
•
climbfuji
commented
- MYJ SFC/PBL from @Qingfu-Liu as in Adding myj to ccpp #306
- NoahMP from @grantfirl as in NoahMP CCPP-compliancy #305
- bugfixes for MYJ from @climbfuji:
- convert to new metadata
- compile original modules with double precision floats
- set internal floating point precision to double precision (kind kfpt)
- correct dimensions of array dkt (runs from (1:im,1:nlevs-1) in FV3, not (1:im,1:nlev))
- bugfixes for NoahMP from @climbfuji:
- disable calculation of snocvr in CCPP, restore in FV3GFS_io.F90 (this is required for b4b identical results when dynamics and FV3GFS_io.F90 are compiled in single precision)
…or handling dates)
…y_pre_run; update GFS_time_vary_pre_run.fv3.F90 to new metadata and remove DDT dependency
add Julie as codeowner
… because it was originally done after the call to ccpp_physics_init in FV3GFS_io.F90/sfc_prop_restart_read
…use it was originally done after the call to ccpp_physics_init in FV3GFS_io.F90/sfc_prop_restart_read); this needs to happen before radition is called
…specific interstitial to test for b4b
….F90 that replaced legacy code (GOTO statements) with more complicated code
…velop_20190909 master: update from gmtb develop 2019/09/09
…rapper.F90 to new metadata format
…to myj_with_dom_mods
…j_with_dom_mods
…CCPP (do in FV3-IPD) to avoid b4b issues when 32BIT=Y
…_BL_MYJPBL.F90 with default double precision
…ule_MYJSFC_wrapper.F90: use double precision floats, remove trailing whitespaces
…e trailing whitespaces, declare local dkt2 array to account for dimension differences
|
See https://github.com/NCAR/NEMSfv3gfs/pull/249 for regression testing. |
climbfuji
requested review from
grantfirl,
JulieSchramm and
llpcarson
as code owners
| ,kfpt=8 & ! floating point variables | ||
| ,kdbl=8 ! double precision | ||
|
|
||
| REAL(kind=kfpt),PARAMETER :: A2=17.2693882,A3=273.15,A4=35.86,CP=1004.6 & |
There was a problem hiding this comment.
More locally-defined constants in this scheme that should be coming from the host. Now that I've been looking for this problem, it seems like this is the norm, not the exception for schemes.
| ,kint=4 & ! integer variables | ||
| !,kfpt=4 & ! floating point variables | ||
| ,kfpt=8 & ! floating point variables | ||
| ,kdbl=8 ! double precision |
There was a problem hiding this comment.
@climbfuji Is this a problem? Should precision of vars be coming from host? Perhaps it's OK since these are only used for local vars?
|
Hi Grant, the locally defined constants are actually more easier to port
the code to other framework (for eample, to HWRF). The warpper code is
built to handle locally defined real*4 or real*8 for the MYJ surface and
PBL schemes.
Qingfu
…On Thu, Sep 19, 2019 at 12:16 PM grantfirl ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In physics/module_BL_MYJPBL.F90
<#320 (comment)>:
> +! ,kdin=idouble &
+! ,kfpt=single &
+! ,kdbl=double
+
+! real (kind=kfpt),parameter :: r4_in=x'ffbfffff'
+! real (kind=kdbl),parameter :: r8_in=x'fff7ffffffffffff'
+! integer(kind=kint),parameter :: i4_in=-999 ! -huge(1)
+
+ integer,parameter:: &
+ klog=4 & ! logical variables
+ ,kint=4 & ! integer variables
+ !,kfpt=4 & ! floating point variables
+ ,kfpt=8 & ! floating point variables
+ ,kdbl=8 ! double precision
+
+ REAL(kind=kfpt),PARAMETER :: A2=17.2693882,A3=273.15,A4=35.86,CP=1004.6 &
More locally-defined constants in this scheme that should be coming from
the host. Now that I've been looking for this problem, it seems like this
is the norm, not the exception for schemes.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#320?email_source=notifications&email_token=AGTS6UQJTOXDZPZV3CLM7RDQKOQXHA5CNFSM4IYDLCGKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCFJYFNY#pullrequestreview-290685623>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGTS6UW2BSUP5T5GYHK2KBTQKOQXHANCNFSM4IYDLCGA>
.
|
|
On the long run we may want to remove the locally-defined precisions and use kind_phys etc. There is a broad agreement that the precision should not be hardcoded in the physics, and for many good reasons (beyond what can be discussed here). All models looking at CCPP (FV3, MPAS, WRF, NEPTUNE) are going to use kind_phys (and potentially other types), hence the portability won't be an issue. |
| kpbl(:)=levs-1 | ||
| ict=1 ! no longer used | ||
|
|
||
| if (lprnt1) then |
There was a problem hiding this comment.
For future reference, all the printing to console done in this routine can be eventually cleaned up by using CCPP error handling (setting errmsg). Also, this highlights the need to enhance CCPP's error messaging capabilities (log/warn messages in addition to error message).
| kind = kind_phys | ||
| intent = inout | ||
| optional = F | ||
| [phy_myj_qsfc] |
There was a problem hiding this comment.
I'm curious why MYJ needs its own variable for this.
| kind = kind_phys | ||
| intent = inout | ||
| optional = F | ||
| [phy_myj_z0base] |
There was a problem hiding this comment.
How is this different than existing roughness length variables?
|
Hi grant, those variables are from previous time step, and used as guess
for the current step. I did not see any other surface layer keeps those
variables from previous time step.
Qingfu
…On Thu, Sep 19, 2019 at 12:30 PM grantfirl ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In physics/module_MYJPBL_wrapper.meta
<#320 (comment)>:
> + units = m s-1
+ dimensions = (horizontal_dimension)
+ type = real
+ kind = kind_phys
+ intent = inout
+ optional = F
+[phy_myj_vz0]
+ standard_name = v_wind_component_at_viscous_sublayer_top
+ long_name = v wind component at viscous sublayer top over water
+ units = m s-1
+ dimensions = (horizontal_dimension)
+ type = real
+ kind = kind_phys
+ intent = inout
+ optional = F
+[phy_myj_z0base]
How is this different than existing roughness length variables?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#320?email_source=notifications&email_token=AGTS6UVZQXMM47WIITENOK3QKOSLRA5CNFSM4IYDLCGKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCFJ2GMY#pullrequestreview-290693939>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGTS6URUJ6RVZ43DKGI66FTQKOSLRANCNFSM4IYDLCGA>
.
|
|
It is worth noting that we purposely didn't touch the original MYJ PBL/SFC schemes, because making this code CCPP compliant requires more or less a rewrite of the schemes. This should be done in the future iff the scheme's performances are promising enough to justify the effort. Some aspects, e.g. the local definition of kinds, need to be addressed nonetheless. |
Ah, yes, I forgot about that. I'm definitely not trying to hold this merge up, just pointing things out for the future if and when this scheme gains traction. |
|
@grantfirl mentioned the need for someone else to review his NoahMP changes, I did that in preparation for my changes on top of it, hence this can be merged (similar for associated PRs). |
