Merge NOAA-GSL gsl/develop into main (update GF CU, MYNN SFC/PBL, RUC LSM, bug fixes in UGWP) #791

climbfuji · 2021-12-01T14:29:03Z

This PR contains

a metadata bug fix and a bug fix for a sign error in UGWP - this is urgently needed in the authoritative repositories
major updates to GF convection, including a small change for the radiation coupling when GF convection is used with GFDDL-MP
updates to RUC LSM, MYNN SFC, MYNN PBL
updates to GFS_debug.F90 to include newly added variables and recently removed variables

Associated PRs:

NOAA-EMC/fv3atm#435
ufs-community/ufs-weather-model#937

For testing, see ufs-community/ufs-weather-model#937

…from the driver.

…changes by trial and error...

…hysics into HEAD

…hysics into gsl_develop_hannah_and_joe_changes_combined_20210712

into gsl_develop_hannah_and_joe_changes_combined_20210712

Minor code clean up

…hysics into gsl_develop_hannah_and_joe_changes_combined_20210712

into gsl_develop_hannah_and_joe_changes_combined_20210712

…ysics/cu_gf_driver_pre.meta

… physics/cu_gf_driver.F90 to achieve b4b identical results in restart runs

…changes_combined_20210712 Wrapper PR for NCAR#97 (Update of GF aerosol treatment and tunings) and NCAR#98 (Updates to MYNN-EDMF)

…b.com/climbfuji/ccpp-physics into update_gsl_develop_from_main_20210721

physics/GFS_debug.F90

grantfirl · 2021-12-03T21:02:23Z

physics/GFS_rrtmg_pre.F90

                else
                ! MYNN sub-grid cloud fraction
                  cldcov(i,k1) = clouds1(i,k1)
                endif
              enddo
            enddo
+          elseif (imfdeepcnv == imfdeepcnv_gf .and. kdt>1) THEN


I'm confused by this logic. Can't this condition and the condition above (do_mynnedmf) both be true simultaneously? If so, I'm not sure an IF/ELSEIF is the best way to handle this. Am I missing something?

If MYNN is present , then it takes care of everything (ie: it acounts for all subgrid clouds in MYNN and GF). If GF is called and MYNN is not present, the subgrid clouds in GF must be account for separately (since they are not accounted for in GFDL microphysics) This is why the elseif imfdeepcnv_gf is present and why it is listed after the if MYNN statement.

If you can come up with better logic I am open

OK, so I definitely was missing some information. Your solution obviously works for the use case, I'm just wondering if there is a clearer way to write it (that won't be interpreted as a bug by subsequent folks reading the code). What about nesting IF's?
if ((imfdeepcnv == imfdeepcnv_gf .or. do_mynnedmf) .and. kdt>1) then
if (do_mynnedmf) then
!calculate cldcov for MYNN
else
!calculate cldcov for GF
endif
else
!rest of options
endif

I don't know if this is any clearer or not. It's less efficient since you're testing for do_mynnedmf twice, but that is probably insignificant. @climbfuji opinions? We could always leave it as-is and maybe putting in a comment explaining things would be sufficient.

I agree, the logic in this part of the code is weird and should be made more clear. I am good with your suggested code change.

grantfirl · 2021-12-03T21:07:50Z

physics/cu_gf_deep.F90

-!              prop_c=.9/aeroadd
+               aeroadd=0.
+               if((psumh(i)>0.).and.(psum2(i)>0.))then
+               aeroadd=((1.e-2*ccnclean)**beta3)*((psumh(i)*1.e0)**(alpha3-1))


Why are we multiplying by 1e.0 in this line and 2459? Is this a placeholder to be changed in the future or something?

You can remove the 1e.0

grantfirl · 2021-12-03T21:17:43Z

physics/cu_gf_driver.meta

@@ -301,6 +322,14 @@
  type = real
  kind = kind_phys
  intent = in
+[aod_gf]
+  standard_name = aod_gf_deep


We should use "aerosol_optical_depth" rather than "aod". The standard names aerosol_optical_depth_for_shortwave_bands_01_16 and aerosol_optical_depth_for_longwave_bands_01_16 already exist. How does this new variable relate? Is it for all radiation bands? Is there a reason that this is specific to GF (could it potentially be used in other schemes)?

This is a specific variable used for only GF. It is not related to aerosol_optical_depth_for_shortwave_bands_01_16 or aerosol_optical_depth_for_longwave_bands_01_16. It is not for radiation bands, it is used as a measure of pollution in the atmosphere. GF is aerosol-aware, which is why it is needed. SAS is also aerosol-aware, but I have not looked close enough into the details of that scheme to see if they would use this.

You could rename it as aerosol_optical_depth_gf. You can also provide additional suggestions if you wish.

I'm thinking that we can just use aerosol_optical_depth full stop. According to the standard name rules (link), we shouldn't add qualifiers like _gf if they aren't absolutely necessary. That way if, say, SAS or any other convective scheme wanted to reuse this variable for their purposes or the same purpose, there wouldn't necessarily be a need to add a new variable (and therefore the memory footprint stays the same).

I am worried about that since people may get confused with aerosol optical depth used in other parts of the code, such as in the radiation. This variable should absolutely not be used in radiation. The name needs to be at least somewhat specific.

@climbfuji agree with what? The need to qualify or trying to keep it general? If it shouldn't be used for radiation, we could qualify that it is to be used in convection schemes. That would at least be more general than saying that only GF can use it?

I agree with Hannah that it needs to be made clear that this is specific to GF (I don't even know if it is compatible with any other convection scheme, but if it is, then it would be better to express it in the standard name). We do have other such cases in CCPP, of course, where we have adopted standard names w/o qualifier. One example being the water/ice friendly aerosols that, in the current representation can only be used by Thompson MP. Of course, if Thompson is not used, than anybody else can use those variables the way they want, but a developer finding aerosols somewhere and thinking that he or she can just use it will run into trouble.

OK, you both have convinced me. :) To be consistent with existing standard names spelling out GF, how about let's settle on aerosol_optical_depth_for_grell_freitas_deep_convection? Definitely longer, but consistent with others.

Heureka! Good with me. I'll make the change along with others over the weekend.

Thanks, this good with me as well.

climbfuji · 2021-12-03T21:30:27Z

@hannahcbarnes @haiqinli @joeolson42 Please take note of the comments that @grantfirl left and let me know if you have any suggestions/changes you wish to make. Thanks!

physics/drag_suite.meta

physics/module_MYNNPBL_wrapper.F90

grantfirl · 2021-12-03T21:58:28Z

physics/module_bl_mynn.F90

@@ -490,7 +519,7 @@ SUBROUTINE  mym_initialize (                                &
    INTEGER :: k,l,lmax
    REAL :: phm,vkz,elq,elv,b1l,b2l,pmz=1.,phh=1.,flt=0.,flq=0.,tmpq
    REAL :: zi
-    REAL, DIMENSION(kts:kte) :: theta
+      REAL, DIMENSION(kts:kte) :: theta,thetav,thlsg,qwsg


Silly, but for readability, consider removing the whitespace change so that var declarations align.

I agree. It's definitely meant to be aligned. I probably fat-fingered a couple spaces without noticing.

I'll fix it together with some other changes that were requested.

grantfirl

I will not pretend to understand all of the code changes represented here, but I've commented on anything unclear that I've detected. Thanks to all the developers of these schemes and nice work!

…te_gsl_global_and_regional_tests

causing cold/moist bias.

…/ccpp-physics into update_gsl_global_and_regional_tests

…te_gsl_global_and_regional_tests

climbfuji · 2021-12-10T18:20:52Z

@grantfirl @hannahcbarnes I addressed the reviewer comments for the gsl/develop update in 83ff129. Please check the changes, especially the change in the if-then-else logic in the RRTMG pre code. Thanks!

climbfuji · 2021-12-13T13:54:47Z

@yangfanglin This PR contains the bug fix for the sign error in physics/drag_suite.F90. It's been merged just now.

yangfanglin · 2021-12-13T14:03:03Z

Thanks, Dom.

…

Sent from my iPhone

On Dec 13, 2021, at 7:55 AM, Dom Heinzeller ***@***.***> wrote: @yangfanglin This PR contains the bug fix for the sign error in physics/drag_suite.F90. It's been merged just now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

joeolson42 and others added 30 commits June 22, 2021 01:38

Updating MYNN-EDMF

9841a99

Final updates for the MYNN-EDMF for HFIP

c02da68

Update of GF aerosol treatment and tunings

d35b3d5

Bug fix

3cb6b75

Bug fixes/clean up: (1) removing doxygen bug, (2) removing j indices …

42b9518

…from the driver.

Bug fixes - removing a 2nd doxygen bug

e2126f1

Bug fixes - removing a 3rd doxygen bug

9a1dc8d

Bug fixes - complaining about comments again...

3fe76ca

more doxygen complaints (I think), but they dont make sense. Testing …

a5909a7

…changes by trial and error...

more doxygen complaints (I think), but they dont make sense. Testing …

c127524

…changes by trial and error...

fix spelling

adf420e

Merge branch 'HCBaerosols' of https://github.com/hannahcbarnes/ccpp-p…

43fd15c

…hysics into HEAD

Remove unneeded variable

da8a13a

Merge branch 'HCBaerosols' of https://github.com/hannahcbarnes/ccpp-p…

65a1823

…hysics into gsl_develop_hannah_and_joe_changes_combined_20210712

Bug fix for clw_allh

5799e6f

Merge branch 'HCBaerosols' of https://github.com/hannahcbarnes/ccpp-p…

5d1881b

…hysics into gsl_develop_hannah_and_joe_changes_combined_20210712

Another bugfix in physics/cu_gf_deep.F90 to prevent qc(i,k) being zero

2c67750

Removing some commented out code.

64d85fe

Merge branch 'gsl/develop' of https://github.com/joeolson42/ccpp-physics

99ad1a7

into gsl_develop_hannah_and_joe_changes_combined_20210712

Modified aod_gf and cactiv_m so that work appriopriately with restart

60380d1

Minor code clean up

Merge branch 'HCBaerosols' of https://github.com/hannahcbarnes/ccpp-p…

5ada9ed

…hysics into gsl_develop_hannah_and_joe_changes_combined_20210712

Taking out OPTIONAL declaration, as it was before.

73f6a47

Merge branch 'gsl/develop' of https://github.com/joeolson42/ccpp-physics

46d9dba

into gsl_develop_hannah_and_joe_changes_combined_20210712

Correct wrong standard names in physics/cu_gf_driver_post.meta and ph…

a786194

…ysics/cu_gf_driver_pre.meta

Remove trailing whitespaces in physics/cu_gf_driver.F90; bug fixes in…

eac0587

… physics/cu_gf_driver.F90 to achieve b4b identical results in restart runs

Add GF variables to physics/GFS_debug.F90

8825827

Merge pull request NCAR#99 from climbfuji/gsl_develop_hannah_and_joe_…

9edfe02

…changes_combined_20210712 Wrapper PR for NCAR#97 (Update of GF aerosol treatment and tunings) and NCAR#98 (Updates to MYNN-EDMF)

Merge branch 'main' of https://github.com/NCAR/ccpp-physics into HEAD

5e3cbf8

Merge branch 'bugfix_thompson_pass_correct_timestep' of https://githu…

c44fe36

…b.com/climbfuji/ccpp-physics into update_gsl_develop_from_main_20210721

Revert change to CODEOWNERS

0e0b7f1

This was referenced Dec 3, 2021

Merge NOAA-GSL gsl/develop into develop, update all GSL regression tests to use new GFS v16 C96L127 ICs, update macOS build config ufs-community/ufs-weather-model#937

Merged

Merge NOAA-GSL gsl/develop into develop, update CCPP SDFs NOAA-EMC/fv3atm#435

Merged

climbfuji marked this pull request as ready for review December 3, 2021 14:10

climbfuji requested review from llpcarson and mkavulich as code owners December 3, 2021 14:10

climbfuji changed the title ~~DRAFT - Merge NOAA-GSL gsl/develop into main~~ Merge NOAA-GSL gsl/develop into main (update GF CU, MYNN SFC/PBL, RUC LSM, bug fixes in UGWP) Dec 3, 2021

grantfirl reviewed Dec 3, 2021

View reviewed changes

physics/GFS_debug.F90 Outdated Show resolved Hide resolved

grantfirl reviewed Dec 3, 2021

View reviewed changes

physics/drag_suite.meta Show resolved Hide resolved

grantfirl reviewed Dec 3, 2021

View reviewed changes

physics/module_MYNNPBL_wrapper.F90 Show resolved Hide resolved

grantfirl reviewed Dec 3, 2021

View reviewed changes

grantfirl approved these changes Dec 3, 2021

View reviewed changes

climbfuji and others added 5 commits December 6, 2021 15:26

Merge branch 'main' of https://github.com/NCAR/ccpp-physics into upda…

ff37c97

…te_gsl_global_and_regional_tests

Bug fix in the computation of soil moisture conductivity. The bug was

c1e1770

causing cold/moist bias.

Merge branch 'gsl_dev_ruc_bugfix' of https://github.com/tanyasmirnova…

0d6e114

…/ccpp-physics into update_gsl_global_and_regional_tests

Merge branch 'main' of https://github.com/NCAR/ccpp-physics into upda…

7a520b4

…te_gsl_global_and_regional_tests

Merge branch 'main' of https://github.com/NCAR/ccpp-physics into upda…

5256aca

…te_gsl_global_and_regional_tests

climbfuji mentioned this pull request Dec 10, 2021

Bug fix in the computation of soil moisture conductivity. NOAA-GSL/ccpp-physics#117

Merged

climbfuji added 3 commits December 10, 2021 10:29

Remove variables that no longer exist from physics/GFS_debug.F90

fcedb92

Address reviewer comments

83ff129

Update DDTs in physics/GFS_debug.F90

2649dce

climbfuji merged commit 4865cba into NCAR:main Dec 13, 2021

grantfirl mentioned this pull request Dec 16, 2021

Update SCM to the latest ccpp-physics main NCAR/ccpp-scm#287

Merged

climbfuji deleted the update_gsl_global_and_regional_tests branch June 27, 2022 03:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge NOAA-GSL gsl/develop into main (update GF CU, MYNN SFC/PBL, RUC LSM, bug fixes in UGWP) #791

Merge NOAA-GSL gsl/develop into main (update GF CU, MYNN SFC/PBL, RUC LSM, bug fixes in UGWP) #791

climbfuji commented Dec 1, 2021 •

edited

Loading

grantfirl Dec 3, 2021

hannahcbarnes Dec 3, 2021

grantfirl Dec 3, 2021

hannahcbarnes Dec 3, 2021

grantfirl Dec 3, 2021

hannahcbarnes Dec 3, 2021

grantfirl Dec 3, 2021

hannahcbarnes Dec 3, 2021

grantfirl Dec 3, 2021

hannahcbarnes Dec 3, 2021

climbfuji Dec 3, 2021

grantfirl Dec 3, 2021

climbfuji Dec 3, 2021

grantfirl Dec 3, 2021

climbfuji Dec 3, 2021

hannahcbarnes Dec 6, 2021

climbfuji commented Dec 3, 2021

grantfirl Dec 3, 2021

joeolson42 Dec 3, 2021

climbfuji Dec 3, 2021

grantfirl left a comment

climbfuji commented Dec 10, 2021

climbfuji commented Dec 13, 2021

yangfanglin commented Dec 13, 2021 via email

Merge NOAA-GSL gsl/develop into main (update GF CU, MYNN SFC/PBL, RUC LSM, bug fixes in UGWP) #791

Merge NOAA-GSL gsl/develop into main (update GF CU, MYNN SFC/PBL, RUC LSM, bug fixes in UGWP) #791

Conversation

climbfuji commented Dec 1, 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

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 Dec 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grantfirl left a comment

Choose a reason for hiding this comment

climbfuji commented Dec 10, 2021

climbfuji commented Dec 13, 2021

yangfanglin commented Dec 13, 2021 via email

climbfuji commented Dec 1, 2021 •

edited

Loading