Implement Ferrier-Aligo MP scheme in CCPP-Physics #352

mzhangw · 2019-11-12T16:27:15Z

This PR includes @ericaligo-NOAA @ChunxiZhang-NOAA @mzhangw 's work to implement Ferrier-Aligo MP scheme into CCPP-physics

…nto FA-HWRF-V4_0a

…nto HAFS_fer_hires

…e used for determining RHgrd

…nto HAFS_fer_hires

Deleted QS since it will not be used. we only need QI. module_mp_fer_hires_pre.F90: changes related to f_ice, f_rain and f_rimef module_mp_fer_hires_pre.F90: added

…ccpp-physics into HAFS_fer_hires

GFS_PBL_generic.F90: define tracers for vertical diffusion GFS_rrtmg_pre.F90: change ncnd module_mp_fer_hires_pre.F90: revised the definition to tracers mp_fer_hires.F90: revised the definition to tracers

…nto HAFS_fer_hires

into HAFS_fer_hires

physics/GFS_MP_generic.F90

grantfirl · 2019-11-12T22:03:29Z

physics/GFS_rrtmg_pre.F90

@@ -713,6 +723,33 @@ subroutine GFS_rrtmg_pre_run (Model, Grid, Sfcprop, Statein,   & ! input
                         Model%sup, Model%kdt, me,                      &
                         clouds, cldsa, mtopa, mbota, de_lgth)               !  ---  outputs

+!        elseif (Model%imp_physics == 15) then           ! F-A cloud scheme


Same comment as above and applies to all commented code in this scheme. Consider removing unless you really need to keep this in for later use, like if you're still debugging the scheme.

grantfirl · 2019-11-12T22:05:01Z

physics/GFS_suite_interstitial.F90

-      imp_physics_zhao_carr_pdf, imp_physics_gfdl, imp_physics_thompson, imp_physics_wsm6, prsi, prsl, prslk, rhcbot,     &
-      rhcpbl, rhctop, rhcmax, islmsk, work1, work2, kpbl, kinver,                                                         &
-      clw, rhc, save_qc, save_qi, errmsg, errflg)
+    subroutine GFS_suite_interstitial_3_run (im, levs, nn, cscnv,       &


Thanks for shortening these lines.

grantfirl · 2019-11-12T22:06:24Z

physics/HAFS_update_moist.F90

@@ -0,0 +1,138 @@
+!>\file HAFS_update_moist.F90


I didn't actually see this executed via a SDF.

It is a prep for HAFS RRTMG. I deleted it from this branch

grantfirl · 2019-11-12T22:07:46Z

physics/HAFS_update_moist.F90

+!--------------------
+!
+      INTEGER :: I,K
+      character(len=*), intent(out) :: errmsg


Doesn't really matter, but these aren't local variables as commented.

grantfirl · 2019-11-12T22:10:36Z

physics/HAFS_update_moist.meta

+  type = integer
+  intent = out
+  optional = F
+########################################################################


Doesn't matter, but I don't think you need metadata entries for empty init and finalize schemes.

grantfirl · 2019-11-12T22:11:40Z

physics/mp_fer_hires.F90

+      !           is glaciated to ice
+      ! * T_ICE_init - maximum temperature (C) at which ice nucleation occurs
+      REAL, PUBLIC, PARAMETER :: T_ICE=-40.,                             &
+                                 T0C=273.15,                             &


T0C might already be a host model constant.

In physcons.F90, there is only a dubious A3 = 273.16 (Kelvin I assume)

grantfirl · 2019-11-12T22:13:08Z

physics/mp_fer_hires.F90

+      USE GFS_typedefs,        ONLY : GFS_control_type
+      implicit none
+
+      type(GFS_control_type),         intent(in)    :: Model


Please avoid passing in GFS_* DDTs to schemes if you can. This will need to be cleaned up later if left in.

grantfirl · 2019-11-12T22:18:54Z

physics/module_MP_FER_HIRES.F90

+!MZ
+!MZ temporary values copied from module_CONSTANTS; ideally they come from host model
+!side
+     REAL, PARAMETER :: pi=3.141592653589793       ! ludolf number


As with a bunch of other schemes, we would like to pass these in if possible. Most of these are host-defined constants.

Agree. we need to consolidate module_CONSTANTS in WRF with physcons . There are a lot difference depend on which core (nmm, fv3, arw)

2. correct tracer diffusions definition before/after PBL in GFS_PBL_generic

…to HAFS_fer_hires

climbfuji

I created an issue to follow-up on using conditionally allocated arrays in situations where they may not be allocated (I haven't checked it, just noting it so that it does not get forgotten): #357 - otherwise these changes look ok to me minus a few minor fixes (remove commented-out lines that were introduced during the development phase and not removed afterwards). These changes can be made carefully and pushed to the branch as an isolated commit so that we do not need to rerun the tests.

climbfuji · 2019-11-22T21:01:00Z

physics/GFS_MP_generic.F90

@@ -277,7 +281,7 @@ subroutine GFS_MP_generic_post_run(im, ix, levs, kdt, nrcm, ncld, nncl, ntcw, nt
 !! and determine explicit rain/snow by snow/ice/graupel coming out directly from MP
 !! and convective rainfall from the cumulus scheme if the surface temperature is below
 !! \f$0^oC\f$.
-      if (imp_physics == imp_physics_gfdl .or. imp_physics == imp_physics_thompson) then
+      if (imp_physics == imp_physics_gfdl .or. imp_physics == imp_physics_thompson ) then


This change shouldn't be in the PR.

climbfuji · 2019-11-22T21:01:31Z

physics/GFS_MP_generic.F90

            total_precip = snow0(i)+ice0(i)+graupel0(i)+rain0(i)+rainc(i)
            if (total_precip > rainmin) then
              srflag(i) = (snow0(i)+ice0(i)+graupel0(i)+csnow)/total_precip
            endif
+          !endif


Please remove this comment line 307

climbfuji · 2019-11-22T21:02:45Z

physics/GFS_rrtmg_pre.F90

@@ -737,8 +748,8 @@ subroutine GFS_rrtmg_pre_run (Model, Grid, Sfcprop, Statein,   & ! input
 !                           clouds, cldsa, mtopa, mbota, de_lgth)               !  ---  outputs
          endif

-        elseif(Model%imp_physics == 8 .or. Model%imp_physics == 6) then		       ! Thompson / WSM6 cloud micrphysics scheme
-
+        elseif(Model%imp_physics == 8 .or. Model%imp_physics == 6 .or.  &


At some point, we should be using == imp_physics_thompson instead of == 8 here (not this PR).

climbfuji · 2019-11-22T21:03:57Z

physics/GFS_suite_interstitial.F90

@@ -14,21 +14,22 @@ end subroutine GFS_suite_interstitial_rad_reset_finalize
 !> \section arg_table_GFS_suite_interstitial_rad_reset_run Argument Table
 !! \htmlinclude GFS_suite_interstitial_rad_reset_run.html
 !!
-    subroutine GFS_suite_interstitial_rad_reset_run (Interstitial, errmsg, errflg)
+    subroutine GFS_suite_interstitial_rad_reset_run (Interstitial, Model, errmsg, errflg)


We have to find a better way for this in the future. We don't want to pass the DDTs if we don't have to.

climbfuji · 2019-11-22T21:04:17Z

physics/GFS_suite_interstitial.F90

@@ -679,6 +685,9 @@ subroutine GFS_suite_interstitial_4_run (im, levs, ltaerosol, cplchm, tracers_to
      real(kind=kind_phys), dimension(im,levs,nn),    intent(inout) :: clw
      ! dqdti may not be allocated
      real(kind=kind_phys), dimension(:,:),           intent(inout) :: dqdti
+      !integer,                        intent(in)    :: mpirank


please remove lines 688 and 689

climbfuji · 2019-11-22T21:10:46Z

physics/mp_fer_hires.F90

+      !           is glaciated to ice
+      ! * T_ICE_init - maximum temperature (C) at which ice nucleation occurs
+      REAL, PUBLIC, PARAMETER :: T_ICE=-40.,                             &
+                                 T0C=273.15,                             &


In physcons.F90, there is only a dubious A3 = 273.16 (Kelvin I assume)

climbfuji · 2019-11-22T21:12:23Z

physics/mp_fer_hires.F90

+      logical,                        intent(in)    :: restart
+      character(len=*),               intent(out)   :: errmsg
+      integer,                        intent(out)   :: errflg
+      real(kind_phys),                intent(out), optional  :: f_ice(1:ncol,1:nlev)


In this place it is ok to have explicit sizes of the dimensions (although assumed-size would be preferable), because the scheme is only called when these arrays are allocated.

climbfuji · 2019-11-22T21:16:29Z

physics/mp_fer_hires.F90

+      real(kind_phys),   intent(inout) :: t(1:ncol,1:nlev)
+      real(kind_phys),   intent(inout) :: q(1:ncol,1:nlev)
+      real(kind_phys),   intent(inout) :: cwm(1:ncol,1:nlev)
+      real(kind_phys),   intent(inout) :: train(1:ncol,1:nlev)


But in this case it is ok, or? mp_fer_hires_run only gets called if Model%imp_physics == Model%imp_physics_fer_hires, in which case train is also allocated?

climbfuji

The code changes now look good to me, I am wondering if we should clean up the commit history. This is a little tricky to get right, but essentially these are the steps:

check out the dtc/develop branch of ccpp-physics somewhere outside your repo
add your remote
git merge --squash your_remote/HAFS_fer_hires

there should be no conflicts, hopefully, if so, fix

you will have one commit message to add, something like "Adding Ferrier Aligo MP scheme and modifications to interstitial/radiation code"
then use a diff tool (diffmerge, meld, ...) to make sure that this ccpp-physics directory is identical to your original FA ccpp-physics directory
then force-push this new branch to your_remote/HAFS_fer_hires and overwrite your existing changes
If this sounds scary to you, let me know and I can do that. I would have to replace your ccpp-physics PR with mine then, but I will make sure you get the credit.

mzhangw · 2019-11-22T22:01:10Z

Please go ahead. Thanks!

climbfuji · 2019-11-22T22:52:48Z

This PR is replaced by #358

mzhangw and others added 30 commits July 1, 2019 16:31

initialize Ferrier-Aligo MP scheme implementation from HWRF V4.0

193435b

add fer_hires wrapper module

e77c0a1

initialize FER_HIRES implementation

d0d4035

Merge branch 'gmtb/develop' of https://github.com/NCAR/ccpp-physics i…

63f07c4

…nto FA-HWRF-V4_0a

pass prebuild

d666a3e

change file name

970ae66

minor fix

d06f755

Merge branch 'gmtb/develop' of https://github.com/NCAR/ccpp-physics i…

bbac675

…nto HAFS_fer_hires

initialize FER_HIRES scheme

7c481b5

update

dbabee7

Merge branch 'gmtb/develop' of https://github.com/NCAR/ccpp-physics i…

b78a1a2

…nto HAFS_fer_hires

delete HWRF F-A scheme

8b886b9

convert USE ESMF to ccpp mpi method; temporary constant treatment

c29c3cd

dimension fixed

473ff9e

further constants fix

80fedc4

Aligo-use the dx of the 1st i poit to set an integer value of dx to b…

fb011da

…e used for determining RHgrd

minor fix

d5f8a62

fortran format fix

73f95a6

Fixed bug to allow both Qi and Qc to be updated from CU scheme.

6b88864

minor fix

9aaa575

Merge branch 'gmtb/develop' of https://github.com/NCAR/ccpp-physics i…

4e0d9bd

…nto HAFS_fer_hires

mp_fer_hires.F90: changed the definitions for f_ice, f_rain and f_rimef.

c7faeb7

Deleted QS since it will not be used. we only need QI. module_mp_fer_hires_pre.F90: changes related to f_ice, f_rain and f_rimef module_mp_fer_hires_pre.F90: added

fix omp message and pass F-A scheme

1426c6e

Merge branch 'chunxi_physics' of https://github.com/ChunxiZhang-NOAA/…

3a26975

…ccpp-physics into HAFS_fer_hires

make consistent standard name as Chunxis implementation

ec729e8

GFS_MP_generic.F90: recalculate srflag

727417c

GFS_PBL_generic.F90: define tracers for vertical diffusion GFS_rrtmg_pre.F90: change ncnd module_mp_fer_hires_pre.F90: revised the definition to tracers mp_fer_hires.F90: revised the definition to tracers

tracer treatment fix

1808226

Merge branch 'gmtb/develop' of https://github.com/NCAR/ccpp-physics i…

44137a3

…nto HAFS_fer_hires

bug fixed in augument list of FER_HIRES

1f8a26a

Merge branch 'HAFS_fer_hires' of https://github.com/mzhangw/ccpp-physics

bed9c0e

into HAFS_fer_hires

grantfirl reviewed Nov 12, 2019

View reviewed changes

physics/GFS_MP_generic.F90 Outdated Show resolved Hide resolved

grantfirl reviewed Nov 12, 2019

View reviewed changes

climbfuji added do not merge work in progress labels Nov 14, 2019

climbfuji mentioned this pull request Nov 14, 2019

HAFS_FER_HIRES #296

Closed

mzhangw added 6 commits November 14, 2019 13:46

1. do some code cleanup

380229c

2. correct tracer diffusions definition before/after PBL in GFS_PBL_generic

Merge branch 'dtc/develop' of https://github.com/NCAR/ccpp-physics in…

482a43b

…to HAFS_fer_hires

cleanup FA codes

db9e3a7

delete update_moist module

ab52b26

minor change

db7fc8d

Merge branch 'dtc/develop' of https://github.com/NCAR/ccpp-physics in…

4c2abd1

…to HAFS_fer_hires

mzhangw mentioned this pull request Nov 22, 2019

configuration files for Ferrier-Aligo MP scheme NCAR/ufs-weather-model#1

Merged

climbfuji requested changes Nov 22, 2019

View reviewed changes

climbfuji removed do not merge work in progress labels Nov 22, 2019

monir format fix

bd4a30c

climbfuji reviewed Nov 22, 2019

View reviewed changes

climbfuji mentioned this pull request Nov 22, 2019

Replacement PR for "Implement Ferrier-Aligo MP scheme in CCPP-Physics" from @mzhangw (squashed commits) #358

Merged

climbfuji added do not merge wontfix labels Nov 22, 2019

climbfuji closed this Nov 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Ferrier-Aligo MP scheme in CCPP-Physics #352

Implement Ferrier-Aligo MP scheme in CCPP-Physics #352

mzhangw commented Nov 12, 2019

grantfirl Nov 12, 2019 •

edited

Loading

mzhangw Nov 22, 2019

grantfirl Nov 12, 2019

grantfirl Nov 12, 2019

mzhangw Nov 22, 2019

grantfirl Nov 12, 2019

grantfirl Nov 12, 2019

grantfirl Nov 12, 2019

climbfuji Nov 22, 2019

grantfirl Nov 12, 2019

mzhangw Nov 22, 2019

grantfirl Nov 12, 2019

mzhangw Nov 22, 2019

climbfuji left a comment

climbfuji Nov 22, 2019

climbfuji Nov 22, 2019

mzhangw Nov 22, 2019

climbfuji Nov 22, 2019

climbfuji Nov 22, 2019

climbfuji Nov 22, 2019

mzhangw Nov 22, 2019

climbfuji Nov 22, 2019

climbfuji Nov 22, 2019

climbfuji Nov 22, 2019

climbfuji left a comment

mzhangw commented Nov 22, 2019

climbfuji commented Nov 22, 2019

Implement Ferrier-Aligo MP scheme in CCPP-Physics #352

Implement Ferrier-Aligo MP scheme in CCPP-Physics #352

Conversation

mzhangw commented Nov 12, 2019

grantfirl Nov 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

climbfuji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

climbfuji left a comment

Choose a reason for hiding this comment

there should be no conflicts, hopefully, if so, fix

mzhangw commented Nov 22, 2019

climbfuji commented Nov 22, 2019

grantfirl Nov 12, 2019 •

edited

Loading