-
Notifications
You must be signed in to change notification settings - Fork 4
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
FA in fv_regional_bc and external_ic #17
Conversation
- In fv_regional_bc: initialize q_rimef and add the cloud physics auto conversion routine for FA scheme.
model/fv_sg.F90
Outdated
@@ -2138,7 +2166,9 @@ subroutine neg_adj2(is, ie, js, je, ng, kbot, hydrostatic, & | |||
qv(i,j,k) = qv2(i,j) | |||
ql(i,j,k) = ql2(i,j) | |||
qi(i,j,k) = qi2(i,j) | |||
if (present(qs)) then | |||
qs(i,j,k) = qs2(i,j) |
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.
adjust indent (add three spaces)
if (ntrac > ntracers) call mpp_error(FATAL,'==> External_ic::get_nggps_ic: more NGGPS tracers & | ||
&than defined in field_table '//trim(fn_gfs_ctl)//' for NGGPS IC') | ||
!mzhang: not in FA | ||
! if (ntrac > ntracers) call mpp_error(FATAL,'==> External_ic::get_nggps_ic: more NGGPS tracers & |
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.
Do we know why this test has been added there? Are there any potentially dangerous consequences if ntrac > ntracers?
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 max number of NGGPS tracers (ntrac) is predefined in gfs_ctrl.nc as 7. I guess that it was the max tracers supported at that time?
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.
Yeah, but why did they put the requirement there? Is there anything wrong if the number of tracers in the file is larger than what is used? I don't see what should be wrong, but you never know if there is some code elsewhere that relies on this condition. But maybe other code changes have made this check superfluous. Let's comment it out as you suggested and see what GFDL says when we want to merge it back.
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.
I don't know if the modification here will impact other places either.
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.
!zhang nwat=4
if ( Atm%flagstruct%nwat .eq. 4 ) then
do k=1,npz
BC_side%q_BC(i,j,k,rainwat) = 0.
BC_side%q_BC(i,j,k,q_rimef) = 0.
BC_side%q_BC(i,j,k,snowwat) = 0.
BC_side%q_BC(i,j,k,graupel) = 0.
.......
q_BC is defined by (:,:,:,1:ntracers). So I don't think snowwat and graupel need to be included here.
I am not sure if mp_auto_conversion_fa is necessary.
…an step - fix a delz bug in subroutine neg_adj2 - further optimize the use of q_rimef in fv_dynamic
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.
I am ok with these changes, but I would like to ask @ChunxiZhang-NOAA to look at this and approve, too. We will need PRs for fv3atm and ufs-weather-model for the submodule pointer updates; @mzhangw please create those to prepare for committing the code. Thanks!
… On Aug 5, 2020, at 4:31 PM, Dom Heinzeller ***@***.***> wrote:
@climbfuji commented on this pull request.
I am ok with these changes, but I would like to ask @ChunxiZhang-NOAA <https://github.com/ChunxiZhang-NOAA> to look at this and approve, too. We will need PRs for fv3atm and ufs-weather-model for the submodule pointer updates; @mzhangw <https://github.com/mzhangw> please create those to prepare for committing the code. Thanks!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#17 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG7TW2WDGL6TDZ57PWU6RALR7HMVZANCNFSM4PQ4JI7Q>.
|
For now, it does not impact all existing RTs and makes HWRF physics working with ntrac = 6 in field_table (no cld_amt).
Man
… On Aug 6, 2020, at 9:12 AM, ChunxiZhang-NOAA ***@***.***> wrote:
@ChunxiZhang-NOAA commented on this pull request.
In tools/external_ic.F90 <#17 (comment)>:
> @@ -570,8 +570,9 @@ subroutine get_nggps_ic (Atm, fv_domain, dt_atmos )
!--- read in the number of tracers in the NCEP NGGPS ICs
call read_data ('INPUT/'//trim(fn_gfs_ctl), 'ntrac', ntrac, no_domain=.TRUE.)
- if (ntrac > ntracers) call mpp_error(FATAL,'==> External_ic::get_nggps_ic: more NGGPS tracers &
- &than defined in field_table '//trim(fn_gfs_ctl)//' for NGGPS IC')
+!mzhang: not in FA
+! if (ntrac > ntracers) call mpp_error(FATAL,'==> External_ic::get_nggps_ic: more NGGPS tracers &
I don't know if the modification here will impact other places either.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#17 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG7TW2TWW3JS2HU7BCZYST3R7LB47ANCNFSM4PQ4JI7Q>.
|
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.
for fv_regional_bc.F90
!zhang nwat=4
if ( Atm%flagstruct%nwat .eq. 4 ) then
do k=1,npz
BC_side%q_BC(i,j,k,rainwat) = 0.
BC_side%q_BC(i,j,k,q_rimef) = 0.
BC_side%q_BC(i,j,k,snowwat) = 0.
BC_side%q_BC(i,j,k,graupel) = 0.
.......
q_BC is defined by (:,:,:,1:ntracers). So I don't think snowwat and graupel need to be included here.
I am not sure if mp_auto_conversion_fa is necessary.
if (ntrac > ntracers) call mpp_error(FATAL,'==> External_ic::get_nggps_ic: more NGGPS tracers & | ||
&than defined in field_table '//trim(fn_gfs_ctl)//' for NGGPS IC') | ||
!mzhang: not in FA | ||
! if (ntrac > ntracers) call mpp_error(FATAL,'==> External_ic::get_nggps_ic: more NGGPS tracers & |
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.
!zhang nwat=4
if ( Atm%flagstruct%nwat .eq. 4 ) then
do k=1,npz
BC_side%q_BC(i,j,k,rainwat) = 0.
BC_side%q_BC(i,j,k,q_rimef) = 0.
BC_side%q_BC(i,j,k,snowwat) = 0.
BC_side%q_BC(i,j,k,graupel) = 0.
.......
q_BC is defined by (:,:,:,1:ntracers). So I don't think snowwat and graupel need to be included here.
I am not sure if mp_auto_conversion_fa is necessary.
good catch!
… On Aug 6, 2020, at 9:49 AM, ChunxiZhang-NOAA ***@***.***> wrote:
@ChunxiZhang-NOAA commented on this pull request.
for fv_regional_bc.F90
!zhang nwat=4
if ( Atm%flagstruct%nwat .eq. 4 ) then
do k=1,npz
BC_side%q_BC(i,j,k,rainwat) = 0.
BC_side%q_BC(i,j,k,q_rimef) = 0.
BC_side%q_BC(i,j,k,snowwat) = 0.
BC_side%q_BC(i,j,k,graupel) = 0.
.......
q_BC is defined by (:,:,:,1:ntracers). So I don't think snowwat and graupel need to be included here.
I am not sure if mp_auto_conversion_fa is necessary.
In tools/external_ic.F90 <#17 (comment)>:
> @@ -570,8 +570,9 @@ subroutine get_nggps_ic (Atm, fv_domain, dt_atmos )
!--- read in the number of tracers in the NCEP NGGPS ICs
call read_data ('INPUT/'//trim(fn_gfs_ctl), 'ntrac', ntrac, no_domain=.TRUE.)
- if (ntrac > ntracers) call mpp_error(FATAL,'==> External_ic::get_nggps_ic: more NGGPS tracers &
- &than defined in field_table '//trim(fn_gfs_ctl)//' for NGGPS IC')
+!mzhang: not in FA
+! if (ntrac > ntracers) call mpp_error(FATAL,'==> External_ic::get_nggps_ic: more NGGPS tracers &
!zhang nwat=4
if ( Atm%flagstruct%nwat .eq. 4 ) then
do k=1,npz
BC_side%q_BC(i,j,k,rainwat) = 0.
BC_side%q_BC(i,j,k,q_rimef) = 0.
BC_side%q_BC(i,j,k,snowwat) = 0.
BC_side%q_BC(i,j,k,graupel) = 0.
.......
q_BC is defined by (:,:,:,1:ntracers). So I don't think snowwat and graupel need to be included here.
I am not sure if mp_auto_conversion_fa is necessary.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#17 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AG7TW2TNO5VYLIMCLNRBLVLR7LGIHANCNFSM4PQ4JI7Q>.
|
@ChunxiZhang-NOAA please submit a follow-up PR to correct whatever needs to be corrected. Thanks! |
* commit of new version of dycore from Weather and Climate Dynamics Group at GFDL * updated versions of GFDL-specific files from dev/gfdl * updated README.md with current release information * cleaned up a few lines in fv_dynamics * new file RELEASE.md with release notes documenting differences between this and the last release * updated RELEASE.md message * hand merge of diagnostic updates * remove trailing spaces from sources * updates to merge some GFDL specific updates into this public release
Pull GCC fix from develop to MAPL-2.0
This PR focus on two topics:
Full RTs passed except fv3_ccpp_gfdlmp_hwrfsas generated non-bit4bit results due to a known fix in CCPP HWRF physics.