Wrapper PR for #94 (GF aerosol updates and tunings) by climbfuji · Pull Request #95 · NOAA-GSL/ufs-weather-model

climbfuji · 2021-07-14T21:13:17Z

This is a wrapper PR for #94 with the correct submodule pointer and .gitmodules updates.

Associated PRs:

NOAA-GSL/ccpp-physics#99
NOAA-GSL/fv3atm#101
#95

Regression testing (updated after fixing b4b restart issues for GF):

Create new baselines using rt_ccpp_dev.conf on Hera with Intel and GNU, then verify against those baselines:
- Intel: all tests pass

rt_ccpp_dev_hera_intel_create.log
rt_ccpp_dev_hera_intel_verify.log

- GNU: all tests pass

rt_ccpp_dev_hera_gnu_create.log
rt_ccpp_dev_hera_gnu_verify.log

Verify against existing baselines from authoritative repositories on Hera with Intel and GNU and make sure that only those tests fail that we expect to fail (but that they run to completion):
- Intel: the following tests fail with b4b mismatches or missing baselines, as expected (they all run to completion)

fv3_rap
fv3_hrrr
fv3_rrfs_v1beta
fv3_rrfs_v1alpha
fv3_gsd
regional_control
regional_quilt_2threads
regional_quilt
regional_quilt_hafs
regional_quilt_netcdf_parallel
regional_quilt_RRTMGP
regional_control_debug
fv3_rrfs_v1beta_debug
fv3_rrfs_v1alpha_debug
fv3_gsd_debug
fv3_gsd_dtend_debug

rt_hera_intel_verify_against_existing.log
rt_hera_intel_verify_against_existing_fail_test.log

- GNU: the following tests fail with b4b mismatches or missing baselines, as expected (they all run to completion)

fv3_rrfs_v1beta
fv3_gsd
fv3_rrfs_v1alpha
fv3_rrfs_v1alpha_debug
fv3_gsd_dtend_debug
fv3_rrfs_v1beta_debug
fv3_gsd_debug
regional_control_debug

rt_hera_gnu_verify_against_existing.log
rt_hera_gnu_verify_against_existing_fail_test.log

…nd testing

SamuelTrahanNOAA

Make sure you set the submodule url and branch for FV3 back to NOAA-GSL gsl/develop before merging. Other than that, this is fine.

hannahcbarnes · 2021-07-15T12:35:09Z

@DomHeinzeller do you have any idea of what needs to be fixed so that the code passes the warm start test?

climbfuji · 2021-07-15T14:47:40Z

@DomHeinzeller do you have any idea of what needs to be fixed so that the code passes the warm start test?

Not at this point. It's usually time consuming to figure this out. I'll look at the code changes, but I really wish that code changes were tested for these kind of things during the development phase and before creating the PRs. I'll test your code changes and Joe's code changes separately to get started.

hannahcbarnes · 2021-07-15T15:27:56Z

I noticed in part of the code I added I have an if statement that has a flag_init and not a flag_restart. Could that cause a problem?

climbfuji · 2021-07-15T15:35:58Z

I noticed in part of the code I added I have an if statement that has a flag_init and not a flag_restart. Could that cause a problem?

My guess is that aod_gf needs to be added to the restart files.

hannahcbarnes · 2021-07-15T15:39:37Z

Could we avoid adding it to the restart files if I go into cu_gf_driver_pre and add aod_gf into this if statement

  if(flag_init .and. .not.flag_restart) then
    forcet(:,:)=0.0
    forceq(:,:)=0.0
    aod_gf(:)=-999.
  else

Then in cu_gf_driver change the if file_init to if aod_gf < 0

climbfuji · 2021-07-15T15:41:58Z

In cu_gf_driver:

      ! set aod and ccn
       if (flag_init) then
         aod_gf(i)=aodc0
       else
         if((cactiv(i).eq.0) .and. (cactiv_m(i).eq.0))then
           if(aodc0>aod_gf(i)) aod_gf(i)=aod_gf(i)+((aodc0-aod_gf(i))*(dt/(aodreturn*60)))
           if(aod_gf(i)>aodc0) aod_gf(i)=aodc0
         endif
       endif

This means that you need to have valid data in aod_gf unless you coldstart the model?

hannahcbarnes · 2021-07-15T15:45:02Z

Yes, but if I add aod_gf to cu_gf_driver_pre like this:
if(flag_init .and. .not.flag_restart) then
forcet(:,:)=0.0
forceq(:,:)=0.0
aod_gf(:)=-999.
else

And change cu_gf_driver to
if(aod_gf(i).lt.0.)then
aod_gf(i)=aodc0
else
if((cactiv(i).eq.0) .and. (cactiv_m(i).eq.0))then
if(aodc0>aod_gf(i)) aod_gf(i)=aod_gf(i)+((aodc0-aod_gf(i))(dt/(aodreturn60)))
if(aod_gf(i)>aodc0) aod_gf(i)=aodc0
endif
endif

Shouldn't that issue be avoided.

hannahcbarnes · 2021-07-15T15:51:02Z

Probably also need to change the code since cactiv_m is treated the same as cactiv in cu_gf_driver_pre and cu_gf_driver_post. I am working on that now.

climbfuji · 2021-07-15T15:54:54Z

Yes, but if I add aod_gf to cu_gf_driver_pre like this:
if(flag_init .and. .not.flag_restart) then
forcet(:,:)=0.0
forceq(:,:)=0.0
aod_gf(:)=-999.
else

And change cu_gf_driver to
if(aod_gf(i).lt.0.)then
aod_gf(i)=aodc0
else
if((cactiv(i).eq.0) .and. (cactiv_m(i).eq.0))then
if(aodc0>aod_gf(i)) aod_gf(i)=aod_gf(i)+((aodc0-aod_gf(i))_(dt/(aodreturn_60)))
if(aod_gf(i)>aodc0) aod_gf(i)=aodc0
endif
endif

Shouldn't that issue be avoided.

Which values will aod_gf have for the first call of a restart run? They would all be zero if not written to disk. Conversely, in continuous runs, they may have non-zero values. This will make a difference in line

if(aodc0>aod_gf(i)) aod_gf(i)=aod_gf(i)+((aodc0-aod_gf(i))_(dt/(aodreturn_60)))

as far as I can see. Also, because of the preceding line

if((cactiv(i).eq.0) .and. (cactiv_m(i).eq.0))then

do we need to care about those, too? For cactiv, Haiqin put code in cu_gf_driver_pre:

      cactiv(:)=nint(conv_act(:))

and conv_act is written to the restart files. But I don't see anything for cactiv_m.

joeolson42 · 2021-07-15T15:55:19Z

I wouldn't rule out MYNN changes. I'm reviewing the code now.

…

On Thu, Jul 15, 2021 at 9:51 AM Hannah C Barnes ***@***.***> wrote: Probably also need to change the code since cactiv_m is treated the same as cactiv in cu_gf_driver_pre and cu_gf_driver_post. I am working on that now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#95 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADLRR3T7NTCCQL754ZB2N43TX37XDANCNFSM5AMHUQDA> .

-- Joseph Olson Model Physics Branch Chief Environmental Prediction Advancement Division NOAA-Global Systems Laboratory Boulder, Colorado

hannahcbarnes · 2021-07-15T15:56:39Z

Yes, I just added cactiv_m to the GFS_restart and cu_gf_driver_pre and cu_gf_driver_post.

Do you want me to add aod_gf to GFS_restart?

climbfuji · 2021-07-15T16:09:31Z

Yes, I just added cactiv_m to the GFS_restart and cu_gf_driver_pre and cu_gf_driver_post.

Do you want me to add aod_gf to GFS_restart?

Yes please.

hannahcbarnes · 2021-07-15T16:11:22Z

Sounds good. I will have those added to the PR in a few minutes.

hannahcbarnes · 2021-07-15T16:30:36Z

I have those changes in my repository and they compiled for me.

Do you want to me to make a pull request for climbfuji:gsl_develop_hannah_and_joe_changes_combined_20210712?

climbfuji · 2021-07-15T16:31:29Z

I have those changes in my repository and they compiled for me.

Do you want to me to make a pull request for climbfuji:gsl_develop_hannah_and_joe_changes_combined_20210712?

I already pulled them in, but I am getting compile errors. Let me check.

climbfuji · 2021-07-15T16:35:08Z

I have those changes in my repository and they compiled for me.
Do you want to me to make a pull request for climbfuji:gsl_develop_hannah_and_joe_changes_combined_20210712?

I already pulled them in, but I am getting compile errors. Let me check.

I'll fix those, please no more changes in your branch.

hannahcbarnes · 2021-07-15T16:41:22Z

Thanks so much!

joeolson42 · 2021-07-15T17:32:42Z

Dom, I found something in module_bl_mynn.F90 that was changed inadvertently, probably when merging back-n-forth with WRF code: REAL, DIMENSION(IMS:IME,KMS:KME), INTENT(inout), optional :: & &qc_bl,qi_bl,cldfra_bl The original code was not declared as optional. Any idea what the consequences may be?

…

-joe

On Thu, Jul 15, 2021 at 10:41 AM Hannah C Barnes ***@***.***> wrote: Thanks so much! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#95 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADLRR3RDVKMTHDPDZCIXRE3TX4FT3ANCNFSM5AMHUQDA> .

-- Joseph Olson Model Physics Branch Chief Environmental Prediction Advancement Division NOAA-Global Systems Laboratory Boulder, Colorado

climbfuji · 2021-07-15T17:39:21Z

Dom, I found something in module_bl_mynn.F90 that was changed inadvertently, probably when merging back-n-forth with WRF code: REAL, DIMENSION(IMS:IME,KMS:KME), INTENT(inout), optional :: & &qc_bl,qi_bl,cldfra_bl The original code was not declared as optional. Any idea what the consequences may be?
…

If those arrays are always allocated, then none. If they are not, then the consequences can be bad (segmentation faults, ...).

climbfuji · 2021-07-15T17:39:52Z

Dom, I found something in module_bl_mynn.F90 that was changed inadvertently, probably when merging back-n-forth with WRF code: REAL, DIMENSION(IMS:IME,KMS:KME), INTENT(inout), optional :: & &qc_bl,qi_bl,cldfra_bl The original code was not declared as optional. Any idea what the consequences may be?
…

If those arrays are always allocated, then none. If they are not, then the consequences can be bad (segmentation faults, ...).

Please go ahead and change it back, we'll have to redo all the testing anyway once the restart issue is resolved.

DomHeinzeller · 2021-07-15T23:03:22Z

Update: still debugging. I fixed one more bug in GF, and I replaced MYNN with satmedmf. Still b4b different for restart runs. Will resume tomorrow.

hannahcbarnes · 2021-07-16T13:08:32Z

Sorry, about the b4b problems. Let me know how I can help.

I finally got a test restart run on hera (hera is super slow...). I got an error "forrtl: severe (408): fort: (2): Subscript #1 of the array NAME2D has value 8 which is greater than the upper bound of 7" I assume I need to increase the size of NAME2d in GFS_restart, which I did not do orginally. Would that impact the b4b error?

DomHeinzeller · 2021-07-16T16:19:00Z

Sorry, about the b4b problems. Let me know how I can help.

I finally got a test restart run on hera (hera is super slow...). I got an error "forrtl: severe (408): fort: (2): Subscript #1 of the array NAME2D has value 8 which is greater than the upper bound of 7" I assume I need to increase the size of NAME2d in GFS_restart, which I did not do orginally. Would that impact the b4b error?

Hi Hannah, yes, I fixed this and now GF is b4b. Will add MYNN back into the mix. I'll also commit the bug fixes, nothing you need to do at the moment. Thanks!

hannahcbarnes and others added 4 commits July 9, 2021 14:47

GF aerosol updates and tunings

6be87b3

Update .gitmodules and submodule pointer for fv3atm for code review a…

7f9e4a2

…nd testing

Update submodule pointer for fv3atm

6ac68ef

Update submodule pointer for fv3atm

b3199fb

This was referenced Jul 14, 2021

Wrapper PR for #97 (Update of GF aerosol treatment and tunings) and #98 (Updates to MYNN-EDMF) NOAA-GSL/ccpp-physics#99

Merged

Wrapper for PR #100 (GF aerosols updates and tunings) NOAA-GSL/fv3atm#101

Merged

DomHeinzeller requested review from DomHeinzeller, SamuelTrahanNOAA, hannahcbarnes, joeolson42 and tanyasmirnova July 14, 2021 21:23

SamuelTrahanNOAA approved these changes Jul 14, 2021

View reviewed changes

climbfuji force-pushed the gsl_develop_hannah_and_joe_changes_combined_20210712 branch from 8348b67 to 7677361 Compare July 15, 2021 17:47

Update submodule pointer for fv3atm

edb5ed8

climbfuji force-pushed the gsl_develop_hannah_and_joe_changes_combined_20210712 branch from 7677361 to edb5ed8 Compare July 16, 2021 16:22

Update submodule pointer for fv3atm

e098d78

DomHeinzeller approved these changes Jul 19, 2021

View reviewed changes

Revert change to .gitmodules and update submodule pointer for fv3atm

d25402f

climbfuji force-pushed the gsl_develop_hannah_and_joe_changes_combined_20210712 branch from 21412c8 to d25402f Compare July 19, 2021 16:25

DomHeinzeller merged commit 9909f5f into NOAA-GSL:gsl/develop Jul 19, 2021

Conversation

climbfuji commented Jul 14, 2021 • edited by DomHeinzeller Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SamuelTrahanNOAA left a comment

Choose a reason for hiding this comment

Uh oh!

hannahcbarnes commented Jul 15, 2021

Uh oh!

climbfuji commented Jul 15, 2021

Uh oh!

hannahcbarnes commented Jul 15, 2021

Uh oh!

climbfuji commented Jul 15, 2021

Uh oh!

hannahcbarnes commented Jul 15, 2021

Uh oh!

climbfuji commented Jul 15, 2021

Uh oh!

hannahcbarnes commented Jul 15, 2021

Uh oh!

hannahcbarnes commented Jul 15, 2021

Uh oh!

climbfuji commented Jul 15, 2021

Uh oh!

joeolson42 commented Jul 15, 2021 via email

Uh oh!

hannahcbarnes commented Jul 15, 2021

Uh oh!

climbfuji commented Jul 15, 2021

Uh oh!

hannahcbarnes commented Jul 15, 2021

Uh oh!

hannahcbarnes commented Jul 15, 2021

Uh oh!

climbfuji commented Jul 15, 2021

Uh oh!

climbfuji commented Jul 15, 2021

Uh oh!

hannahcbarnes commented Jul 15, 2021

Uh oh!

joeolson42 commented Jul 15, 2021 via email

Uh oh!

climbfuji commented Jul 15, 2021

Uh oh!

climbfuji commented Jul 15, 2021

Uh oh!

DomHeinzeller commented Jul 15, 2021

Uh oh!

hannahcbarnes commented Jul 16, 2021

Uh oh!

DomHeinzeller commented Jul 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

climbfuji commented Jul 14, 2021 •

edited by DomHeinzeller

Loading