Remove inconsistencies in computation of air density with Thompson MP #79

tanyasmirnova · 2021-02-23T02:15:10Z

These changes promote a consistent use of air density and units of water friendly aerosol in the wrappers around Thompson MP and the internal code in module_mp_thompson.F90.

Replaced dry-air density with the moist-air density for consistency with Thompson MP.
Fixed units for water friendly aerosols in the computation of number concentration of liquid water.

Associated PRs:

#79
NOAA-GSL/fv3atm#74
NOAA-GSL/ufs-weather-model#64

For regression testing, see NOAA-GSL/ufs-weather-model#64

computation of number concentration of liquid water. 2. Replaced dry-air density with the moist-air density for consistency with Thompson MP.

climbfuji · 2021-02-23T03:32:56Z

@tanyasmirnova there are conflicts reported, presumaby because you created the PR based on code before the gsl/develop update from master today.

climbfuji · 2021-02-23T03:34:39Z

Note that I have not yet merged NOAA-GSL/fv3atm#73 and NOAA-GSL/ufs-weather-model#63 (happening in the next few minutes), but I did merge #78 earlier today.

climbfuji · 2021-02-23T03:39:35Z

Both PRs have been merged. Please pull in the latest changes from gsl/develop and make sure that the regression tests either pass or fail if expected (but run to completion) on Hera with Intel and/or Gnu. Thank you!

…into thomp_nc_density

climbfuji · 2021-02-23T16:17:29Z

@tanyasmirnova your PR shows changes in the RRTMGP submodule - I don't think you made any changes there. Can you check that your submodule pointer is correct and points to the same hash as gsl/develop, please?

Also, @dustinswales do you want to take a look at this PR from Tanya for the NOAA-GSL gsl/develop branch and see if there is anything that needs to be changed in RRTMGP?

dustinswales · 2021-02-23T16:32:03Z

@tanyasmirnova
Can you propagate these changes to GFS_rrtmgp_thompsonMP_pre.F90?
https://github.com/NCAR/ccpp-physics/blob/master/physics/GFS_rrtmgp_thompsonmp_pre.F90
You would only need to move the new computation of rho (line 150) into the loop, just below where qv_mp is beingcomputed. Then the same change to the call to make_DropletNumber as you did in GFS_rrtmg_pre.F90.

tanyasmirnova · 2021-02-23T17:34:10Z

@climbfuji @dustinswales I haven't changed the RRTMGP code yet.

Fix gthe units of water-friendly aerosols in the call to make_DropletNumber.

tanyasmirnova · 2021-02-23T17:43:46Z

@dustinswales Just now I committed changes to GFS_rrtmgp_thompsonmp_pre.F90.

climbfuji · 2021-02-25T17:34:44Z

@tanyasmirnova we still need to fix the submodule pointer for rrtmgp in your PR. Are the changes otherwise ok and consistent with Greg's proposed changes?

tanyasmirnova · 2021-02-25T18:42:11Z

@climbfuji @dustinswales Dom, Could you or Dustin help to fix the pointer?

climbfuji · 2021-02-25T18:47:11Z

@climbfuji @dustinswales Dom, Could you or Dustin help to fix the pointer?

cd FV3/ccpp/physics/physics/rte-rrtmgp
git remote update
git checkout 33c8a984c17cf41be5d4c2928242e1b4239bfc40
cd ..
git add rte-rrtmgp
git commit -m "Fix submodule pointer for rte-rrtmgp"
git push ...

tanyasmirnova · 2021-02-25T18:55:42Z

@climbfuji There is no 33c8a984c17cf41be5d4c2928242e1b4239bfc40 in my code.

climbfuji · 2021-02-25T19:13:30Z

33c8a984c17cf41be5d4c2928242e1b4239bfc40

tanyasmirnova#4

Thomp nc density fix submodule pointer rrtmgp

climbfuji · 2021-02-25T20:12:02Z

Thanks, @tanyasmirnova. Ok, back to the original question: is this PR ready from your side in light of Greg's upcoming PR?

tanyasmirnova · 2021-02-25T20:15:06Z

@climbfuji Dom, yes, my PR is ready. I can also make a PR on NCAR/ccpp branch to merger with Greg's changes. What do you think?

climbfuji · 2021-02-25T20:16:28Z

@climbfuji Dom, yes, my PR is ready. I can also make a PR on NCAR/ccpp branch to merger with Greg's changes. What do you think?

Probably not needed. When Greg's code is merged into NCAR master, we'll bring it over immediately.

tanyasmirnova · 2021-02-25T20:18:20Z

Sounds good.

climbfuji · 2021-02-25T20:24:09Z

Sounds good.

I'll create the fv3atm and ufs-weather-model PRs now and run the regression tests on hera.

tanyasmirnova · 2021-02-25T20:43:26Z

@climbfuji Thank you very much, Dom.

gthompsnJCSDA · 2021-02-25T22:51:52Z

physics/GFS_rrtmg_pre.F90

@@ -761,7 +763,7 @@ subroutine GFS_rrtmg_pre_run (im, levs, lm, lmk, lmp, n_var_lndp,        &
          do k=1,lm
            do i=1,im
              if (ltaerosol .and. qc_mp(i,k)>1.e-12 .and. nc_mp(i,k)<100.) then
-                nc_mp(i,k) = make_DropletNumber(qc_mp(i,k)*rho(i,k), nwfa(i,k)) * orho(i,k)
+                nc_mp(i,k) = make_DropletNumber(qc_mp(i,k)*rho(i,k), nwfa(i,k)*rho(i,k)) * orho(i,k)


There are inconsistencies with how I am testing the limits of QC and NC in mp_thompson code. And why are we needing to make droplet number in radiation code? Shouldn't this only be done in one place (MP code) and the resulting number just be given to radiation schemes?

I don't like all the duplication of code for fear that something goes awry/different in one than another place. If we need to make another isolation of ensuring species number and mass jointly make sense, then let's do it once and never more than once. But radiation code seems weird to me calling droplet or ice number fixes.

@joeolson42 @tanyasmirnova @hannahcbarnes was the reason for this the logic in the sgscloud_radpre and sgscloud_radpost schemes, i.e. the modification of qc and qi before radiation with boundary layer clouds from MYNN PBL in sgscloud_radpre, and then restoring the old values in ``sgscloud_radpost`?

We have to add the call to make_DropletNumber here only for the case when there are subgrid clouds created in PBL and convective schemes. They create sub-grid scale mixing ratios, but no NCs. After that the calc_effectRad is called using consistent mixing rations and nc_mp and ni_mp. Effective radii are needed for the radiation. After call to radiation, the sub-grid clouds are subtracted to restore the resolved mixing ratios from Thompson MP.

gthompsnJCSDA · 2021-02-25T22:53:25Z

physics/GFS_rrtmgp_thompsonmp_pre.F90

    do iLay = 1, nLev
       do iCol = 1, nCol    
          qv_mp(iCol,iLay) = q_lay(iCol,iLay)/(1.-q_lay(iCol,iLay))
+          rho(iCol,iLay)  = 0.622*p_lay(1:nCol,1:nLev)/(con_rd*t_lay(1:nCol,1:nLev)*(qv_mp(iCol,iLay)+0.622))    


To save some memory, shouldn't we be using scalar (singular) values inside the loops not arrays? Unless we are saving them for later usage, in which case I guess it's necessary.

I think the loops in lines 149-166 and in lines 169-178 can be combined, which would allow us to make rho and orho scalars.

I added here just one line at request from Dustin Swales.

Note that this line is also incorrect, because it multiplies arrays p_lay(1:nCol,1:nLev) and t_lay(1:nCol,1:nLev), i.e. entire 2-dim arrays, with a scalar qv_mp(iCol,iLay) and assigns it to a scalar rho(iCol,iLay). This leads to compile errors in debug mode. I am going to fix this in the PR I am preparing for @tanyasmirnova.

Of course! A copy/paste problem Thank you for fixing this.

gthompsnJCSDA · 2021-02-25T22:56:19Z

physics/GFS_rrtmgp_thompsonmp_pre.F90

@@ -169,7 +169,7 @@ subroutine GFS_rrtmgp_thompsonmp_pre_run(nCol, nLev, nTracers, ncnd, doSWrad, do
    do iLay = 1, nLev
       do iCol = 1, nCol  
          if (ltaerosol .and. qc_mp(iCol,iLay) > 1.e-12 .and. nc_mp(iCol,iLay) < 100.) then
-             nc_mp(iCol,iLay) = make_DropletNumber(qc_mp(iCol,iLay)*rho(iCol,iLay), nwfa(iCol,iLay)) * orho(iCol,iLay)
+             nc_mp(iCol,iLay) = make_DropletNumber(qc_mp(iCol,iLay)*rho(iCol,iLay), nwfa(iCol,iLay)*rho(iCol,iLay)) * orho(iCol,iLay)


Same comment as before. This becomes a 3rd time for possible differences in code to exist. I don't see it as necessary.

Is this possibly because timestep#1 itself calls radiation before microphysics? If so, we should resolve this problem permanently like it can be done in WRF using module_initialize_real.F to ensure no mass can exist without a corresponding number for any MP scheme for any species in 2 moments. It would be far more general and safe that way.

I don't recall the reasoning, but it shouldn't be the timestep, because mp_thompson_init runs before any of the _run routines. @tanyasmirnova @joeolson42 @hannahcbarnes made this cloud-radiation interaction changes originally. I hope they can comment.

To address your other comment about 3rd time. If it isn't necessary for rrtmgp, then it isn't necessary for rrtmg and will disappear in both. Let's hear what the GSL developers have to say.

I don't know the history why this call to make_dropletnumber() is in here.
But when using subgrid scale clouds, the cloud-fields are adjusted just prior to calling this routine in https://github.com/NCAR/ccpp-physics/blob/master/physics/module_SGSCloud_RadPre.F90.

My assumption was that the #-concentration needed to be updated consistently before calling radiation?
Hence why it is the same in RRTMGP as RRTMG.

I commented earlier on make_DropletNumber. We have to do it every time step for subgrid clouds.

@dustinswales Your assumption is correct.

The rrtmg_pre code prepares effective radii for the calls to progcld* for all microphysics schemes. Therefore it is logical to call calc_effecRad for THompson MP in this subroutine. RRTMGP has a separate subroutine for Thompson MP: GFS_rrtmgp_thompsonmp_pre.F90.

Hmm, you are right.

I strongly urge the group to consider coordinating a single place where we treat creation of water droplet number or ice crystal number for sub-grid-scale clouds in a unified single location, not among one or more variants of radiation physics. I am already handling explicit/grid-scale clouds within my MP scheme in a uniform way. We should do likewise for SGS clouds. And, in so doing, we can also permit potential other SGS cloud schemes to do likewise. In my view, this should be really simple to do in the file Dustin mentioned (module_SGSCloud_RadPre.F90). Could you also look over my code mods in PR#567 and note that I have included specific size constraints as parameter constants at top of MP-thompson that could be linked via USE ..., ONLY .... statements? And, in a related note, my changes to the radiative effective radii also removed the internal size constraints such that after calling the size calculation, the MIN/MAX statements are now quite important. This permits the actual calculation within the subroutine to go beyond the lower/upper limits in contrast to the old behavior.

@gthompsnJCSDA
In the RRTMGP enabled physics we collect all of the MP specific information to a single module (this one in the case of Thompson MP), a simple MP-2-radiation interface. There are other modules that do the same thing for different MP/radiation couplings (e.g GFS_rrtmgp_gfdlmp_pre.F90).

Conceptually, the SGS clouds are not related to this step in the process. The SGS clouds come in from other physical parameterizations (e.g. BL and convection) and the role of the sgsCloud_radpre routine is to couple these other MP to the cloud fields, which needs to be done before we couple the MP to the radiation.

I see no problem with importing min/max's if there needs to be bounding on the effective radii. I do exactly that for the RRTMGP particle sizes.

gthompsnJCSDA · 2021-02-25T22:56:46Z

physics/GFS_suite_interstitial.F90

                !> - Convert specific humidity to dry mixing ratio
                qv_mp(i,k) = spechum(i,k) / (one-spechum(i,k))
+                !> - Density of air in kg m-3
+                rho_air(i,k) = 0.622*prsl(i,k) / (con_rd*save_tcp(i,k)*(qv_mp(i,k)+0.622))


Same comment about memory footprint and possibly using a local scalar variable not arrays.

Yes, in this place certainly possible. One could also define another scalar orho = 1/rho and do only one division instead of two (754, 763).

gthompsnJCSDA · 2021-02-25T22:57:52Z

physics/mp_thompson.F90

@@ -304,7 +304,7 @@ subroutine mp_thompson_init(ncol, nlev, con_g, con_rd, restart,   &

           ! If qc is in boundary conditions but nc is not, calculate nc from qc, rho and nwfa
           if (maxval(qc_mp)>0.0 .and. maxval(nc_mp)==0.0) then
-             nc_mp = make_DropletNumber(qc_mp*rho, nwfa) * orho
+             nc_mp = make_DropletNumber(qc_mp*rho, nwfa*rho) * orho


This should already be a part of PR#567

If it is, I'll be giving a merge conflict to resolve, so no problem.

climbfuji · 2021-02-25T23:50:55Z

@tanyasmirnova let me make the two performance improvements that @gthompsnJCSDA recommended. If they work and don't change the answer (i.e. don't spoil your retros), I'll send you a PR to update your branch. I'll keep my fingers of the effective radii calculations, promise!

tanyasmirnova · 2021-02-25T23:59:36Z

Ok, sounds good.

DomHeinzeller · 2021-02-26T14:27:47Z

Ok, sounds good.

@tanyasmirnova see tanyasmirnova#5

…GFS_suite_interstitial.F90 and physics/GFS_rrtmgp_thompsonmp_pre.F90

Bugfixes for thomp_nc_density

1. Fixed units inconsistency for water friendly aerosols in the

5f7968b

computation of number concentration of liquid water. 2. Replaced dry-air density with the moist-air density for consistency with Thompson MP.

tanyasmirnova requested a review from DomHeinzeller as a code owner February 23, 2021 02:15

Merge branch 'gsl/develop' of https://github.com/NOAA-GSL/ccpp-physics …

a0b31a5

…into thomp_nc_density

Replace dry-air density to moist-air density in RRTMGP.

9cd399d

Fix gthe units of water-friendly aerosols in the call to make_DropletNumber.

Update submodule pointer for physics/rte-rrtmgp

a2d800b

tanyasmirnova and others added 2 commits February 25, 2021 19:17

Update submodule pointer for physics/rte-rrtmgp

5a9d256

Merge pull request #4 from climbfuji/thomp_nc_density

8449be5

Thomp nc density fix submodule pointer rrtmgp

This was referenced Feb 25, 2021

Remove inconsistencies in computation of air density with Thompson MP NOAA-GSL/fv3atm#74

Merged

Remove inconsistencies in computation of air density with Thompson MP NOAA-GSL/ufs-weather-model#64

Merged

DomHeinzeller changed the title ~~This PR removes inconsistencies in computation of air density with Thompson MP~~ Remove inconsistencies in computation of air density with Thompson MP Feb 25, 2021

gthompsnJCSDA reviewed Feb 25, 2021

View reviewed changes

climbfuji and others added 2 commits February 26, 2021 08:26

Bugfixes, performance improvements and formatting updates in physics/…

4c74498

…GFS_suite_interstitial.F90 and physics/GFS_rrtmgp_thompsonmp_pre.F90

Merge pull request #5 from climbfuji/bugfixes_tanya_thomp_nc_density

f06e592

Bugfixes for thomp_nc_density

DomHeinzeller approved these changes Feb 26, 2021

View reviewed changes

DomHeinzeller merged commit c82e501 into NOAA-GSL:gsl/develop Feb 26, 2021

climbfuji mentioned this pull request Mar 30, 2021

Update Thompson MP and RUC LSM from gsl/develop NCAR/ccpp-physics#600

Merged

Remove inconsistencies in computation of air density with Thompson MP #79

Remove inconsistencies in computation of air density with Thompson MP #79

Conversation

tanyasmirnova commented Feb 23, 2021 • edited by DomHeinzeller Loading

climbfuji commented Feb 23, 2021

climbfuji commented Feb 23, 2021

climbfuji commented Feb 23, 2021

climbfuji commented Feb 23, 2021

dustinswales commented Feb 23, 2021

tanyasmirnova commented Feb 23, 2021

tanyasmirnova commented Feb 23, 2021

climbfuji commented Feb 25, 2021

tanyasmirnova commented Feb 25, 2021

climbfuji commented Feb 25, 2021

tanyasmirnova commented Feb 25, 2021

climbfuji commented Feb 25, 2021

climbfuji commented Feb 25, 2021

tanyasmirnova commented Feb 25, 2021

climbfuji commented Feb 25, 2021

tanyasmirnova commented Feb 25, 2021

climbfuji commented Feb 25, 2021

tanyasmirnova commented Feb 25, 2021

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

tanyasmirnova Feb 25, 2021 • edited Loading

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

climbfuji commented Feb 25, 2021

tanyasmirnova commented Feb 25, 2021

DomHeinzeller commented Feb 26, 2021

tanyasmirnova commented Feb 23, 2021 •

edited by DomHeinzeller

Loading

tanyasmirnova Feb 25, 2021 •

edited

Loading