Cleanup grandfathered stuff (use of constants from host model)-1st round #525
Conversation
XiaSun-Atmos
force-pushed
the
constant
branch
from
bc7067e to
ce565e7
Compare
XiaSun-Atmos
changed the title
|
@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. |
XiaSun-Atmos
requested review from
climbfuji,
grantfirl,
JulieSchramm and
llpcarson
as code owners
…210729 Update "constant" branch with latest changes in authoritative repositories
physics/m_micro.F90
Outdated
| @@ -397,6 +394,14 @@ subroutine m_micro_run( im, lm, flipv, dt_i & | |||
| ipr = 1 | |||
|
|
|||
| ! rhr8 = 1.0 | |||
|
|
|||
| onebcp=one/cp | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
It would be great if it can be done. Otherwise, we can do it another time.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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!
|
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.
|
physics/gscond.f
Outdated
| &, cpr=cp*r, rcp=h1/cp) | ||
|
|
||
| parameter (h1=1.e0, d00=0.e0 & | ||
| &, epsq=2.e-12) |
There was a problem hiding this comment.
How about removing unnecessary blanks?
physics/gscond.f
Outdated
| @@ -158,6 +157,12 @@ subroutine zhaocarr_gscond_run (im,km,dt,dtf,prsl,ps,q,clw1 & | |||
| enddo | |||
| !-----------------prepare constants for later uses----------------- | |||
| ! | |||
| elwv=hvap | |||
| eliv=hvap+hfus | |||
| r=rd | |||
There was a problem hiding this comment.
I recommend adding blanks on either side of = sign and aligh = sign
physics/m_micro.F90
Outdated
| onebcp=one/cp | ||
| onebg=one/grav | ||
| kapa=rgas*onebcp | ||
| cpbg=cp/grav |
There was a problem hiding this comment.
Again, the same recommendation about adding blanks before and after the = sign
Constant branch update from Dom 2021/08/02
|
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
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.
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
Description
This PR cleans up the constants that are defined in schemes
-rainmin (passed)
-karman constant (passed)
...
use physconsfrom schemesRelated issues
NCAR/ccpp-physics #104 #245
Testing
See ufs-community/ufs-weather-model#726
Dependencies
#525
NOAA-EMC/fv3atm#360
ufs-community/ufs-weather-model#726