The new CCPP suite FV3_GFS_v16b_ugwpv1 with GFSv16b #550
Conversation
…_master_20201118 Update gsd/develop from master 2020/11/18
ValeryYudin-NOAA
requested review from
climbfuji,
grantfirl,
JulieSchramm and
llpcarson
as code owners
|
Why is there a change to RRTMGP in this PR? Does the submodule for rte-rrtmgp need to be updated on the PR branch? |
Yes, it seems like. |
|
Are these the correct associated PRs? If so, both the fv3atm and ufs-weather-model PRs have merge conflicts. |
|
Ok, I will work on this and give you the update next week, by Monday
and write smt more appropriate to you,
…On Fri, Jan 15, 2021 at 10:53 AM grantfirl ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In physics/ugwpv1_gsldrag.F90
<#550 (comment)>:
> + logical :: exists
+ real :: dxsg
+ integer :: k
+
+ character(len=*), intent(out) :: errmsg
+ integer, intent(out) :: errflg
+
+ ! Initialize CCPP error handling variables
+ errmsg = ''
+ errflg = 0
+!=============================================
+! 3 cases for ORO-schemes + NGWs:
+! gwd_opt => "1 and 2, 3, 22, 33'
+! 1) gsldrag: do_gsl_drag_ls_bl, do_gsl_drag_ss, do_gsl_drag_tofd, do_ugwp_v1
+! 2) CIRES-v1: do_ugwp_v1, do_ugwp_v1_orog_only, do_tofd, ldiag_ugwp
+!=============================================
Maybe this isn't the PR submitter's responsibility, but I am super
confused about how this scheme is configured. For example, in this comment,
it says that there are "3 cases for ORO-schemes + NGWs" and then there are
5 different valid values of gwd_opt. Then, 2 different sets of logicals are
written that presumably must all be true for the given configuration. As
part of this PR, in a comment, could we get a clear explanation of all
possible configurations of this scheme, including how the gwd_opt variable
should correspond to the various logicals?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#550 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOVBSB3ZVVE5OURUXVTNNWDS2B6I3ANCNFSM4V5JYGTQ>
.
|
physics/ugwpv1_gsldrag.F90
Outdated
| ! dusfcg = du_ogwcol + du_oblcol + du_osscol + du_ofdcol | ||
| ! | ||
| if (kdt <= 2 .and. me == master) then | ||
| print *, ' unified drag_suite_run ', kdt |
There was a problem hiding this comment.
Consider commenting these out or controlling by a debug flag for merging into master.
physics/ugwpv1_gsldrag.F90
Outdated
| ! | ||
| if (kdt <= 2 .and. me == master) then | ||
|
|
||
| print *, ' unified_ugwp orogw_v1 ', kdt, me, nmtvr |
physics/ugwpv1_gsldrag.F90
Outdated
| ! write(6,*) ' non-stationary GWs with GMAO/MERRA GW-forcing ' | ||
| print * | ||
|
|
||
| print *, ' ugwp_v1 ', kdt |
| kind = len=* | ||
| intent = in | ||
| optional = F | ||
| [jdat] |
There was a problem hiding this comment.
missing intent and optional attributes for jdat
physics/cires_ugwpv1_initialize.F90
Outdated
| ! rv => con_rv, cpd => con_cp, fv => con_fvirt,& | ||
| ! arad => con_rerth | ||
| implicit none | ||
|
|
There was a problem hiding this comment.
Are these constants being used internally in the scheme instead of those being passed in via the CCPP? If so, that defeats the purpose of passing the constants in via the argument list, right? There should be no reason why every scheme needs its own version of most of these.
physics/cires_ugwpv1_initialize.F90
Outdated
| ktg(k) = ktg(k-1) | ||
|
|
||
| if (me == master) then | ||
| write(6, * ) ' zkm(k), kvg(k), kvg(k)*(6.28/5000.)**2, kion(k) ... init_global_gwdis' |
There was a problem hiding this comment.
Consider commenting out or controlling via a debug flag for merge into master.
physics/cires_ugwpv1_initialize.F90
Outdated
| allocate (lzmet(nwav), czmet(nwav), mkzmet(nwav), dczmet(nwav), dmkz(nwav) ) | ||
|
|
||
| if (me == master) then | ||
| print *, 'ugwp_v1/v0: init_gw_wmsdis_control ' |
There was a problem hiding this comment.
Same comment RE: print statements.
| rcstar = 1./cstar | ||
|
|
||
| if (me == master) then | ||
| print * |
There was a problem hiding this comment.
Same comment RE: print statements.
|
|
||
| contains | ||
|
|
||
| subroutine read_tau_amf(me, master, errmsg, errflg) |
There was a problem hiding this comment.
Please confirm that if this subroutine is called, it does not get invoked from a "run" stage (reading tables should only take place in the "init" stage).
There was a problem hiding this comment.
Confirm, it will be a part of init-n, as it was before Jan 6 update of in-n
physics/cires_tauamf_data.F90
Outdated
|
|
||
|
|
||
| if (me == master) then | ||
| print * |
There was a problem hiding this comment.
same comment RE: print statements
physics/cires_tauamf_data.F90
Outdated
| if (me == master ) then | ||
|
|
||
| 223 format( 2x, 'vay-limb', I4, 5(2x, F10.3)) | ||
| print *, 'ugwp-v1 indx_ugwp ', size(dlat), ' npts ', npts |
There was a problem hiding this comment.
same comment RE: print statements
| print *, ' Error in time-interpolation for tau_amf_interp ' | ||
| print *, ' it1, it2, ntau_d2t ', it1, it2, ntau_d2t | ||
| print *, ' Error in time-interpolation see cires_tauamf_data.F90 ' | ||
| stop |
There was a problem hiding this comment.
Physics within the CCPP should not be allowed to stop the model. Set the CCPP error flag to something other than 0 and put the print statements into the CCPP error message variable and return.
There was a problem hiding this comment.
I realize that this isn't a CCPP-facing subroutine. That means that the CCPP error handling variables should be passed down into this subroutine.
physics/cires_ugwpv1_module.F90
Outdated
| ! | ||
| ! | ||
| use machine, only : kind_phys | ||
| use ugwp_common, only : arad, pi, pi2, hpscale, rhp, rhp2, rh4 |
There was a problem hiding this comment.
Why does this scheme need its own version of pi? It's fine to have scheme-specific constants, but for physical constants that should be shared across all physics, including things like pi, please pass them in as arguments -- not only to the CCPP interface routines, but down to the internal scheme code as necessary.
There was a problem hiding this comment.
I will follow my style in the module type, otherwise I need to rewrite my subs to satisfy your CCPP-ruless passing
all variables through the CALL-statements. Passing the con_pi, con_g ....etc we can do for the high-level drivers if you need
| open (unit = nlunit, file = trim(fn_nml), action = 'read', status = 'old', iostat = ios) | ||
| endif | ||
| rewind (nlunit) | ||
| read (nlunit, nml = cires_ugwp_nml) |
There was a problem hiding this comment.
I'm assuming that this subroutine only every gets called during the CCPP init phase. Please confirm.
|
|
||
| launch_level = max(k-1, 5) ! above 5-layers from the surface | ||
| if (me == master) then | ||
| print *, 'cires_ugwpv1 klev_ngw =', launch_level, nint(pmb(launch_level)) |
There was a problem hiding this comment.
Same comment RE: print statements.
physics/cires_ugwpv1_oro.F90
Outdated
|
|
||
|
|
||
| if ( kdt == -1 .and. me == master) then | ||
| print *, ' orogw_v1 scale2 ', cdmbgwd(2) |
There was a problem hiding this comment.
Same comment RE: print statements.
physics/cires_ugwpv1_oro.F90
Outdated
| ! | ||
| ! fcrit_v1/fr_flow | ||
| ! | ||
| goto 788 |
There was a problem hiding this comment.
Is there a way to not use GOTOs and achieve the same thing?
There was a problem hiding this comment.
I will take care on this
physics/cires_ugwpv1_oro.F90
Outdated
| fr = heff/zw1 ! Fr-GSL = Fr * OD -> gamma | ||
|
|
||
| fr = min(fr, frmax) | ||
| fr2 = fr*fr |
There was a problem hiding this comment.
Minor nitpick: the whitespace in this section (tabs) is inconsistent and makes this code hard to read, IMO.
There was a problem hiding this comment.
What is "IMO?
|
|
||
| !============ debug ------------------------------------------------ | ||
| if (kdt <= 2 .and. me == 0) then | ||
| print *, 'vgw-oro done gwdps_v0 in ugwp-v0 step-proc ', kdt, me |
There was a problem hiding this comment.
same comment RE: print statements.
| ! | ||
| enddo | ||
| print * | ||
| stop |
There was a problem hiding this comment.
Please don't stop the model -- use ccpp error handling to pass back a non-zero error flag and return.
| ! vtofd(k) = -v(k)/(1. + var_temp*krf | ||
| ! enddo | ||
|
|
||
| do k=1, levs |
There was a problem hiding this comment.
There is more inconsistent tabbing in this section that makes this code hard to read.
|
|
||
| use cires_ugwpv1_module,only : maxdudt, maxdtdt, max_eps, dked_min, dked_max | ||
|
|
||
| use ugwp_common , only : rgrav, grav, cpd, rd, rv, rcpdl, grav2cpd, & |
There was a problem hiding this comment.
Lots of constants here that are repetition of those that should be passed in via the CCPP.
physics/cires_ugwpv1_solv2.F90
Outdated
| ! | ||
| implicit none | ||
| ! | ||
| real(kind=kind_phys), intent(in) :: con_g, con_cp, con_rd, con_rv, con_omega, con_pi, con_fvirt |
There was a problem hiding this comment.
Some constants are passed in like the CCPP expects, but then they are also available via ugwp_common above. This is confusing, IMO.
There was a problem hiding this comment.
Someone from your CCPP-team put them before me in the call sequence polishing my origimal codes
physics/cires_ugwpv1_solv2.F90
Outdated
| ! test for input fields | ||
| ! | ||
| if (mpi_id == master .and. kdt < -2) then | ||
| print *, im, levs, dtp, kdt, ' vay-solv2-v1' |
There was a problem hiding this comment.
same comment RE: print statements
physics/cires_ugwpv1_solv2.F90
Outdated
| ! --------------------------------------------- | ||
| do jk= km1,levs | ||
| tvc = atm(jk) * (1. +fv*aqm(jk)) | ||
| tvm = atm(jk-1) * (1. +fv*aqm(jk-1)) |
There was a problem hiding this comment.
more inconsistent tabbing that makes this section hard to read
|
Grant-
Thanks for the great Review !
Please let me know (send me e-mail) when you will finish ( I got 34 e-mails
from your side).
I will clean up all debugging prints pushed due to the "original"
ccpp-style to recompute
constant-related variables during the execution etc...(ccpp vs ipd
differences) and use of meta files.
*Questions and clarification:*
*"There is more inconsistent tabbing in this section that makes this code
hard to read"*
Please CLARIFY about TABBING show me please what You meant !
*Bad line => Good line, simple example*
*On constants:*
*The con_pi and con_g etc.. are "great" constants that belong to the
CCPP-metafile descriptions !!!*
*They deserve to be in the "ARGUMENT" list of ALL CCPP subroutines after
you disregard the use phys_cons !*
*I saw many times how CCPP-subs recompute inside (during the runtime) some
constant-derived variables*
*that's why I'm using the ugwp_common and avoid this "?-able" style for
model optimization (during init-n).*
Example:
use ugwp_common, only : arad, pi, pi2, hpscale, rhp, rhp2, rh4
to avoid like
*pi2 = 2.*con_pi*
Cheers,
…-Valery
On Fri, Jan 15, 2021 at 12:36 PM grantfirl ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In physics/cires_ugwpv1_module.F90
<#550 (comment)>:
> @@ -0,0 +1,557 @@
+
+module cires_ugwpv1_module
+
+!
+! driver is called after pbl & before chem-parameterizations
+! it uses ugwp_common (like phys_cons) and some module-param od solvers/sources init-modules
+!....................................................................................
+! order = dry-adj=>conv=mp-aero=>radiation -sfc/land- chem -> vertdiff-> [rf-gws]=> ion-re
+!...................................................................................
+!
+!
+ use machine, only : kind_phys
+ use ugwp_common, only : arad, pi, pi2, hpscale, rhp, rhp2, rh4
Why does this scheme need its own version of pi? It's fine to have
scheme-specific constants, but for physical constants that should be shared
across all physics, including things like pi, please pass them in as
arguments -- not only to the CCPP interface routines, but down to the
internal scheme code as necessary.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#550 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOVBSB7M5QHP6QY2E3SA3YLS2CKLLANCNFSM4V5JYGTQ>
.
|
|
Sorry, I'm not fluent with your git-style communication:
*Are these the correct associated PRs?*
*NOAA-EMC/fv3atm#227 <NOAA-EMC/fv3atm#227>*
*ufs-community/ufs-weather-model#360
<ufs-community/ufs-weather-model#360>*
*If so, both the fv3atm and ufs-weather-model PRs have merge conflicts.*
Could you please explain me what you have in mind
about *"both the fv3atm and ufs-weather-model PRs have merge conflicts".*
*On Dec 29 Jun explained to me the following:: "you do need to make PR
under CCPP, fv3atm and ufs-weather-model, also make sure the branch on top
repository is pointing to your branch on the lower repository. Thanks. Jun"*
…-Valery
On Fri, Jan 15, 2021 at 1:10 PM Valery Yudin - NOAA Affiliate <
valery.yudin@noaa.gov> wrote:
Grant-
Thanks for the great Review !
Please let me know (send me e-mail) when you will finish ( I got 34
e-mails from your side).
I will clean up all debugging prints pushed due to the "original"
ccpp-style to recompute
constant-related variables during the execution etc...(ccpp vs ipd
differences) and use of meta files.
*Questions and clarification:*
*"There is more inconsistent tabbing in this section that makes this code
hard to read"*
Please CLARIFY about TABBING show me please what You meant !
*Bad line => Good line, simple example*
*On constants:*
*The con_pi and con_g etc.. are "great" constants that belong to the
CCPP-metafile descriptions !!!*
*They deserve to be in the "ARGUMENT" list of ALL CCPP subroutines after
you disregard the use phys_cons !*
*I saw many times how CCPP-subs recompute inside (during the runtime)
some constant-derived variables*
*that's why I'm using the ugwp_common and avoid this "?-able" style for
model optimization (during init-n).*
Example:
use ugwp_common, only : arad, pi, pi2, hpscale, rhp, rhp2, rh4
to avoid like
*pi2 = 2.*con_pi*
Cheers,
-Valery
On Fri, Jan 15, 2021 at 12:36 PM grantfirl ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In physics/cires_ugwpv1_module.F90
> <#550 (comment)>:
>
> > @@ -0,0 +1,557 @@
> +
> +module cires_ugwpv1_module
> +
> +!
> +! driver is called after pbl & before chem-parameterizations
> +! it uses ugwp_common (like phys_cons) and some module-param od solvers/sources init-modules
> +!....................................................................................
> +! order = dry-adj=>conv=mp-aero=>radiation -sfc/land- chem -> vertdiff-> [rf-gws]=> ion-re
> +!...................................................................................
> +!
> +!
> + use machine, only : kind_phys
> + use ugwp_common, only : arad, pi, pi2, hpscale, rhp, rhp2, rh4
>
> Why does this scheme need its own version of pi? It's fine to have
> scheme-specific constants, but for physical constants that should be shared
> across all physics, including things like pi, please pass them in as
> arguments -- not only to the CCPP interface routines, but down to the
> internal scheme code as necessary.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#550 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AOVBSB7M5QHP6QY2E3SA3YLS2CKLLANCNFSM4V5JYGTQ>
> .
>
|
Great work, Valery. I think that I've finished going through this PR for CCPP-compliancy purposes, although I may have missed something given how many lines of code there are here. Here are some clarifications: The tabbing comments are just related to consistently starting lines within "if" blocks or do-loops on the same column. This is likely just due to differences in how various text editors handle tabs, but if you look at the code that I commented on it GitHub's interface (e.g., in the orogw_v1 subroutine of cires_ugwpv1_oro.F90), you'll see that the beginnings of many lines within one do-loop changes quite a bit. It's just easier to read, if all of the lines that get executed together start in the same column. On the constants, there is definitely no problem with defining your own derived constants, like 2*pi. But the ugwp_common also defines its own set of physical constants, some of which are repeats of the ones passed in via the CCPP argument lists. I didn't go through the code carefully enough to see if any of the repeat constants in ugwp_common were actually used instead of the once passed in via the CCPP argument list. One could find out by deleting the repeat physical constants from ugwp_common altogether and see if there are any compilation errors. |
If you go to the linked pull requests for fv3atm and ufs-weather-model, at the bottom, you will see that GitHub says "This branch has conflicts that must be resolved". It also lists what files have conflicts. This just means that your pull request branches can't be merged into the target branches without manually merging the conflicted files. In practice, this usually means that your pull request branches are not up-to-date with the latest master (or whatever branch that you're trying to merge into). |
|
Thank you, Grant
To my understanding my branches were "ahead" of "masters/develops" from
which I forked them..
I need *the direct instructions from you or Dom and Jun how to resolve
these "conflicts*" or understand
what is wrong (out of date).
I cannot resolve them by myself.
…-Valery
=================
I thought I made/updated for everything after the "post New Year commit"
of Dom, Jan 6/7,
a new style of initialization, spending weekends fixing
GFS_typedefs.F90/meta ( thousands of lines)
and repeating RTs.
=================
On Fri, Jan 15, 2021 at 1:40 PM grantfirl ***@***.***> wrote:
Sorry, I'm not fluent with your git-style communication: *Are these the
correct associated PRs?* *NOAA-EMC/fv3atm#227
<NOAA-EMC/fv3atm#227> <NOAA-EMC/fv3atm#227
<NOAA-EMC/fv3atm#227>>* *ufs-community/ufs-weather-model#360
<ufs-community/ufs-weather-model#360>
<ufs-community/ufs-weather-model#360
<ufs-community/ufs-weather-model#360>>* *If so,
both the fv3atm and ufs-weather-model PRs have merge conflicts.* Could
you please explain me what you have in mind about *"both the fv3atm and
ufs-weather-model PRs have merge conflicts".* *On Dec 29 Jun explained to
me the following:: "you do need to make PR under CCPP, fv3atm and
ufs-weather-model, also make sure the branch on top repository is pointing
to your branch on the lower repository. Thanks. Jun"*
… <#m_-4567710585367801139_>
-Valery On Fri, Jan 15, 2021 at 1:10 PM Valery Yudin - NOAA Affiliate <
***@***.***> wrote:
Grant- Thanks for the great Review ! Please let me know (send me e-mail)
when you will finish ( I got 34 e-mails from your side). I will clean up
all debugging prints pushed due to the "original" ccpp-style to recompute
constant-related variables during the execution etc...(ccpp vs ipd
differences) and use of meta files. *Questions and clarification:* *"There
is more inconsistent tabbing in this section that makes this code hard to
read"* Please CLARIFY about TABBING show me please what You meant ! *Bad
line => Good line, simple example* *On constants:* *The con_pi and con_g
etc.. are "great" constants that belong to the CCPP-metafile descriptions
!!!* *They deserve to be in the "ARGUMENT" list of ALL CCPP subroutines
after you disregard the use phys_cons !* *I saw many times how CCPP-subs
recompute inside (during the runtime) some constant-derived variables* *that's
why I'm using the ugwp_common and avoid this "?-able" style for model
optimization (during init-n).* Example: use ugwp_common, only : arad, pi,
pi2, hpscale, rhp, rhp2, rh4 to avoid like *pi2 = 2.con_pi Cheers,
-Valery On Fri, Jan 15, 2021 at 12:36 PM grantfirl @.**> wrote: > @.***
commented on this pull request. > ------------------------------ > > In
physics/cires_ugwpv1_module.F90 > <#550 (comment)
<#550 (comment)>>: >
> > @@ -0,0 +1,557 @@ > + > +module cires_ugwpv1_module > + > +! > +!
driver is called after pbl & before chem-parameterizations > +! it uses
ugwp_common (like phys_cons) and some module-param od solvers/sources
init-modules >
+!....................................................................................
> +! order = dry-adj=>conv=mp-aero=>radiation -sfc/land- chem -> vertdiff->
[rf-gws]=> ion-re >
+!...................................................................................
> +! > +! > + use machine, only : kind_phys > + use ugwp_common, only :
arad, pi, pi2, hpscale, rhp, rhp2, rh4 > > Why does this scheme need its
own version of pi? It's fine to have > scheme-specific constants, but for
physical constants that should be shared > across all physics, including
things like pi, please pass them in as > arguments -- not only to the CCPP
interface routines, but down to the > internal scheme code as necessary. >
> — > You are receiving this because you authored the thread. > Reply to
this email directly, view it on GitHub > <#550 (review)
<#550 (review)>>,
> or unsubscribe >
https://github.com/notifications/unsubscribe-auth/AOVBSB7M5QHP6QY2E3SA3YLS2CKLLANCNFSM4V5JYGTQ
> . >
If you go to the linked pull requests for fv3atm and ufs-weather-model, at
the bottom, you will see that GitHub says "This branch has conflicts that
must be resolved". It also lists what files have conflicts. This just means
that your pull request branches can't be merged into the target branches
without manually merging the conflicted files. In practice, this usually
means that your pull request branches are not up-to-date with the latest
master (or whatever branch that you're trying to merge into).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#550 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOVBSB2HDR6DBATOD4FECQDS2CR43ANCNFSM4V5JYGTQ>
.
|
If you started your branches from masters/develops, they were definitely "ahead" at that time, but it looks like since your pull requests were submitted, some others got accepted/merged. So, unless you continually update your pull request branch with the master/develop until it is merged, it will always be "behind" until it gets merged or other development stops. Keeping your pull request branch updated should just be a matter of doing a 'git pull' from masters/develops to pull in the latest masters/develops and then a 'git push' to push it back up to GitHub, but I'll defer to the fv3atm and ufs-weather-model code managers @climbfuji @junwang-noaa to help you get this squared away. |
[submodule "physics/rte-rrtmgp"] |
|
Maybe this isn't the PR submitter's responsibility, but I am super confused
about how this scheme is configured. For example, in this comment, it says
that there are "3 cases for ORO-schemes + NGWs" and then there are 5
different valid values of gwd_opt. Then, 2 different sets of logicals are
written that presumably must all be true for the given configuration. As
part of this PR, in a comment, could we get a clear explanation of all
possible configurations of this scheme, including how the gwd_opt variable
should correspond to the various logicals?
*REPLY: on gwd_opt*
!============================================================================
!
! gwd_opt => "1 and 2, 3, 22, 33' see previous GSL-commits to cccp Nov
2020
! related to GSL-oro drag suite
! *for use of the new-GSL/old-GFS/EMC inputs for sub-grid orography *
! see details inside /ufs-weather-model/FV3/io/FV3GFS_io.F90
! FV3GFS_io.F90: if (Model%gwd_opt==3 .or. Model%gwd_opt==33 .or. &
! FV3GFS_io.F90: Model%gwd_opt==2 .or. Model%gwd_opt==22 ) then
! FV3GFS_io.F90: if ( (Model%gwd_opt==3 .or. Model%gwd_opt==33)
.or. &
! FV3GFS_io.F90: ( (Model%gwd_opt==2 .or. Model%gwd_opt==22)
.and. &
!
! gwd_opt=1 -current 14-element GFS-EMC subgrid-oro input
! gwd_opt=2 and 3 24-element -current 14-element GFS-EMC subgrid-oro
input
! GSL uses the gwd_opt flag to control "extra" diagnostics (22 and 33) for
gwd_opt =2 and 3
! see drag_suite.F90
! CCPP may use gwd_opt to determine 14 or 24 variables for the input
! but at present you work with "nmtvr" , see related code
!
! GFS_GWD_generic.F90: integer, intent(in) :: im, levs, nmtvr
!GFS_GWD_generic.F90: real(kind=kind_phys), intent(in) ::
mntvar(im,nmtvr)
!GFS_GWD_generic.F90: * if (nmtvr == 14) then ! gwd_opt=1 current
operational - as of 2014*
!GFS_GWD_generic.F90: elseif (nmtvr == 10) then ????
!GFS_GWD_generic.F90: elseif (nmtvr == 6) then ????
!GFS_GWD_generic.F90: *elseif (nmtvr == 24) then ! GSD_drag_suite
and unified_ugwp gwd_opt=2,3*
!
! 1) gsldrag: do_gsl_drag_ls_bl, do_gsl_drag_ss, do_gsl_drag_tofd,
do_ugwp_v1
! 2) CIRES-v1: do_ugwp_v1, do_ugwp_v1_orog_only, do_tofd,
ldiag_ugwp
!==============================================================================
…On Fri, Jan 15, 2021 at 10:53 AM grantfirl ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In physics/ugwpv1_gsldrag.F90
<#550 (comment)>:
> + logical :: exists
+ real :: dxsg
+ integer :: k
+
+ character(len=*), intent(out) :: errmsg
+ integer, intent(out) :: errflg
+
+ ! Initialize CCPP error handling variables
+ errmsg = ''
+ errflg = 0
+!=============================================
+! 3 cases for ORO-schemes + NGWs:
+! gwd_opt => "1 and 2, 3, 22, 33'
+! 1) gsldrag: do_gsl_drag_ls_bl, do_gsl_drag_ss, do_gsl_drag_tofd, do_ugwp_v1
+! 2) CIRES-v1: do_ugwp_v1, do_ugwp_v1_orog_only, do_tofd, ldiag_ugwp
+!=============================================
Maybe this isn't the PR submitter's responsibility, but I am super
confused about how this scheme is configured. For example, in this comment,
it says that there are "3 cases for ORO-schemes + NGWs" and then there are
5 different valid values of gwd_opt. Then, 2 different sets of logicals are
written that presumably must all be true for the given configuration. As
part of this PR, in a comment, could we get a clear explanation of all
possible configurations of this scheme, including how the gwd_opt variable
should correspond to the various logicals?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#550 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOVBSB3ZVVE5OURUXVTNNWDS2B6I3ANCNFSM4V5JYGTQ>
.
|
|
Valery closed this PR inadvertently. |
|
Ligia-
^Thank you!
-Valery
…On Mon, Jan 18, 2021 at 1:03 PM ligiabernardet ***@***.***> wrote:
Reopened #550 <#550>.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#550 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AOVBSB737RVOH63BABZE3KLS2SHX5ANCNFSM4V5JYGTQ>
.
|
|
A visual explanation of the effects of the various "do_ugwp_xxx" namelist options can be found at: |
* Add 'time_iso' variable to 6-tile history files * Add '_Encoding' attribute to time_iso Co-authored-by: jswhit2 <Jeffrey.S.Whitaker@noaa.gov>
he new CCPP suite FV3_GFS_v16b_ugwpv1 and regression tests for warm_start and cold_starts
with GFSv16b (low resolution, C96_L127). The new suite represents the upgrade of uGWP (V1),CU-CIRES. It is capable to reproduce the realistic FV3GFS climate, including equatorial dynamics (QBO and SAO, 20-60 km)) and annual cycles in the extra-tropics and polar regions in the upper atmsosphere (20-80km). It can be executed and tested with the gsldrag sub-grid scale orography physics of gsl/develop for UFS-R2O planned activity in 2021.
Issue(s) addressed
issues to be closed with this PR in repositories : ufs-weather-model #347 issue
fv3atm #222 issue
ccpp-physics #542 issue
Testing
How were these changes tested? answer: hera
What compilers / HPCs was it tested with? answer: intel
Are the changes covered by regression tests? answer: yes (new baseline for UFS-127L-C96 see ufsw-weather-model/tests)
Have the ufs-weather-model regression test been run? answer: hera-intel