Merge May 16 NCAR main branch to gsl/develop by SamuelTrahanNOAA · Pull Request #153 · NOAA-GSL/ccpp-physics

SamuelTrahanNOAA · 2022-05-18T01:41:20Z

This PR will update gsl/develop to the latest NCAR main as of May 16.

Also merges Joe's changes from #143

…g calcnfromq from running, which creates number concentration from the initial condition hydrometeor mass

…rious points of very small mass with large reflectivity were showing up, perhaps because of the very large time step in UFS (40s). This helps eliminate those.

…t cleaner. Changes to calcnfromq to set droplet number as 9 micron radius droplets, and then deplete CCN if turned on. Also set masses to zero if less than qxmin.

…s way)

…er mixing ratios masses are transferred to water vapor. Also added second estimate for graupel number conc. and take minimum. Added air density limit in setvtz and nssl_2mom_gs to limit fall speed or rhovt. Added limit on Bigg freezing to only act if freezing radius is 8mm or less.

…d flag for nssl_ccn_on - Updataed microphysics - Radiation (rrtmg) includes calculated rain radius. Test code to compute radii in the subroutine, but something not right with incoming number concentrations - Renamed mp_nsslg to mp_nssl

…ine instead of using physcons module

…physics into feature/nsslmicro

…n, cloud ice, snow, graupel, and hail. Graupel and hail have predicted bulk density via the particle volume. Hail can be deactived. Simple CCN concentration can be predicted, either as the count of unactivated or activated nuclei. (Mansell et al. 2010, JAS)

…ers)

- Split NSSL conditional in GFS_rrtmg_pre.F90 from Thompson for now - Text comments in radiation_clouds.f

…g calcnfromq from running, which creates number concentration from the initial condition hydrometeor mass

joeolson42 · 2022-05-18T15:49:26Z

+                      ! If rain/snow present, use GFDL MP cloud-fraction...
+                      if (tracer(iCol,iLay,i_cldrain)>1.0e-7 .OR. tracer(iCol,iLay,i_cldsnow)>1.0e-7) then
+                         cld_frac(iCol,iLay) = tracer(iCol,iLay,i_cldtot)
+                      endif


This MYNN portion (lines 183 to about 190 and the associated endif) should be removed.

According to Sam's comments below, we aren't supposed to make changes, but as soon as we are allowed to make changes, this part of the code will need to be removed.

Is this a critical bug that needs to be fixed in the community repository? Or can we just change it in gsl/develop and worry about the community in a couple months?

It will only cause problems when we start testing our suite with RRTMGP, so no, it's not immediately urgent. It may take considerable time to go through all of the radiation-related changes and find a clean fix.

joeolson42 · 2022-05-18T16:02:23Z

+          endif
+       enddo
+    enddo
+  end subroutine cloud_mp_MYNN


We haven't started testing RRTMGP yet, but I'm sure this code/module will need to be revisited. This seems to be a partial duplication of SGSCloud_RadPre and I'm not certain yet about the order of calls. This concern is not just for the MYNN but also for GF. We need to have this code in one module only.

Same comment here: It will only cause problems when we start testing with RRTMGP. It looks like the organization/improvement changes they made to the radiation code were just a first step.

SamuelTrahanNOAA · 2022-05-18T16:13:03Z

We should not add any code changes. This PR needs to mark a point in the branch history where everything worked on all platforms, and matched the authoritative repositories. GSL does not have the ability to test on all platforms, so our only recourse is to not change anything in this PR, unless absolutely necessary.

joeolson42 · 2022-05-18T16:45:52Z


-    LOGICAL, INTENT(IN) :: mix_chem,fire_turb
+    LOGICAL, INTENT(IN) :: mix_chem,fire_turb,rrfs_smoke


Here and below, I don't think we need rrfs_smoke because this smoke functionality simply leverages the chem mixing routine, so if rrfs_smoke is set to true in the namelist, then mix_chem should be set to true somewhere upstream of the MYNN. That is, the MYNN doesn't need to even know about this parameter.

These changes are necessary to run on Cheyenne with the gnu compiler. If you want to try reducing the number of changes without causing the model to crash, they you're welcome to do so... if you have a Cheyenne account.

joeolson42 · 2022-05-18T16:51:58Z

@@ -159,7 +161,8 @@ SUBROUTINE mynnedmf_wrapper_run(        &
     &  icloud_bl, do_mynnsfclay,                          &
     &  imp_physics, imp_physics_gfdl,                     &
     &  imp_physics_thompson, imp_physics_wsm6,            &
-     &  chem3d, frp, mix_chem, fire_turb, nchem, ndvel,    &
+     &  chem3d, frp, mix_chem, rrfs_smoke, fire_turb, nchem, ndvel, &


same comment as in module_bl_mynn.F90, we shouldn't need both rrfs_smoke and mix_chem. The MYNN doesn't need to know the specific application, it only needs to know that the chem mixing routines are to be used.

These changes are necessary to run on Cheyenne with the gnu compiler. If you want to try reducing the number of changes without causing the model to crash, they you're welcome to do so... if you have a Cheyenne account.

Then there must be a bug in the smoke implementation part of the MYNN changes because it should only take one flag to activate/use that portion of the code. That was definitely a rushed implementation that needs a revision.

It was. I had an hour to get it working.

joeolson42

The only critical change I see is the removal of a block of code in a rrtmgp module (see comments). The rest of my comments are just as much questions as requests. The overwhelming majority of the changes I see are welcomed.

SamuelTrahanNOAA · 2022-05-18T18:06:58Z

The only critical change I see is the removal of a block of code in a rrtmgp module (see comments). The rest of my comments are just as much questions as requests. The overwhelming majority of the changes I see are welcomed.

@joeolson42 Are you asking me to change something in this PR? If so, what?

joeolson42 · 2022-05-18T18:31:53Z

@SamuelTrahanNOAA Sam, the only change I'm referring to is commented on above, In physics/GFS_rrtmgp_cloud_mp.F90, there is a block of code which I had removed from an RRTMG module but it was apparently copied into this RRTMGP module before our code was merged with the community repo.

SamuelTrahanNOAA · 2022-05-18T19:17:06Z

@joeolson42 I'm stll not sure what you want me to remove. Is it lines 162 through 211 of physics/GFS_rrtmgp_cloud_mp.F90?

joeolson42 · 2022-05-18T19:36:54Z

Just the mynn portion. I think the GF portion is harmless but probably not needed. That should be tested to be sure.

Lines 183-190 need to be deleted.
Then remove ".or. do_mynnedmf" from line 182.
Then remove the "endif" on line 195.

SamuelTrahanNOAA · 2022-05-18T19:45:03Z

@joeolson42 Like this?

          do iLay = 1, nLev
             do iCol = 1, nCol
                !
                ! SGS clouds present, use cloud-fraction modified to include sgs clouds.
                !
                if (imfdeepcnv==imfdeepcnv_gf .and. kdt>1) then
                  ! If no convective cloud condensate present, use GFDL MP cloud-fraction....
                  if (qci_conv(iCol,iLay) <= 0.) then
                    cld_frac(iCol,iLay) = tracer(iCol,iLay,i_cldtot)
                  endif
                !
                ! No SGS clouds, use GFDL MP cloud-fraction...
                !
                else
                   cld_frac(iCol,iLay) = tracer(iCol,iLay,i_cldtot)
                endif
             enddo
          enddo

joeolson42 · 2022-05-18T19:53:20Z

@SamuelTrahanNOAA that's correct.

SamuelTrahanNOAA · 2022-05-18T19:54:50Z

@joeolson42 Do you expect the results of any tests to change?

joeolson42 · 2022-05-18T19:57:58Z

It should change the results for any suite that runs both MYNN-EDMF and RRTMGP, but I don't think we have been testing that combination yet.

SamuelTrahanNOAA · 2022-05-18T20:00:11Z

In that case, I won't change it in this PR. I'm going to do another PR in a day or two, to add the regression tests we need to debug other problems. I can add your changes at that time. If you have any more changes to fix the other issues you mentioned, please give them to me as soon as possible.

Keep in mind though, that the code does have to work on Cheyenne gnu, so you can't do this:

   real(kind_phys), intent(inout), optional :: varname(ims:ime,jms:jme)

or it'll crash on some versions of the gnu compiler.

SamuelTrahanNOAA · 2022-05-18T20:01:35Z

This is what you have to do instead:

    real(kind_phys), intent(inout) :: varname(:,:)

That's required by the CCPP coding standards because the gnu compiler (and apparently others) won't accept a null pointer to an optional argument with specified bounds.

a MERRA2 bug fixed from RRFS

MicroTed added 30 commits October 14, 2021 21:05

Fixed typo in meta file

315489b

Fixed typo and missing declaration

2b95bde

- Fixed setting of itimestep on first model step. This was preventin…

4b35ce9

…g calcnfromq from running, which creates number concentration from the initial condition hydrometeor mass

Turned on zeroing out of hail for low number concentration. Some spu…

1cfe2c8

…rious points of very small mass with large reflectivity were showing up, perhaps because of the very large time step in UFS (40s). This helps eliminate those.

Added extra printout info for large fall speeds.

c27901c

Call calcnfromq on every time step, which helps keep boundaries a bi…

21a2875

…t cleaner. Changes to calcnfromq to set droplet number as 9 micron radius droplets, and then deplete CCN if turned on. Also set masses to zero if less than qxmin.

Removed re_rain from interface (not used and not planning to use thi…

06fc773

…s way)

Changed itimestep to a purely local variable (i.e., not saved)

6d484f7

Fixed bug in setting array values of "rain" (noticed by E. Aligo)

635e028

Fixed missing setting of save arrays for NSSL.

e18f790

Update to newer base code plus some cleanup of NSSL microphysics

b565f5f

Made IF test on tracer indices in post_run consistent with pre_run

3b7b139

Switched 'vertical_dimension' to 'vertical_layer_dimension'

7e41947

Added convert_dry_rho flag

7e38a90

Removed some commented code; pass in physical constants to init rout…

b17240a

…ine instead of using physcons module

Added dependencies to RUC physics

53658e5

Cleaned up unused code and variables.

da10201

Merge branch 'feature/nsslmicro' of https://github.com/MicroTed/ccpp-…

961d9b6

…physics into feature/nsslmicro

- Fixes subroutine end statements (causes error on some older compil…

c8bbd69

…ers)

Add missing 'nthl' to call interface

a5f236f

- Pass effrr into NSSL driver

e38406a

- Split NSSL conditional in GFS_rrtmg_pre.F90 from Thompson for now - Text comments in radiation_clouds.f

Turned off a print statement.

841eab2

Restore the incorrectly removed flags.

72e7693

Turn off setting rain radius for now.

d9f2ced

Fixed typo in meta file

2b92bcb

Fixed typo and missing declaration

8157607

- Fixed setting of itimestep on first model step. This was preventin…

a8f3948

…g calcnfromq from running, which creates number concentration from the initial condition hydrometeor mass

Change to GSL CODEOWNERS

d40f5e2

SamuelTrahanNOAA requested review from christinaholtNOAA, haiqinli, hannahcbarnes, joeolson42, mdtoyNOAA, tanyasmirnova and zhanglikate as code owners May 18, 2022 01:41

SamuelTrahanNOAA mentioned this pull request May 18, 2022

Merge May 16 ufs-community develop to gsl/develop NOAA-GSL/ufs-weather-model#138

Merged

16 tasks

joeolson42 reviewed May 18, 2022

View reviewed changes

tanyasmirnova reviewed May 18, 2022

View reviewed changes

Comment thread physics/module_sf_ruclsm.F90

joeolson42 reviewed May 18, 2022

View reviewed changes

joeolson42 approved these changes May 18, 2022

View reviewed changes

hannahcbarnes approved these changes May 18, 2022

View reviewed changes

Returned the comments on soil resistance back.

eccf83a

SamuelTrahanNOAA merged commit 7f8b211 into NOAA-GSL:gsl/develop May 18, 2022

zhanglikate referenced this pull request in zhanglikate/ccpp-physics Mar 1, 2024

Merge pull request NCAR#153 from AnningCheng-NOAA/mr2_peq

a492add

a MERRA2 bug fixed from RRFS


		LOGICAL, INTENT(IN) :: mix_chem,fire_turb
		LOGICAL, INTENT(IN) :: mix_chem,fire_turb,rrfs_smoke

Conversation

SamuelTrahanNOAA commented May 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

SamuelTrahanNOAA commented May 18, 2022

Uh oh!

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!

joeolson42 May 18, 2022 • 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!

joeolson42 left a comment

Choose a reason for hiding this comment

Uh oh!

SamuelTrahanNOAA commented May 18, 2022

Uh oh!

joeolson42 commented May 18, 2022

Uh oh!

SamuelTrahanNOAA commented May 18, 2022

Uh oh!

joeolson42 commented May 18, 2022

Uh oh!

SamuelTrahanNOAA commented May 18, 2022

Uh oh!

joeolson42 commented May 18, 2022

Uh oh!

SamuelTrahanNOAA commented May 18, 2022

Uh oh!

joeolson42 commented May 18, 2022

Uh oh!

SamuelTrahanNOAA commented May 18, 2022

Uh oh!

SamuelTrahanNOAA commented May 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

18 participants

SamuelTrahanNOAA commented May 18, 2022 •

edited

Loading

joeolson42 May 18, 2022 •

edited

Loading