Move Noah MP init to CCPP and update Noah MP code #575

climbfuji · 2021-02-21T13:15:46Z

This PR does several things:

Move the Noah MP initialization of surface fields from fv3atm (io/FV3GFS_io.F90) to CCPP from @climbfuji. This was done consistently with the previous move of the Noah initialization to GFS_phys_time_vary for the following two reasons:
- several of the Noah MP fields may be required by other physics that run before Noah MP (order in the suite definition file)
- per issue Reorganization of "time_vary" interstitials #549, we are looking at reorganizing the entire time_vary group in the near future anyway
Updates to Noah MP and Noah MP - radiation coupling from @HelinWei-NOAA
Contains the changes from Initialize ice flux in CMEPSA bug fix. "flag_cice" used to be a proxy for "not 1st time step cold… #563 (Initialize ice flux in CMEPSA bug fix) from @shansun6

Associated PRs:

#575
NOAA-EMC/fv3atm#249
ufs-community/ufs-weather-model#425

For regression testing, see ufs-community/ufs-weather-model#425.

… start" in NEMS, but is no longer the case in CMEPS. It is now replaced by "kdt>1". This will change results in all coupled models. Co-authored-by: ben.green@noaa.gov

…fc_noah_wrfv4_interstitial

…dard names in physics/GFS_phys_time_vary.fv3.meta and physics/sfc_noahmp_drv.meta

…ahmp_init_in_ccpp

…matting in physics/rrtmg_sw_pre.F90

barlage · 2021-02-23T18:41:42Z

physics/GFS_phys_time_vary.fv3.F90

+                 tgxy(ix)     = tsfcl(ix)
+                 tahxy(ix)    = tsfcl(ix)
+
+                 if (snowd(ix) > 0.01 .and. tsfcl(ix) > con_t0c ) tvxy(ix)  = con_t0c


should these various float factors be set to _kind_phys at this time? Or we can address this when this gets moved to the noahmp module later.

@barlage I made this change for all reals used in the NoahMP init in commit f2d2b12 - do you want to check if this is what you had in mind, please?

@climbfuji looks good to me

barlage · 2021-02-23T18:49:54Z

physics/noahmp_tables.f90

@@ -725,7 +735,7 @@ module noahmp_tables
    real :: refkdt_table = 3.0         !parameter in the surface runoff parameterization
    real :: frzk_table  =0.15          !frozen ground parameter
    real :: zbot_table  =  -8.0        !depth [m] of lower boundary soil temperature
-    real :: czil_table = 0.075         !parameter used in the calculation of the roughness length for heat
+    real :: czil_table = 0.01          !parameter used in the calculation of the roughness length for heat


@HelinWei-NOAA currently, this isn't used for the noahmp options we use, but this is too low, probably should have been 0.1

Since this isn't used now, we can change those values with the future PR for Rongqian's sfcdiff modification.

Anything you want me to change?

@climbfuji see PR to your branch

barlage · 2021-02-23T18:56:05Z

physics/sfc_noahmp_drv.F90

+  real(kind=kind_phys), parameter :: a4      = 35.86
+  real(kind=kind_phys), parameter :: a23m4   = a2*(a3-a4)
+
+  real, parameter                 :: undefined       =  -1.e36   ! TODO change to smaller value


@climbfuji @HelinWei-NOAA I didn't realize all these noahmp updates were going in at this time; there are a few unfinished TODOs in here, they should not cause any problems but we will want to address them in a follow-up PR, if not now.

Ok, so no changes to make for now?

@climbfuji Dom - is there a ccpp undefined value? I would like to change this to a lower magnitude value because it causes problems when using nco to calculate totals. If there is a ccpp value we can use it or I will set it to a smaller value.

should we set it to the missing_value = 9.99e20_r8 used in other places?

I essentially did that here:

barlage@98f53cf

barlage · 2021-02-23T18:57:30Z

physics/sfc_noahmp_drv.F90

@@ -0,0 +1,1529 @@
+#define CCPP


@HelinWei-NOAA I see this doesn't use the new name for this file. Was that intentional?

just try to be consistent with other drivers: sfc_drv_ruc.F90, sfc_drv.f, sfc_sice.f,sfc_ocean.F, and sfc_nst.f. All start with sfc

then we should probably call it sfc_drv_noahmp

@barlage can you please clarify? I see

!> \file sfc_noahmp_drv.F90

which is consistent with the name of the file. Or did you want to rename the file althogether? Or do when we move to your external repository?

@climbfuji sorry for the cryptic message; when I first did the driver rewrite, I had a different name for this file but Helin changed it. I was trying to make the naming follow some logic but there are several other cases where names are not logical so let's just keep it for now.

…ahmp_init_in_ccpp

…ation

HelinWei-NOAA · 2021-02-25T19:56:03Z

Dom, Can you wait for a while? I am working with Mike and will provide you with the updated version of Noah MP driver (finish those TODO) and the table. Thanks. Helin

…

On Thu, Feb 25, 2021 at 1:38 PM Dom Heinzeller ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In physics/noahmp_tables.f90 <#575 (comment)>: > @@ -725,7 +735,7 @@ module noahmp_tables real :: refkdt_table = 3.0 !parameter in the surface runoff parameterization real :: frzk_table =0.15 !frozen ground parameter real :: zbot_table = -8.0 !depth [m] of lower boundary soil temperature - real :: czil_table = 0.075 !parameter used in the calculation of the roughness length for heat + real :: czil_table = 0.01 !parameter used in the calculation of the roughness length for heat Anything you want me to change? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#575 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALPHKYEMNQCOYDDYA756LLDTA2KKDANCNFSM4X67HZ6Q> .

climbfuji · 2021-02-25T20:02:20Z

Ok. We would be the next in line in the commit queue, but we can let another PR jump the queue, of course.

…

On Feb 25, 2021, at 12:56 PM, HelinWei-NOAA ***@***.***> wrote: Dom, Can you wait for a while? I am working with Mike and will provide you with the updated version of Noah MP driver (finish those TODO) and the table. Thanks. Helin On Thu, Feb 25, 2021 at 1:38 PM Dom Heinzeller ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In physics/noahmp_tables.f90 > <#575 (comment)>: > > > @@ -725,7 +735,7 @@ module noahmp_tables > real :: refkdt_table = 3.0 !parameter in the surface runoff parameterization > real :: frzk_table =0.15 !frozen ground parameter > real :: zbot_table = -8.0 !depth [m] of lower boundary soil temperature > - real :: czil_table = 0.075 !parameter used in the calculation of the roughness length for heat > + real :: czil_table = 0.01 !parameter used in the calculation of the roughness length for heat > > Anything you want me to change? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#575 (comment)>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/ALPHKYEMNQCOYDDYA756LLDTA2KKDANCNFSM4X67HZ6Q> > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#575 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5C2RONMI4G6MNNGUO3V3LTA2TODANCNFSM4X67HZ6Q>.

Fix noahmp driver TODO

…cpp-physics into noahmp_init_in_ccpp

climbfuji · 2021-03-01T15:24:27Z

@barlage @HelinWei-NOAA @shansun6 Can I please ask you to look at the code changes again and approve, if satisfied? It's our turn in the commit queue today. @shansun6 I pulled your PR #563 into this PR. Thanks!

ShanSunNOAA · 2021-03-01T15:42:12Z

Hi Dom, The initialization of iceflux looks good to me. I approve of it. Thanks, Shan

…

On Mon, Mar 1, 2021 at 8:24 AM Dom Heinzeller ***@***.***> wrote: @barlage <https://github.com/barlage> @HelinWei-NOAA <https://github.com/HelinWei-NOAA> @shansun6 <https://github.com/shansun6> Can I please ask you to look at the code changes again and approve, if satisfied? It's our turn in the commit queue today. @shansun6 <https://github.com/shansun6> I pulled your PR #563 <#563> into this PR. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#575 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALORMVUQPXRRZJTPPFVYFVDTBOWTZANCNFSM4X67HZ6Q> .

grantfirl · 2021-03-01T18:00:05Z

physics/module_sf_noahmp_glacier.f90

-  real, parameter :: rw     = 461.269   !gas constant for  water vapor (j/kg/k)
-  real, parameter :: denh2o = 1000.     !density of water (kg/m3)
-  real, parameter :: denice = 917.      !density of ice (kg/m3)
+  real (kind=kind_phys), parameter :: grav   = 9.80616   !acceleration due to gravity (m/s2)


Unless there is a good reason for NoahMP to be defining its own physical constants, they should come through the CCPP where possible.

grantfirl · 2021-03-01T18:02:16Z

physics/module_sf_noahmplsm.f90

-  real, parameter :: rw     = 461.269   !gas constant for  water vapor (j/kg/k)
-  real, parameter :: denh2o = 1000.     !density of water (kg/m3)
-  real, parameter :: denice = 917.      !density of ice (kg/m3)
+  real (kind=kind_phys), parameter :: grav   = 9.80616   !acceleration due to gravity (m/s2)


Same comment WRT physical constants. Perhaps can be addressed in a followup PR at some point if all the testing has already been done.

Yes, these will need to go in an issue and become a follow-up PRs, baselines already created on a few platforms.

Recorded here: #582

This physics code will be tied to the Noah-MP repository in the future #551 so CCPP will not have any control of this. We would need to work with the Noah-MP community to resolve this.

We'll need to make sure that there are ways to set the constants via the argument list when using Noah MP in CCPP, and at the same time cater for other options. There are ways to do this, it's not a problem. But we cannot have each scheme define its own (and maybe slightly different) gravitational acceleration, pi, ...

To be clear, I'm talking about a subset of the physical constants defined here. In particular, grav, sb, vkc, tfrz, hsub, hvap, hfus, cwat, cice, cpair, rair, rw, and denh2o are all defined by existing CCPP host models. And by "going through the CCPP", I'm referring to physics using physical constants defined by the host model so that all physics using them can use a consistent set. Otherwise, if every scheme has its own value of gravity, specific heats, etc., this introduces a lot of inconsistencies. If other users of the NoahMP repository require the scheme to define its own physical constants, then we need to figure out a solution that works for everybody. One solution would be to define a NoahMP specific constants module. If using the CCPP, during the init phase of the scheme, we could pass in the physical constants from the host and set the constants in the NoahMP-specific constants module inside. If NOT using the CCPP, one could use pre-defined values in the NoahMP-specific module and not overwrite them in the init phase.

SMoorthi-emc · 2021-03-01T18:39:43Z

This is not good. there has to be a unified way of specifying constants. Every component cannot have it's own physical constants defined. This is bad.

climbfuji · 2021-03-01T18:46:11Z

This is not good. there has to be a unified way of specifying constants. Every component cannot have it's own physical constants defined. This is bad.

Agreed, and it will be fixed. It also violates the CCPP requirements.

barlage · 2021-03-01T18:52:17Z

Let's move this discussion to #582.

) * update ccpp-physics hash: update surface net heat fluxes for AQM coupling

ShanSunNOAA and others added 6 commits February 4, 2021 04:28

A bug fix. "flag_cice" used to be a proxy for "not 1st time step cold…

df8a923

… start" in NEMS, but is no longer the case in CMEPS. It is now replaced by "kdt>1". This will change results in all coupled models. Co-authored-by: ben.green@noaa.gov

Move Noah MP initialization to GFS_phys_time_vary_init

18062bf

Update standard name for soil layer thickness in sfc_noah_wrfv4 and s…

382d090

…fc_noah_wrfv4_interstitial

Cleanup in physics/GFS_phys_time_vary.fv3.{meta,F90}

88c321c

updating noah mp

8b0179b

Replace integer dimensions for four Noah MP arrays with variable stan…

fd4ae28

…dard names in physics/GFS_phys_time_vary.fv3.meta and physics/sfc_noahmp_drv.meta

climbfuji changed the title ~~Move Noah MP init to CCPP~~ Move Noah MP init to CCPP and update Noah MP code Feb 21, 2021

climbfuji added 2 commits February 21, 2021 06:20

Merge branch 'master' of https://github.com/NCAR/ccpp-physics into no…

15fb502

…ahmp_init_in_ccpp

Bugfix for uninitialized data in physics/radiation_surface.f, fix for…

865d085

…matting in physics/rrtmg_sw_pre.F90

This was referenced Feb 22, 2021

Move Noah MP init to CCPP and add new variables for Noah MP NOAA-EMC/fv3atm#249

Merged

Move Noah MP init to CCPP and update Noah MP regression tests, ice flux init bug fix in CCPP ufs-community/ufs-weather-model#425

Merged

climbfuji marked this pull request as ready for review February 23, 2021 03:27

climbfuji requested review from grantfirl, JulieSchramm and llpcarson as code owners February 23, 2021 03:27

climbfuji requested review from barlage and SMoorthi-emc February 23, 2021 03:28

barlage reviewed Feb 23, 2021

View reviewed changes

climbfuji added 3 commits February 25, 2021 05:26

Merge branch 'master' of https://github.com/NCAR/ccpp-physics into no…

93fa29e

…ahmp_init_in_ccpp

Fix merge conflicts in RRTMGP code

4f16a3e

Append _kind_phys to all reals that are used in the Noah MP initializ…

f2d2b12

…ation

barlage and others added 4 commits February 25, 2021 18:21

resolve some TODO in noahmp driver

98f53cf

change czil in table to more reasonable value

bfba6d7

Merge pull request #7 from barlage/fix_noahmp_todo

659a891

Fix noahmp driver TODO

Merge branch 'ini_ice_flux_20210203' of https://github.com/shansun6/c…

32b34a4

…cpp-physics into noahmp_init_in_ccpp

HelinWei-NOAA approved these changes Mar 1, 2021

View reviewed changes

barlage approved these changes Mar 1, 2021

View reviewed changes

grantfirl reviewed Mar 1, 2021

View reviewed changes

climbfuji mentioned this pull request Mar 1, 2021

Noah MP must use host model constants instead of its own ones #582

Open

climbfuji mentioned this pull request Mar 2, 2021

Initialize ice flux in CMEPSA bug fix. "flag_cice" used to be a proxy for "not 1st time step cold… #563

Merged

climbfuji merged commit fb731c5 into NCAR:master Mar 3, 2021

climbfuji mentioned this pull request Mar 3, 2021

some bug fixes for Noah MP #556

Closed

grantfirl mentioned this pull request Mar 9, 2021

update GFS_phys_time_vary.scm for NoahMP init changes and UGWP v1 #586

Merged

climbfuji deleted the noahmp_init_in_ccpp branch June 27, 2022 03:20

HelinWei-NOAA pushed a commit to HelinWei-NOAA/ccpp-physics that referenced this pull request Feb 26, 2023

Update surface net heat fluxes for AQM coupling (was NCAR#575) (NCAR#578

01f40e6

) * update ccpp-physics hash: update surface net heat fluxes for AQM coupling

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move Noah MP init to CCPP and update Noah MP code #575

Move Noah MP init to CCPP and update Noah MP code #575

climbfuji commented Feb 21, 2021 •

edited

barlage Feb 23, 2021

climbfuji Feb 25, 2021

barlage Feb 26, 2021

barlage Feb 23, 2021

HelinWei-NOAA Feb 23, 2021

climbfuji Feb 25, 2021

barlage Feb 26, 2021

barlage Feb 23, 2021

climbfuji Feb 25, 2021

barlage Feb 26, 2021

HelinWei-NOAA Feb 26, 2021

barlage Feb 26, 2021

barlage Feb 23, 2021

HelinWei-NOAA Feb 23, 2021

barlage Feb 23, 2021

climbfuji Feb 25, 2021

barlage Feb 26, 2021

HelinWei-NOAA commented Feb 25, 2021 via email

climbfuji commented Feb 25, 2021 via email

climbfuji commented Mar 1, 2021

ShanSunNOAA commented Mar 1, 2021 via email

grantfirl Mar 1, 2021

grantfirl Mar 1, 2021

climbfuji Mar 1, 2021

climbfuji Mar 1, 2021

barlage Mar 1, 2021

climbfuji Mar 1, 2021

grantfirl Mar 1, 2021 •

edited

SMoorthi-emc commented Mar 1, 2021

climbfuji commented Mar 1, 2021

barlage commented Mar 1, 2021

Move Noah MP init to CCPP and update Noah MP code #575

Move Noah MP init to CCPP and update Noah MP code #575

Conversation

climbfuji commented Feb 21, 2021 • edited

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

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

HelinWei-NOAA commented Feb 25, 2021 via email

climbfuji commented Feb 25, 2021 via email

climbfuji commented Mar 1, 2021

ShanSunNOAA commented Mar 1, 2021 via email

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

grantfirl Mar 1, 2021 • edited

Choose a reason for hiding this comment

SMoorthi-emc commented Mar 1, 2021

climbfuji commented Mar 1, 2021

barlage commented Mar 1, 2021

climbfuji commented Feb 21, 2021 •

edited

grantfirl Mar 1, 2021 •

edited