Cleanup grandfathered stuff (use of constants from host model)-1st round by XiaSun-Atmos · Pull Request #525 · NCAR/ccpp-physics

XiaSun-Atmos · 2020-12-04T23:20:43Z

Description

This PR cleans up the constants that are defined in schemes

move constants defined in schemes to the argument list
-rainmin (passed)
-karman constant (passed)
...
remove use physcons from schemes

Related issues

NCAR/ccpp-physics #104 #245

Testing

See ufs-community/ufs-weather-model#726

Dependencies

#525
NOAA-EMC/ufsatm#360
ufs-community/ufs-weather-model#726

XiaSun-Atmos · 2021-01-12T02:34:34Z

@climbfuji The 1st round clean-up is finished and passed RT tests on Hera using intel. The 2nd round will focus on those internal dependencies.

…into HEAD

…210729 Update "constant" branch with latest changes in authoritative repositories

SMoorthi-emc · 2021-07-30T00:09:04Z


 !      rhr8 = 1.0
+
+       onebcp=one/cp


When these constants are in a parameter statement, they are computed once.
When they are a fortran statement like here they are computed every time step - somewhat inefficient.

Agreed. One way to improve this is to make these variables module variables and use the _init routine of a scheme to compute them once at the beginning of the run. We have done that for some schemes, but not for all. Do you want me to make the change for m_micro?

It would be great if it can be done. Otherwise, we can do it another time.

@SMoorthi-emc I made the change. Before the change, both PROD and DEBUG tests gave b4b identical results to the old code. After making the change, the DEBUG tests (control_csawmg_debug, control_csawmgt_debug)are still bit-for-bit, but the PROD tests are different (control_csawmg, control_csawmgt). I will check if there is a way to rewrite the code to retain b4b identical results in PROD mode, but not sure if it is possible.

Thanks Dom.
If the order of computation changed, bit for bit may reproducibility may not happen. As long as the coding is correct, I have no problem with that.

Got it. Since this PR is supposed to be a quick one w/o changing the baseline, I suggest to defer this change to later?

EDIT: Scrap that. Let's do it right away and create new baselines.

@SMoorthi-emc I made the changes as requested, including the formatting changes in gscond.f. Now the tests csawmg and csawmgt give b4b identical results compared to the existing baseline in DEBUG mode, but not in PROD mode. This means the code is correct, the Intel compiler is just optimizing the code differently. Please take a look. The commit that contains all these changes, based on the existing PR, is 4fa6a3b. Thanks!

SMoorthi-emc · 2021-07-30T20:18:46Z

Ok

…

Sent from my iPhone

On Jul 30, 2021, at 3:40 PM, Dom Heinzeller ***@***.***> wrote: @climbfuji commented on this pull request. In physics/m_micro.F90: > @@ -397,6 +394,14 @@ subroutine m_micro_run( im, lm, flipv, dt_i & ipr = 1 ! rhr8 = 1.0 + + onebcp=one/cp Got it. Since this PR is supposed to be a quick one w/o changing the baseline, I suggest to defer this change to later? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

SMoorthi-emc · 2021-08-01T00:25:58Z

-     &,          cpr=cp*r,      rcp=h1/cp)
+
+      parameter (h1=1.e0,       d00=0.e0                                &           
+     &,          epsq=2.e-12)                               


How about removing unnecessary blanks?

SMoorthi-emc · 2021-08-01T00:27:16Z

 !
+      elwv=hvap
+      eliv=hvap+hfus
+      r=rd


I recommend adding blanks on either side of = sign and aligh = sign

SMoorthi-emc · 2021-08-01T00:28:32Z

+       onebcp=one/cp
+       onebg=one/grav
+       kapa=rgas*onebcp
+       cpbg=cp/grav


Again, the same recommendation about adding blanks before and after the = sign

Constant branch update from Dom 2021/08/02

SMoorthi-emc · 2021-08-03T14:59:34Z

Thanks Dom, Looks good. Moorthi

…

On Tue, Aug 3, 2021 at 10:54 AM Dom Heinzeller ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In physics/m_micro.F90 <#525 (comment)>: > @@ -397,6 +394,14 @@ subroutine m_micro_run( im, lm, flipv, dt_i & ipr = 1 ! rhr8 = 1.0 + + onebcp=one/cp @SMoorthi-emc <https://github.com/SMoorthi-emc> I made the changes as requested, including the formatting changes in gscond.f. Now the tests csawmg and csawmgt give b4b identical results compared to the existing baseline in DEBUG mode, but not in PROD mode. This means the code is correct, the Intel compiler is just optimizing the code differently. Please take a look. The commit that contains all these changes, based on the existing PR, is 4fa6a3b <4fa6a3b>. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#525 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALLVRYXK7IVWIIXU3VE55HTT277INANCNFSM4UOCOITA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email> .

-- Dr. Shrinivas Moorthi Research Meteorologist Modeling and Data Assimilation Branch Environmental Modeling Center / National Centers for Environmental Prediction 5830 University Research Court - (W/NP23), College Park MD 20740 USA Tel: (301)683-3718 e-mail: ***@***.*** Phone: (301) 683-3718 Fax: (301) 683-3718

Constants update from NCAR main 2021/08/05

Constants update from main 2021/08/10

llpcarson

looks good to me, approved

grantfirl

Looks OK to me.

Add metadata for three constants that are required for the changes in PR NCAR/ccpp-physics#525. The motivation behind these changes is that physical parameterizations should receive constants (e.g. gravitational acceleration) from the host model via the argument list instead of importing them from some Fortran module or defining them locally. This ensures consistency and enhances interoperability.

NCAR#525) * Added coupling of GOCART aerosols with radiation related to issue#899 in NCAR/ccpp-physics * Updated ccpp/physics to include Barry Baker's updates for wet deposition in the Thompson scheme * Updated physics/rte-rrtmgp with the latest commit in ccpp/physics * Update ccpp/physics to include the updates of precipitation fluxes outputs in the Thompson microphysics scheme * Updated ccpp/physics for fixing a bug in mp_thompson.F90

XiaSun-Atmos added 2 commits December 4, 2020 19:13

move constant rainmin for gfdl cloud microphys to argument list

760be1f

move constant of rainmax from other two schemes

6500702

XiaSun-Atmos added the work in progress label Dec 5, 2020

XiaSun-Atmos added 17 commits December 7, 2020 17:29

clean up von karman (karman=0.4); add karman to physcon

94e179f

minor fix

fd6bb5e

clean use physcons in cu_gf_driver

69d33b9

clean use physcon in dcyc2

3215b4b

clean up physcon in getprs_fv3

a730494

amend dcyc2

149d8f6

clean use physcons in gscond

3f35140

clean use_physcons in m_micro

4661b73

clean use_physcons in module_MYNNPBL_wrapper

436b410

clean up use_physcons in module_SGSCloud_RadPre

f695abb

clean up use_physcons in precpd

719338d

Milestone, several amends and rt tests passed

57958be

duplicate meta entry, remove latvap not used in _run

51e8c01

move calculation to the execution section

ec19a7e

move calculation to the execution section for module_MYNNPBL_wrapper

ba72347

remove physcon; use public for tranfer cons

720607b

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

ce565e7

XiaSun-Atmos force-pushed the constant branch from bc7067e to ce565e7 Compare January 11, 2021 23:50

XiaSun-Atmos changed the title ~~Cleanup grandfathered stuff (use of constants from host model)~~ Cleanup grandfathered stuff (use of constants from host model)-1st round Jan 12, 2021

revert changes to radlw_main for 2nd round clean up

501251e

XiaSun-Atmos marked this pull request as ready for review January 12, 2021 02:34

XiaSun-Atmos requested review from JulieSchramm, climbfuji, grantfirl and llpcarson as code owners January 12, 2021 02:34

XiaSun-Atmos removed the work in progress label Jan 12, 2021

climbfuji and others added 3 commits July 29, 2021 09:25

Merge branch 'constant' of https://github.com/XiaSun-NOAA/ccpp-physics …

a00a5aa

…into HEAD

Fix merge conflicts

3ba9d4d

Merge pull request #2 from climbfuji/constant_xia_update_from_main_20…

859e48e

…210729 Update "constant" branch with latest changes in authoritative repositories

This was referenced Jul 29, 2021

Cleanup grandfathered-in constants in CCPP - round 1 NOAA-EMC/ufsatm#360

Merged

Cleanup grandfathered-in constants in CCPP - round 1 ufs-community/ufs-weather-model#726

Merged

climbfuji requested a review from SMoorthi-emc July 29, 2021 22:57

SMoorthi-emc reviewed Jul 30, 2021

View reviewed changes

SMoorthi-emc reviewed Aug 1, 2021

View reviewed changes

SMoorthi-emc approved these changes Aug 1, 2021

View reviewed changes

climbfuji and others added 2 commits August 2, 2021 09:41

Initialize MG constants in init phase, adjust formatting in gscond.f

4fa6a3b

Merge pull request #3 from climbfuji/constant__update_dom_20210802

48e0837

Constant branch update from Dom 2021/08/02

climbfuji and others added 4 commits August 5, 2021 08:38

Merge branch 'main' of https://github.com/NCAR/ccpp-physics into HEAD

b80b69b

Merge pull request #4 from climbfuji/constants_update_dom_20210805

0ce2100

Constants update from NCAR main 2021/08/05

Merge branch 'main' of https://github.com/NCAR/ccpp-physics into HEAD

21f33f1

Merge pull request #5 from climbfuji/constants_update_dom_20210810

ba50531

Constants update from main 2021/08/10

llpcarson approved these changes Aug 11, 2021

View reviewed changes

grantfirl approved these changes Aug 11, 2021

View reviewed changes

climbfuji approved these changes Aug 11, 2021

View reviewed changes

climbfuji merged commit aaa4792 into NCAR:main Aug 11, 2021

Conversation

XiaSun-Atmos commented Dec 4, 2020 • edited by climbfuji Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Testing

Dependencies

Uh oh!

XiaSun-Atmos commented Jan 12, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

climbfuji Jul 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SMoorthi-emc commented Jul 30, 2021 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SMoorthi-emc commented Aug 3, 2021 via email

Uh oh!

llpcarson left a comment

Choose a reason for hiding this comment

Uh oh!

grantfirl left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

XiaSun-Atmos commented Dec 4, 2020 •

edited by climbfuji

Loading

climbfuji Jul 30, 2021 •

edited

Loading