Sm jul212020

[SMoorthi-emc]

Changing pull request from SM_Jun142020 to SM_Jul212020 as the later one is up to date with the master

[climbfuji]

This PR supersedes #466 - for a description and a discussion/first review of the proposed changes see there.

[grantfirl]

Where does the hard-coded 0.33 come from? Should it be a parameter?

[grantfirl]

I reviewed for CCPP-specific stuff, not any of the science or logic changes, necessarily. I understand the need for specifying precision for hard-coded constants, but I don't necessarily understand using "r8" as the string. It was my understanding that r8 was short for real(8) or double precision. But, in comments, it was said that using r8 was to facilitate the ability to switch to single-precision as needed in the future. Wasn't that the purpose of kind_phys to begin with? If kind_phys is too much to type, fine, but could we agree on something like "kp" instead of "r8" and then be ruthlessly consistent throughout CCPP schemes?

[SMoorthi-emc]

Grant,
The reason I chose "_r8" was simply because "MGx" codes from NCAR exclusively used that notation and I did not want to change their style. I have no special affinity for that particular string. However, I don't want long string as the coding becomes messy.
CCPP folks may want to standardize all codes at some point.

[SMoorthi-emc]

Hi Grant, That parameter "0.33" represents the ratio of water density to average snow density over arctic sea ice. The same value of 0.33 was also used in "sfc_sice.f" It could be a parameter, if one wishes. I am just trying to fix a bug that exists in the current code. Moorthi

…

On Tue, Jul 28, 2020 at 5:32 PM grantfirl ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In physics/sfc_cice.f <#481 (comment)>: > hflx(i) = dtsfc(i) * tem * cpinv evap(i) = dqsfc(i) * tem * hvapi stress(i) = sqrt(dusfc(i)*dusfc(i) + dvsfc(i)*dvsfc(i)) * tem + + snwdph(i) = snowd(i) * 1000.0_kind_phys + weasd(i) = snwdph(i) * 0.33_kind_phys Where does the hard-coded 0.33 come from? Should it be a parameter? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#481 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALLVRYSSBTVFFGPMES5YTFDR547WFANCNFSM4PKQF3VA> .

-- 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: Shrinivas.Moorthi@noaa.gov Phone: (301) 683-3718 Fax: (301) 683-3718

[climbfuji]

Grant,
The reason I chose "_r8" was simply because "MGx" codes from NCAR exclusively used that notation and I did not want to change their style. I have no special affinity for that particular string. However, I don't want long string as the coding becomes messy.
CCPP folks may want to standardize all codes at some point.

Can I suggest a compromise and use _kp (short for _kind_phys) in those modules, instead of _r8? In that case

use machine, only : kp => kind_phys

and be consistent throughout the file to only use _kp. In light of what Vijay and Fanglin mentioned yesterday, it seems more forward-thinking to use _kp than _r8 (and a simple search & replace in the files you modified will do the trick). We are certainly not asking you to fix existing _r8 in the original MG code (although this is something we should clean up rather sooner than later, however gets to it first; the reason it came with _r8 is because WRF w/o CCPP doesn't know about the concept of _kp and always compiles physics in double precision). Likewise, I would advocate for modifying the code changes in this PR that turn constants like 1.0 into 1.0d0 such that they become 1.0_kp unless double precision is explicitly required, independent of the precision with which the physics are compiled. Does this make sense?

Yesterday was the first time that I heard about the requirements to compile all physics in single precision or even mix precisions per user request. The former should be relatively straightforward as long as we clean up the physics code beforehand and use _kind_phys/_kp consistently. The latter will be more difficult to implement - think about what happens to a variable that gets passed from one scheme to another - if one scheme is double precision and the other is single precision, we will end up allocating additional arrays and copying data back and forth - the benefit of a faster computation in that particular scheme will be eaten up by the data copies. Only if this is done carefully and for more or less confined blocks of physics (e.g. all radiation in double precision, all "other" physics in single precision) can I see benefits right now. But that's for another time and deserves a good strategy and careful planning (for sure, the automatic code generation and metadata concept in CCPP will help with the implementation).

[SMoorthi-emc]

I will try to change to "_kp" in some codes.
I do not recommend single precision for physics; some parts may not converge, especially if there are iteration.

[climbfuji]

Dom, bad news. It is not "kp". Even with "pk", and optimization, I got the same error.
So, I have no clue why, but it won't work (it worked ok with O0)

This mystery was solved - _kp works and will be used instead of _r8.

[climbfuji]

Superseded by #486.