Sm jul302020 #486

SMoorthi-emc · 2020-08-09T02:03:44Z

I would like this pull request to replace the pull request "Sm jul232020 #149". this is the latest version after merging with rrtmgp update.

…into SM_Mar032020

… Whole Atmosphere Model

…e physics routines

…h nsst model

climbfuji

One tiny request for change, otherwise this looks good to me!

climbfuji · 2020-08-10T13:09:42Z

physics/GFS_rrtmg_pre.F90

@@ -598,7 +590,7 @@ subroutine GFS_rrtmg_pre_run (Model, Grid, Sfcprop, Statein,   & ! input
                qc_mp (i,k) = tracer1(i,k,ntcw)/(1.-qvs)
                qi_mp (i,k) = tracer1(i,k,ntiw)/(1.-qvs)
                qs_mp (i,k) = tracer1(i,k,ntsw)/(1.-qvs)
-                nc_mp (i,k) = nt_c*orho(i,k1)
+                nc_mp (i,k) = nt_c*orho(i,k)


Thanks for fixing this bug.

climbfuji · 2020-08-10T13:13:13Z

physics/GFS_suite_interstitial.F90

+              endif
+            enddo
+          endif
+        endif


@SMoorthi-emc @dustinswales Do you agree that the block in lines 297-333 is a repetition of the block in lines 257-295 (which is to be executed if use_GP_jacobian, an RRTMGP-specific flag, is .false.)? If so, please delete lines 297-333 and we can close issue #485 when the code is merged.

Dom, I have removed these duplicate code, which happened while merging.
I guess RRTMGP does not take into account fractional grid or ice/water fractions.

Thanks for doing this. Yes, I agree that RRTMGP has not been implemented for or tested with the fractional grid or ice/water fractions. Something to tackle for @dustinswales in the future.

@SMoorthi-emc @climbfuji
Thanks for removing this excess code.
In GP the surface state is implicit within the Jacobian of the upwelling/downwelling LW, so there's no need to condition the flux adjustment factor on the surface.

Dustin,
Does RRTMGP takes into account fluxes over land, water and ice within a grid box through Jacobian?

@SMoorthi-emc
Yes, this is my understanding.
@RobertPincus Can you chime in here?

@SMoorthi-emc @dustinswales RRTMGP assumes uniform but spectrally-varying surfaces under each grid cell for the radiation calculation. Does that answer the question?

No, I don't understand how you know the temp over land, ice, and water and ice fraction spectrally.

@dustinswales Can you point @SMoorthi-emc to where the surface temperature is set for RRTMGP ?

@SMoorthi-emc
In GFS_surface_composites.F90/.meta the tsfc/"surface_skin_temperature" is conditioned on surface type, land/ice/ocean.

climbfuji · 2020-08-10T13:15:09Z

physics/GFS_surface_composites.meta

@@ -604,7 +613,7 @@
 [min_lakeice]
  standard_name = lake_ice_minimum
  long_name = minimum lake ice value
-  units = ???
+  units = frac


climbfuji · 2020-08-10T13:18:49Z

physics/micro_mg3_0.F90

@@ -324,7 +324,7 @@ subroutine micro_mg_init(                                         &

  !-----------------------------------------------------------------------

-  dcs       = micro_mg_dcs * 1.0e-6
+  dcs       = micro_mg_dcs * 1.0e-6_r8


I think it is ok to use _r8 here, given that the existing definition for the floating point kind is r8. Suggests that this was implemented with double precision in mind and signals that anyone trying to run this in single precision need to be cautious.

I personally still think that is is confusing to use _r8 when it is customary to reserve that for double precision. If one pays attention, they can see that it is set to kind_phys above, but as you know, not everyone is so observant. I also think that we are taking a leap assuming that your interpretation (that leaving in r8 sends a message to future developers) is obvious. A comment about it where r8 is set to kind_phys could clarify things though: "Although this scheme was developed with double precision in mind, the real kind is set to a variable to be consistent with other physics and to be easily modifiable in one place" or something like that.

climbfuji · 2020-08-10T14:15:58Z

physics/GFS_suite_interstitial.F90

+              endif
+            enddo
+          endif
+        endif


Thanks for doing this. Yes, I agree that RRTMGP has not been implemented for or tested with the fractional grid or ice/water fractions. Something to tackle for @dustinswales in the future.

ShanSunNOAA · 2020-08-10T14:19:31Z

physics/GFS_MP_generic.F90

-            srflag(i) = 0.                     ! clu: default srflag as 'rain' (i.e. 0)
-            if (tsfc(i) >= 273.15) then
+            srflag(i) = zero                     ! clu: default srflag as 'rain' (i.e. 0)
+            if (tsfc(i) >= 273.15_kind_phys) then


There is one parameter defined in GFS_typedefs.F90: real(kind=kind_phys) :: tcr = 273.16d0

Should tcr be used here?

Good catch, @shansun6. I see the local variable is 273.15, tcr is 273.16. Further down, t850 is compared to a local 273.16 value - @SMoorthi-emc wouldn't it be better to follow @shansun6 suggestion and use tcr in both places? Any reason why the two values differ by one hundreds of a degree Kelvin?

Will this change the answer for the regression test baselines?

Shan, I think there is a reason why this number is 273,15. I did not change this number, only added "kind_phys".

ShanSunNOAA · 2020-08-10T14:21:19Z

physics/GFS_MP_generic.F90

+            if (rain(i) > rainmin) then
+              tem1 = max(zero, (rain(i)-rainc(i))) * sr(i)
+              tem2 = one / rain(i)
+              if (t850(i) > 273.16_kind_phys) then


Similar here: should line 313 be replaced by if (t850(i) > tcr)?

This could be changed, but I would leave for the next edition.

Fine by me.

ShanSunNOAA · 2020-08-10T15:02:26Z

physics/GFS_surface_composites.F90

              else
-                cice(i) = zero
+                cice(i)        = zero
+                flag_cice(i)   = .false.


Line 85 is not necessary because of line 79.

I am not sure about that. It is just resetting "flag_cice" if it is "true" and "cice < min_seaice"

Sorry - spoke too soon. I took it back. Line 85 should be there. -Shan

ShanSunNOAA

Changes made to routines that I am familiar with are all good. Very nice work, Moorthi! -Shan

grantfirl · 2020-08-10T15:42:20Z

physics/sfc_drv.f

-      real(kind=kind_phys), parameter :: a4      = 35.86
+      real(kind=kind_phys), parameter :: zero    = 0.0_kind_phys
+      real(kind=kind_phys), parameter :: one     = 1.0_kind_phys
+      real(kind=kind_phys), parameter :: rhoh2o  = 1000.0_kind_phys


This constant already exists physcons as rhowater.

All I did was to add kind_phys to what already was there. Yes, we could do a lot more cleanup.
When I was doing some of these changes, the reason was to get restart reproducibility, for which precision was important.

grantfirl · 2020-08-10T15:44:53Z

physics/sfc_drv.f

+      real(kind=kind_phys), parameter :: one     = 1.0_kind_phys
+      real(kind=kind_phys), parameter :: rhoh2o  = 1000.0_kind_phys
+      real(kind=kind_phys), parameter :: a2      = 17.2693882_kind_phys
+      real(kind=kind_phys), parameter :: a3      = 273.16_kind_phys


As already pointed out, 273.15 and 273.16 both already exist in physcons as con_t0c and con_ttp, respectively.

grantfirl · 2020-08-10T15:47:57Z

physics/wv_saturation.F

@@ -9,12 +9,12 @@
 !! This module contain some utility functions for saturation vapor pressure.
      module wv_saturation
 #ifdef GEOS5
-       use MAPL_ConstantsMod, r8 => MAPL_R8
+       use MAPL_ConstantsMod, kp => MAPL_kp


Is this going to cause an issue if GEOS5 is ever set?

Good catch Grant. this happened becasue I changed "_r8" to "_kp".
I don't think there is any "MAPL_kp". I could change this back now or leave it for the future. We are really not using this option.

I would suggest to either change it back to avoid that someone runs into an issue in a few years time and the knowledge about this issue-to-fix is gone, or to create an issue on the same ccpp-physics GitHub repo so that the next commit can fix this.

ShanSunNOAA · 2020-08-10T15:49:26Z

Hi Moorthi, I realized this myself before seeing your email - sorry about the false alarm. This is actually a fairly important line. Shan

…

On Mon, Aug 10, 2020 at 9:11 AM SMoorthi-emc ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In physics/GFS_surface_composites.F90 <#486 (comment)>: > else - cice(i) = zero + cice(i) = zero + flag_cice(i) = .false. I am not sure about that. It is just resetting "flag_cice" if it is "true" and "cice < min_seaice" — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#486 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALORMVQTGO3UDNJHSXE5SELSAAE23ANCNFSM4PY6EKFA> .

grantfirl

IMO, it's good to go as-is if it passes RTs, although I left some comments that could be addressed in the future. Most of them are related to code that existed before this PR, but were just made more obvious by the changes in this PR.

DusanJovic-NOAA · 2020-08-10T16:38:55Z

physics/wv_saturation.F

@@ -9,7 +9,7 @@
 !! This module contain some utility functions for saturation vapor pressure.
      module wv_saturation
 #ifdef GEOS5
-       use MAPL_ConstantsMod, kp => MAPL_kp
+       use MAPL_ConstantsMod, kp => MAPL_R8


Shouldn't line 17 (use machine, only : kp => kind_phys) be inside #ifdef NEMS_GFS? Like, for example, in aer_cloud.F or cldwat2m_micro.F

SMoorthi-emc added 30 commits January 10, 2020 17:44

Merge remote-tracking branch 'ncarphy/master'

262d1d9

updating cycle to distingush lakes from ocean

99ac1a0

minot changes in sfcsub

f8eb82c

Merge remote-tracking branch 'upstream/master'

1e0a583

Merge branch 'master' of https://github.com/SMoorthi-EMC/ccpp-physics …

fd0bc5a

…into SM_Mar032020

correcting errors in ccpp when levr < levs; this fix is essential for…

ab6cb5a

… Whole Atmosphere Model

a bug fix

3ba810b

after merging with ncar master and some fix to RAS

465f019

Merge remote-tracking branch 'upstream/master'

74de553

Merge branch 'master' into SM_Apr182020

861e4eb

after merging with NCAR/ccpp-physics/master and some bug fixes in som…

f1c24fb

…e physics routines

removing some unneeded do loops in sfc_drv.f - results reproduce

c316f79

updating precision of constants in several physics routines

5953c52

adding _kind_phys to constants in some physics routines

e19953d

updating to fix a potential snow bug

f85730d

fixing some comment lines

0241449

some minor change with same result

4694c00

update consistent with ipd

e216116

Merge remote-tracking branch 'upstream/master' into SM_Apr282020

13c3ee3

testing an alternate option in ccpp

cdac822

some additional updates to the code

d61ecbe

some updates for several routines

f245b7a

Merge remote-tracking branch 'origin/SM_Apr182020' into SM_Apr282020

8ff5aa5

Merge remote-tracking branch 'upstream/master'

9d0808d

after merging with ccpp master

2b34488

removed tisfcin_cpl and tseain_cpl as they are not needed

e146375

minor update to surface_composites

513cb29

merged with ccpp-physics, updated some code, tested coupled model wit…

bbf56bb

…h nsst model

change 633.0 to 622.0_r8

3cdcdaa

updating nst model coupled with ocean model

4c08f73

SMoorthi-emc added 4 commits July 30, 2020 17:39

fixing one file

e530096

fixing issues related to z0 and restart reproducibility

4b11e84

adding _kp in place of -r8 in some routines

04f660b

after merging with ccpp-physics updated with rrtmgp

b9ff5e7

SMoorthi-emc requested review from climbfuji, grantfirl, JulieSchramm and llpcarson as code owners August 9, 2020 02:03

climbfuji requested changes Aug 10, 2020

View reviewed changes

climbfuji requested a review from ShanSunNOAA August 10, 2020 13:39

removingduplicate lines in GFS_suite_interstitial.F90

14aa89b

climbfuji approved these changes Aug 10, 2020

View reviewed changes

ShanSunNOAA reviewed Aug 10, 2020

View reviewed changes

fixing a typo in sfcsub.F

d3dcae7

ShanSunNOAA reviewed Aug 10, 2020

View reviewed changes

ShanSunNOAA approved these changes Aug 10, 2020

View reviewed changes

grantfirl reviewed Aug 10, 2020

View reviewed changes

grantfirl approved these changes Aug 10, 2020

View reviewed changes

reverting MAPL_kp back to MAPL_R8

c3ddc93

DusanJovic-NOAA reviewed Aug 10, 2020

View reviewed changes

moving definition of kp inside ifdef in wv_saturation.F

f3b797f

climbfuji mentioned this pull request Aug 11, 2020

Duplication of code in GFS_suite_interstitial.F90 #485

Closed

SMoorthi-emc mentioned this pull request Aug 11, 2020

creating ufs-s2s-model branch for PR ufs-community/ufs-s2s-model#161

Merged

climbfuji mentioned this pull request Aug 11, 2020

Sm jul212020 #481

Closed

climbfuji merged commit 09c4ee3 into NCAR:master Aug 13, 2020

grantfirl mentioned this pull request Aug 18, 2020

Update master branch to work with ccpp-physics master NCAR/ccpp-scm#196

Merged

Sm jul302020 #486

Sm jul302020 #486

Conversation

SMoorthi-emc commented Aug 9, 2020

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

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

ShanSunNOAA 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

ShanSunNOAA commented Aug 10, 2020 via email

grantfirl left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grantfirl left a comment •

edited

Loading