-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for cloud microphysics hail species #171
Conversation
Addresses compile failure when OMP is turned on (e.g., on Hera)
(Removed SFE code for now)
…in external_ic.F90 to check value of nwat
@@ -849,7 +862,44 @@ subroutine fv_dynamics(npx, npy, npz, nq_tot, ng, bdt, consv_te, fill, | |||
used = send_data(idiag%id_mdt, dtdt_m, fv_time) | |||
endif | |||
|
|||
if( nwat==6 ) then | |||
if( nwat==7 ) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the right way to add a new nwat to the code. I am hoping that eventually there can be a more generic way to do this though so that we don't need separate hard codings for different numbers of ice categories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Something like metadata for the tracers, where an integer flag could indicate liquid (1), ice (2), or non-hydrometeor (0).
I noticed that the SCM sets 'nwat' in the GFS_typedefs routine. Perhaps a similar approach could be done here and avoid needing to set both imp_physics and nwat in the namelist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would work. There should be an easy way for the physics to query the dynamics for the nwat value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nwat value is already being shared with the physics during initialization. The same mechanisms used by the IPD is in place for the CCPP, the Init_parm struct has an %nwat entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bensonr Right, nwat is being passed to model%init as intent(in). It looks like it is used before that, so it might take some changes to be able to set it in model%init. Something to look at for a future update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good addition that provides essential functionality (more schemes now have separate hail and graupel categories, something that may be necessary in future global storm resolving models) and correctly follows existing code standards. I approve this merge if it passes the appropriate regression tests.
model/fv_regional_bc.F90
Outdated
BC_side%q_BC(i,j,k,snowwat) = 0. | ||
BC_side%q_BC(i,j,k,graupel) = 0. | ||
BC_side%q_BC(i,j,k,hailwat) = 0. | ||
if ( BC_side%pt_BC(i,j,k) > 273.16 ) then ! > 0C all liq_wat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a clever way of establishing boundary conditions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking again at this, a separate nwat=7 section seems unnecessary. There can be just one section with
if ( Atm%flagstruct%nwat .eq. 6 .or. Atm%flagstruct%nwat .eq. 7 ) then
if ( Atm%flagstruct%nwat .eq. 7 ) BC_side%q_BC(is:ie,j,1:npz,hailwat) = 0.
do k=1,npz
...
@@ -1878,6 +1904,364 @@ subroutine neg_adj3(is, ie, js, je, ng, kbot, hydrostatic, peln, delz, pt, dp, | |||
|
|||
end subroutine neg_adj3 | |||
|
|||
! TAS: XXX check to make sure this doesn't need any modifications vs neg_adj2 or neg_adj3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this routine does not actually do any fixing to qh. This might be OK, although then there isn't much reason for the new routine.
The original reason for the neg_adj was because the older GFS physics created negative tracer values, which this then fixed. If the physics is no longer doing that then the routine may be unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good catch. (@tsupinie - thoughts?) The microphysics scheme here should not output negative hail mass values (below-threshold mass values are moved to water vapor), we could call neg_adj3 instead? Should we assume the same for other new microphysics schemes that have hail (nwat=7)?
@@ -228,6 +228,14 @@ subroutine populate_coarse_diag_type(Atm, coarse_diagnostics) | |||
coarse_diagnostics(index)%units = 'kg/kg/s' | |||
coarse_diagnostics(index)%reduction_method = MASS_WEIGHTED | |||
|
|||
index = index + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding this. It would be neat if more people are using the coarse-graining as an output option.
@@ -4443,6 +4472,9 @@ subroutine prt_mass(km, nq, is, ie, js, je, n_g, nwat, ps, delp, q, area, domain | |||
write(*,*) 'Total snow ', trim(gn), '=', qtot(snowwat)*ginv | |||
if (graupel > 0) & | |||
write(*,*) 'Total graupel ', trim(gn), '=', qtot(graupel)*ginv | |||
if (hailwat > 0) & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good to see all of the stdout diagnostics being correctly implemented. You have done a very careful job.
Hi, Ted. Another thing you can do is to check if the 'hailwat' tracer is
defined. If the tracer is not defined then get_tracer_index('hailwat') will
return a negative value.
Lucas
…On Mon, Jan 31, 2022 at 11:49 AM Ted Mansell ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In model/fv_regional_bc.F90
<#171 (comment)>
:
> @@ -3808,7 +3811,51 @@ subroutine remap_scalar_nggps_regional_bc(Atm &
enddo
enddo
endif
- endif ! data source /= FV3GFS GAUSSIAN NEMSIO/NETCDF and GRIB2 FILE
+ if ( Atm%flagstruct%nwat .eq. 7 ) then
+ do k=1,npz
+ do i=is,ie
+ qn1(i,k) = BC_side%q_BC(i,j,k,liq_wat)
+ BC_side%q_BC(i,j,k,rainwat) = 0.
+ BC_side%q_BC(i,j,k,snowwat) = 0.
+ BC_side%q_BC(i,j,k,graupel) = 0.
+ BC_side%q_BC(i,j,k,hailwat) = 0.
+ if ( BC_side%pt_BC(i,j,k) > 273.16 ) then ! > 0C all liq_wat
Looking again at this, a separate nwat=7 section seems unnecessary. There
can be just one section with
if ( Atm%flagstruct%nwat .eq. 6 .or. Atm%flagstruct%nwat .eq. 7 ) then
if ( Atm%flagstruct%nwat .eq. 7 ) BC_side%q_BC(is:ie,j,1:npz,hailwat) = 0.
do k=1,npz
...
—
Reply to this email directly, view it on GitHub
<#171 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMUQRVD4D4YHCZRWC7V2DQ3UY24RZANCNFSM5NB6624Q>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you commented.Message ID:
***@***.***>
|
Good point, Lucas. Although currently the assumption is that hail would only be in a 7-class scheme, future ones may not conform to that, so switching toward checking for categories generally may make it easier to incorporate options like P3 in the future. I have experimented with using a combined ice+snow category, as well. This particular section seems to be set up for a special case (expecting only nwat=6), so the mods are just to be sure all bases are covered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new updates are good. Thanks.
@junwang-noaa This PR has been fully reviewed and is ready to be added to the commit queue and merge. Please let me know when you would like me to merge. |
OK, I have updated to dev/emc, but I'm having trouble getting the whole thing to compile (e.g., module_wrt_grid_comp.F90) (also updated fv3atm to current develop) Turns out to be an issue with io/inline_post_stub.F90 not having subroutine interface up to date compared to inline_post.F90. |
@MicroTed The PR#173 was committed, would you please merge to the top of dev/emc branch? |
Yes, it should be there. |
@junwang-noaa I believe this is on the commit queue for today. Can I merge this now? |
@laurenchilutti We are still running RT. Please let you know when all the tests finishes. |
@laurenchilutti Would you please merge this PR? Thanks |
* Add get_nth_domain_info subroutine * Deallocate local arrays in fv_dyn_bundle_setup * adding back a line that was mistakenly deleted in a previous commit (NOAA-GFDL#166) * Do not print debug messages to stderr * fix for 4diau with iau_filter_increments=T (NOAA-GFDL#167) * fix for 4diau with iau_filter_increments=T * fix time interval for 3DIAU * fix typo in comment * fix bug found in review by @MingjingTong-NOAA * change tnext to integer variable itnext Co-authored-by: jswhit2 <Jeffrey.S.Whitaker@noaa.gov> * Use mpp_error instead of write statements in model/fv_regional_bc.F90 * Attempt at integrating fixes on top of dev/emc branch. (NOAA-GFDL#173) * Support for cloud microphysics hail species (NOAA-GFDL#171) * Incorporated Tim Supinie's updates to handle a hail category (nwat=7) * Added print for hail max/min (non-debug section) * Add hallwat to !OMP shared variables in main loop Addresses compile failure when OMP is turned on (e.g., on Hera) * Update to states as used in SFE2021 (Removed SFE code for now) * Cleanup after rebase * Redo removal of alt. namelist read * Removed tools/module_diag_hailcast.F90 * Fixes from Jili Dong * Add hailwat to tools/external_ic.F90 * Set logic in fv_regional_bc.F90 to match current code; Change logic in external_ic.F90 to check value of nwat * Recommended changes * Check actual index (hailwat) instead of assuming a positive value based on 'nwat' value * Split nwat=7 bits into separate loops (see if it affects nwat=6) Co-authored-by: Larissa Reames <52886575+LarissaReames-NOAA@users.noreply.github.com> Co-authored-by: LarissaReames-NOAA <larissa.reames@noaa.gov> Co-authored-by: Dusan Jovic <dusan.jovic@noaa.gov> Co-authored-by: laurenchilutti <60401591+laurenchilutti@users.noreply.github.com> Co-authored-by: Jeff Whitaker <jswhit@fastmail.fm> Co-authored-by: jswhit2 <Jeffrey.S.Whitaker@noaa.gov> Co-authored-by: MatthewPyle-NOAA <48285220+MatthewPyle-NOAA@users.noreply.github.com> Co-authored-by: Ted Mansell <37668594+MicroTed@users.noreply.github.com> Co-authored-by: Larissa Reames <52886575+LarissaReames-NOAA@users.noreply.github.com> Co-authored-by: LarissaReames-NOAA <larissa.reames@noaa.gov>
Description
Adds support for hail hydrometeor category (nwat = 7) for 7-class cloud microphysics. Specifically for the NSSL microphysics scheme, but should work for any microphysics that has separate graupel and hail/frozen drops categories.
Related PR for CCPP physics: NCAR/ccpp-physics#761
Related PR for Single Column Model: NCAR/ccpp-scm#277
Related PR of fv3atm: NOAA-EMC/fv3atm#472
How Has This Been Tested?
Tested to work with fv3atm develop branch on Macos (gcc-10) and Unix (NOAA/Jet, Intel compilers)
Cases run with "grad student test" case (2019061518) and RRFS domain tests
Has not yet been ported to dev/gfdl branch, but fv3atm should still compile and run with other suites that have nwat <= 6.
Checklist:
Changes follow or are similar to previous work by Tim Supinie @tsupinie, who deserves much of the credit here. Also contributions by @LarissaReames-NOAA