Implement positive-definite TVD methods for tracer transport in EDMF-TKE PBL schemes and SAMF convective schemes #754

rmontuoro · 2021-10-06T21:26:55Z

This PR introduces positive-definite Total Variation Diminishing (TVD) mass-flux methods recently developed at NOAA/EMC (Jongil Han) to improve tracer transport in EDMF-TKE PBL schemes and SAMF cumulus convective parameterizations.

The included code changes have been tested on Hera/Intel and Orion/Intel platforms.

Please see issue #753 for details. Results of MJO and jet stream diagnostic tests are available in this document.

mass-flux schemes for tracer diffusion in the PBL and advection in cumulus convection schemes. Author: Jongil Han <jongil.han@noaa.gov>

…array.

…vailable.

climbfuji

These changes are not easy to review, therefore thank you for the link to the document. To me it looks a bit like mixed bag, with a slight improvement for the tvd runs over the mid-latitude NH (PAC skill section). I wanted to add @JongilHan66 as reviewer, but I cannot since he has not yet accepted the invitation to collaborate on ccpp-physics.

From a CCPP coding point of view, the code changes are fine, although it wouldn't hurt to use more self-explanatory variable names internally in the schemes and add a little more documentation.

Just to confirm, this will change the answer for both the non-aerosol runs and thee aerosol runs, correct?

SMoorthi-emc · 2021-10-08T13:40:56Z

These TVD related changes are made by Jongil Han. Yes, it will change results.

JongilHan66

The changes look good.

climbfuji · 2021-11-05T14:44:15Z

@grantfirl @llpcarson Can you please review this PR? We are hoping to merge it next Monday. @SMoorthi-emc @yangfanglin I want to confirm with you that you are ok with the proposed changes. Thanks!

climbfuji · 2021-11-05T14:49:11Z

@rmontuoro I see there is a merge conflict for physics/satmedmfvdifq.meta. I guess you will have to pull in the latest updates from the authoritative repositories for all your PRs (note the upcoming ufs-weather-model HAFS doc update for the top-level repository, fv3atm and ccpp-physics won't change before your PR).

yangfanglin · 2021-11-05T15:30:32Z

These changes are important not only for the updated schemes themselves but also for other physics processes like microphysics and atmospheric compositions. The developers have rigorously tested the impact on sciences. So I approve this PR if there are no code merging issues.

…

On Fri, Nov 5, 2021 at 10:44 AM Dom Heinzeller ***@***.***> wrote: @grantfirl <https://github.com/grantfirl> @llpcarson <https://github.com/llpcarson> Can you please review this PR? We are hoping to merge it next Monday. @SMoorthi-emc <https://github.com/SMoorthi-emc> @yangfanglin <https://github.com/yangfanglin> I want to confirm with you that you are ok with the proposed changes. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#754 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKY5N2KBJM2OGQ2TRFXBW6DUKP3VVANCNFSM5FPYHL5Q> . 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&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

-- *Fanglin Yang, Ph.D.* *Chief, Model Physics Group* *Modeling and Data Assimilation Branch* *NOAA/NWS/NCEP Environmental Modeling Center* *https://www.emc.ncep.noaa.gov/gmb/wx24fy/fyang/ <https://www.emc.ncep.noaa.gov/gmb/wx24fy/fyang/>*

SMoorthi-emc · 2021-11-05T17:28:39Z

physics/samfdeepcnv.f

+     &                     (ctro(i,k,kk)+ctro(i,k-1,kk)))/factor
+                   chem_c(i,k,n)=fscav(n)*ecko(i,k,kk)
+                   tem=chem_c(i,k,n)/(1.+c0t(i,k)*dz)
+                   chem_pw(i,k,n)=c0t(i,k)*dz*tem*eta(i,k-1)


I would have put blanks arout "=".

SMoorthi-emc · 2021-11-05T17:30:42Z

physics/samfdeepcnv.f

@@ -2009,8 +2052,8 @@ subroutine samfdeepcnv_run (im,km,itc,ntc,cliq,cp,cvap,           &
          dp = 1000. * del(i,1)
          dellah(i,1) = edto(i) * etad(i,1) * (hcdo(i,1)
     &                   - heo(i,1)) * grav / dp
-          dellaq(i,1) = edto(i) * etad(i,1) * (qrcdo(i,1)
-     &                   - qo(i,1)) * grav / dp
+          dellaq(i,1) = edto(i) * etad(i,1) * qrcdo(i,1)


If you use dpi = grav / (1000.0*del(i,1), then you could eliminate multiple divides

Agree.

@climbfuji - do we still have time to implement these suggestions?

Absolutely!

SMoorthi-emc

There are lots of changes in the code and I assume Jongil has checked it well.
I have not verified everything.

rmontuoro · 2021-11-05T17:40:57Z

@rmontuoro I see there is a merge conflict for physics/satmedmfvdifq.meta. I guess you will have to pull in the latest updates from the authoritative repositories for all your PRs (note the upcoming ufs-weather-model HAFS doc update for the top-level repository, fv3atm and ccpp-physics won't change before your PR).

Just updated all PRs to the latest code revisions.

to increase runtime performance as suggested by S. Moorthi.

rmontuoro · 2021-11-08T19:01:23Z

@SMoorthi-emc - I've implemented your recommendations and added a few more changes to improve performance for the deep & shallow convective schemes.

@SMoorthi-emc and @JongilHan66 - could you please take a look at the changes and let me know if you agree with them?

SMoorthi-emc · 2021-11-08T19:03:59Z

physics/samfdeepcnv.f

+              dt = to(i,k)
+              dg = gamma
+              dh = heso(i,k)
+              dz = -1.*(zo(i,k+1)-zo(i,k))


Not trying to be picky, but we really don't need this multiplication by "-1".

it could be just a "-" or you could reverse the two items inside "( )"

Agree! However, this should not add compute time (the compiler should recognize this).

@SMoorthi-emc - I've implemented your suggested changes.

SMoorthi-emc · 2021-11-08T19:06:35Z

physics/samfdeepcnv.f

+              dt = to(i,k)
+              dg = gamma
+              dh = heso(i,k)
+              dz = -1.*(zo(i,k+1)-zo(i,k))


it could be just a "-" or you could reverse the two items inside "( )"

SMoorthi-emc · 2021-11-08T19:31:31Z

great.

…

On Mon, Nov 8, 2021 at 2:28 PM Raffaele Montuoro ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In physics/samfdeepcnv.f <#754 (comment)>: > @@ -1992,18 +1992,18 @@ subroutine samfdeepcnv_run (im,km,itc,ntc,cliq,cp,cvap, & do i = 1, im if (cnvflg(i) .and. k < jmin(i)) then gamma = el2orc * qeso(i,k) / to(i,k)**2 - dhh=hcdo(i,k) - dt=to(i,k) - dg=gamma - dh=heso(i,k) - dz=-1.*(zo(i,k+1)-zo(i,k)) + dhh = hcdo(i,k) + dt = to(i,k) + dg = gamma + dh = heso(i,k) + dz = -1.*(zo(i,k+1)-zo(i,k)) @SMoorthi-emc <https://github.com/SMoorthi-emc> - I've implemented your suggested changes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#754 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ALLVRYQVI4E3EMKJKHTAMDLULAQFVANCNFSM5FPYHL5Q> . 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&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

-- 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

junwang-noaa · 2021-11-08T19:32:47Z

@JongilHan66 Are the code changes looking good to you?

JongilHan66 · 2021-11-08T20:57:11Z

The changes which I agree on look good and would save some computing time.

…chemes (NCAR#408) This PR implements changes supporting TVD methods for PBL and cumulus convective parameterizations introduced by ccpp/physics PR NCAR#754 (Jongil Han, NOAA/EMC).

rmontuoro added 5 commits October 5, 2021 22:53

Implement positive-definite Total Variation Diminishing (TVD)

ed3768b

mass-flux schemes for tracer diffusion in the PBL and advection in cumulus convection schemes. Author: Jongil Han <jongil.han@noaa.gov>

Remove unused input argument for ozone tracer index.

c1f0706

Use proper index for rain water tracer in vertically-diffused tracer …

2758810

…array.

Adjust negative mixing ratios for rain water only if such tracer is a…

9a7131f

…vailable.

Improve test for rain water tracer.

7d080b8

rmontuoro requested review from climbfuji, grantfirl and llpcarson as code owners October 6, 2021 21:26

This was referenced Oct 6, 2021

Support implementation of TVD methods in PBL and cumulus convective schemes NOAA-EMC/fv3atm#408

Merged

Introduce positive-definite TVD methods for PBL and cumulus convective schemes ufs-community/ufs-weather-model#858

Merged

climbfuji requested review from SMoorthi-emc and yangfanglin October 8, 2021 13:28

climbfuji reviewed Oct 8, 2021

View reviewed changes

climbfuji requested a review from JongilHan66 October 30, 2021 01:37

JongilHan66 approved these changes Nov 1, 2021

View reviewed changes

rmontuoro added 2 commits November 5, 2021 16:37

Merge branch 'main' into feature/tvd

975afe7

Remove remaining optional arguments from metadata.

0014964

SMoorthi-emc reviewed Nov 5, 2021

View reviewed changes

SMoorthi-emc approved these changes Nov 5, 2021

View reviewed changes

rmontuoro added 2 commits November 5, 2021 20:04

Adding spaces to improve code readability as suggested by S. Moorthi.

1455ddd

Precompute a few quantities in SAMF deep & shallow convective schemes

6bc90ec

to increase runtime performance as suggested by S. Moorthi.

SMoorthi-emc reviewed Nov 8, 2021

View reviewed changes

SMoorthi-emc approved these changes Nov 8, 2021

View reviewed changes

Remove multiplication by -1.

3c00f02

climbfuji approved these changes Nov 9, 2021

View reviewed changes

climbfuji merged commit d189642 into NCAR:main Nov 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement positive-definite TVD methods for tracer transport in EDMF-TKE PBL schemes and SAMF convective schemes #754

Implement positive-definite TVD methods for tracer transport in EDMF-TKE PBL schemes and SAMF convective schemes #754

rmontuoro commented Oct 6, 2021

climbfuji left a comment •

edited

Loading

SMoorthi-emc commented Oct 8, 2021

JongilHan66 left a comment

climbfuji commented Nov 5, 2021

climbfuji commented Nov 5, 2021

yangfanglin commented Nov 5, 2021 via email

SMoorthi-emc Nov 5, 2021

rmontuoro Nov 5, 2021

SMoorthi-emc Nov 5, 2021

rmontuoro Nov 5, 2021

climbfuji Nov 5, 2021

SMoorthi-emc left a comment

rmontuoro commented Nov 5, 2021

rmontuoro commented Nov 8, 2021

SMoorthi-emc Nov 8, 2021

SMoorthi-emc Nov 8, 2021

rmontuoro Nov 8, 2021 •

edited

Loading

rmontuoro Nov 8, 2021

SMoorthi-emc Nov 8, 2021

SMoorthi-emc commented Nov 8, 2021 via email

junwang-noaa commented Nov 8, 2021

JongilHan66 commented Nov 8, 2021

Implement positive-definite TVD methods for tracer transport in EDMF-TKE PBL schemes and SAMF convective schemes #754

Implement positive-definite TVD methods for tracer transport in EDMF-TKE PBL schemes and SAMF convective schemes #754

Conversation

rmontuoro commented Oct 6, 2021

climbfuji left a comment • edited Loading

Choose a reason for hiding this comment

SMoorthi-emc commented Oct 8, 2021

JongilHan66 left a comment

Choose a reason for hiding this comment

climbfuji commented Nov 5, 2021

climbfuji commented Nov 5, 2021

yangfanglin commented Nov 5, 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

SMoorthi-emc left a comment

Choose a reason for hiding this comment

rmontuoro commented Nov 5, 2021

rmontuoro commented Nov 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmontuoro Nov 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SMoorthi-emc commented Nov 8, 2021 via email

junwang-noaa commented Nov 8, 2021

JongilHan66 commented Nov 8, 2021

climbfuji left a comment •

edited

Loading

rmontuoro Nov 8, 2021 •

edited

Loading