Skip to content

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

Merged
DomHeinzeller merged 26 commits into
NOAA-GSL:gsl/developfrom
climbfuji:gsl_develop_hannah_and_joe_changes_combined_20210712
Jul 19, 2021
Merged

Wrapper PR for #97 (Update of GF aerosol treatment and tunings) and #98 (Updates to MYNN-EDMF)#99
DomHeinzeller merged 26 commits into
NOAA-GSL:gsl/developfrom
climbfuji:gsl_develop_hannah_and_joe_changes_combined_20210712

Conversation

@climbfuji
Copy link
Copy Markdown

@climbfuji climbfuji commented Jul 14, 2021

This PR combines the changes in #97 and #98 and adds one additional bugfix in cu_gf_deep.F90 to prevent qc(i,k) being zero.

Associated PRs:

#99
NOAA-GSL/fv3atm#101
NOAA-GSL/ufs-weather-model#95

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

Copy link
Copy Markdown

@hannahcbarnes hannahcbarnes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks for your help @DomHeinzeller. Glad to see that adding the 1e-8 factor in cu_gf_deep for qc and qch resolved the issue.

Copy link
Copy Markdown

@hannahcbarnes hannahcbarnes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks for your help @DomHeinzeller with the crashes.

Copy link
Copy Markdown
Collaborator

@SamuelTrahanNOAA SamuelTrahanNOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some issues in the code might get flagged by upstream reviewers, but they don't bother me much. I approve.

Comment thread physics/cu_gf_driver.F90 Outdated
flag_for_scnv_generic_tend,flag_for_dcnv_generic_tend, &
dtend,dtidx,ntqv,ntiw,ntcw,index_of_temperature,index_of_x_wind, &
index_of_y_wind,index_of_process_scnv,index_of_process_dcnv, &
index_of_y_wind,index_of_process_scnv,index_of_process_dcnv, &
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a trailing space on this line. That isn't a big deal, but it may annoy other reviewers when this goes upstream.

Comment thread physics/cu_gf_driver.F90 Outdated
do k=kts,ktf
do i=its,itf
tem = (cuten(i)*outq(i,k) + cutenm(i)*outqm(i,k))* dt
tem = (cuten(i)*outq(i,k) + cutenm(i)*outqm(i,k))* dt
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two trailing space on this line, which may bother upstream reviewers.

Comment thread physics/module_bl_mynn.F90 Outdated
b(1)=1.+dtz(k)*dfh(k+1) - 0.5*dtz(k)*s_aw(k+1)
c(1)=-dtz(k)*dfh(k+1) - 0.5*dtz(k)*s_aw(k+1)
d(1)=chem1(k,ic) + dtz(k) * -vd1(ic)*chem1(1,ic) - dtz(k)*s_awchem(k+1,ic)
!a(1)=0.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this commented-out code need to be in the repository?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly not. I'll remove it.

@@ -6226,7 +6591,7 @@ SUBROUTINE DMP_mf( &
edmf_ent(K)=edmf_ent(K)+UPA(K,i)*ENT(K,i)
edmf_qc(K) =edmf_qc(K) +UPA(K,i)*UPQC(K,i)
#if (WRF_CHEM == 1)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this dead code? Do we ever define WRF_CHEM=1 in FV3?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't used by me but maybe Ravan and Jordan use it. All WRF_CHEM if-defs will be removed soon as part of our ongoing work to "universalize" the MYNN to all dycores. This will be completed along with the merge with Laura Fowler's version of MYNN-EDMF. At that point, this WRF-CHEM code will be officially absorbed into the MYNN and probably regulated/protected by a new namelist option.

@DomHeinzeller
Copy link
Copy Markdown

All good points, @SamuelTrahanNOAA . Lots of cleanup work that needs to be done (at some point ...)

Copy link
Copy Markdown
Collaborator

@joeolson42 joeolson42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am never satisfied but I approve it anyway.

Copy link
Copy Markdown

@DomHeinzeller DomHeinzeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GSD warmstart runs fail with both Intel and GNU - this needs to be fixed before the code can be merged.

Copy link
Copy Markdown

@DomHeinzeller DomHeinzeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regression tests pass as required after several bug fixes.

@DomHeinzeller DomHeinzeller merged commit 9edfe02 into NOAA-GSL:gsl/develop Jul 19, 2021
@climbfuji climbfuji deleted the gsl_develop_hannah_and_joe_changes_combined_20210712 branch June 27, 2022 03:15
zhanglikate referenced this pull request in zhanglikate/ccpp-physics Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants