Unified ugwp#55
Conversation
…nto unified_ugwp Merging unified_ugwp branch with latest gsd/develop updates.
DomHeinzeller
left a comment
DomHeinzeller
left a comment
There was a problem hiding this comment.
This is a beautiful, clean implementation of the combined gravity wave drag schemes. I was very picky along the way to request passing constants used deep inside the physics from the host model, and to reduce clutter to stdout/stderr if possible. Plus a few other, small changes.
Here is my suggestion to address the constants and stdout/stderr concerns:
Constants
It seems everything draws constants from module ugwp_common_v1. If we could find a way to set those constants as part of/first task of the init call, then we can leave all code unchanged and still get the host model constants.
This will most likely change the answer, so we should make this change in a controlled way: first run the tests to verify that all existing regression tests pass when using internal constants. Then swap to use host model constants (maybe even using #ifdef statements in ugwp_common_v1 and make sure by manual inspection that the results are still acceptable.
stdout/stderr
There is a large amount of output written to stdout and stderr in the current code. This may be usedul for development, testing and debugging. I would wrap all but the absolutely necessary print statements in an if (debug) block and then have a single debug variable, maybe in ugwp_common_v1, that can be set to .true. by the user (or automatically using
#ifdef DEBUG
logical, parameter :: debug = .true.
#else
logical, parameter :: debug = .false.
#endif
Either way, the compiler knows at compile time what the value is and can optimize out all these blocks if no needed.
Lastly, I would suggest to remove any existing cires_ugwp files (CCPP entry points and dependencies) that are no longer needed.
| tau_kx(:) = tau_kx(:)/tau_norm | ||
|
|
||
| if (kdt == 1) then | ||
| 771 format( 'vay-oro19 ', 3(2x,F8.3)) |
There was a problem hiding this comment.
We would like to reduce the amount of clutter written to stdout, but for the time being and while this is under development, it is ok for me.
| ! arad => con_rerth | ||
| implicit none | ||
|
|
||
| real, parameter :: grav =9.81, cpd = 1004. |
There was a problem hiding this comment.
Constants should come from the host model through the argument list, if possible. Most if not all of these are defined in fv3atm already.
There was a problem hiding this comment.
Leaving as-is for now, but am eliminating "use" statements which call for the physical constants.
There was a problem hiding this comment.
IMO, it is dangerous for every scheme to be defining their own physical constants. They should be defined in one place and used across physics. In the CCPP, sending constants through the argument list enables this -- each host model that uses the CCPP defines physical constants in their own way (which is why you can't rely on "use physcons") and as long as the host provides metadata for the constants so that the CCPP knows what to pass in, it all works as expected with common values amongst physics. In practice, for FV3, the constants may be defined in physcons.F90, but they have metadata and this metadata is made known to the CCPP framework at build time.
There was a problem hiding this comment.
I am still seeing places in the code that do import constants from module ugwp_common_v1? Did I misread your reply?
There was a problem hiding this comment.
I see what you mean, but for the subroutines that are actually called in the ugwp_v1 version, the physical constants that are used are passed in through the argument lists. I would like to leave the other subroutines as-is, as I believe a lot of them will be removed after cleaning up (with Valery next month).
There was a problem hiding this comment.
Thanks for the explanation. As long as you remember all these details ... (I won't, that's for sure).
|
Hi Dom,
I’m in the process of passing the physical constants through the chain of subroutine calls, but I was curious if instead of doing this, would it be okay to use a “use” statement, such as: “use physcons, only: con_grav, con_omega”, instead of using “ugwp_common_v1”.
— Mike
… On Oct 15, 2020, at 12:29 PM, DomHeinzeller ***@***.***> wrote:
@DomHeinzeller requested changes on this pull request.
This is a beautiful, clean implementation of the combined gravity wave drag schemes. I was very picky along the way to request passing constants used deep inside the physics from the host model, and to reduce clutter to stdout/stderr if possible. Plus a few other, small changes.
Here is my suggestion to address the constants and stdout/stderr concerns:
Constants
It seems everything draws constants from module ugwp_common_v1. If we could find a way to set those constants as part of/first task of the init call, then we can leave all code unchanged and still get the host model constants.
This will most likely change the answer, so we should make this change in a controlled way: first run the tests to verify that all existing regression tests pass when using internal constants. Then swap to use host model constants (maybe even using #ifdef statements in ugwp_common_v1 and make sure by manual inspection that the results are still acceptable.
stdout/stderr
There is a large amount of output written to stdout and stderr in the current code. This may be usedul for development, testing and debugging. I would wrap all but the absolutely necessary print statements in an if (debug) block and then have a single debug variable, maybe in ugwp_common_v1, that can be set to .true. by the user (or automatically using
#ifdef DEBUG
logical, parameter :: debug = .true.
#else
logical, parameter :: debug = .false.
#endif
Either way, the compiler knows at compile time what the value is and can optimize out all these blocks if no needed.
Lastly, I would suggest to remove any existing cires_ugwp files (CCPP entry points and dependencies) that are no longer needed.
In physics/cires_orowam2017.F90 <#55 (comment)>:
> @@ -0,0 +1,347 @@
+module cires_orowam2017
+
+
+contains
+
+
+ subroutine oro_wam_2017(im, levs,npt,ipt, kref,kdt,me,master, &
+ & dtp,dxres, taub, u1, v1, t1, xn, yn, bn2, rho, prsi, prsL, &
+ & del, sigma, hprime, gamma, theta, &
+ & sinlat, xlatd, taup, taud, pkdis)
+!
+ USE MACHINE , ONLY : kind_phys
+ use ugwp_common_v1 , only : grav, omega2
These constants should be provided by the host model and passed in via the argument list, if possible.
In physics/cires_orowam2017.F90 <#55 (comment)>:
> + nzi = levs+1
+
+ do iw = 1, nw
+! !kxw = 0.25/(dxres)*iw
+ kxw = kxmin+(iw-1)*dkx
+ akx(iw) = kxw
+ akx2(iw) = kxw*kxw
+ aspkx(iw) = kxw ** (kx_slope)
+ tau_kx(iw) = aspkx(iw)*dkx
+ enddo
+
+ tau_norm = sum(tau_kx)
+ tau_kx(:) = tau_kx(:)/tau_norm
+
+ if (kdt == 1) then
+771 format( 'vay-oro19 ', 3(2x,F8.3))
We would like to reduce the amount of clutter written to stdout, but for the time being and while this is under development, it is ok for me.
In physics/cires_orowam2017.F90 <#55 (comment)>:
> +! if (taud(i,k) .gt. 0)taud(i,k)=taud(i,k)*.01
+! if (abs(taud(i,k)).ge.axmax)taud(i,k)=sign(taud(i,k),axmax)
+ enddo
+ endif ! taub > 0
+ enddo ! oro-points (i, j, ipt)
+!23456
+ end subroutine oro_wam_2017
+!-------------------------------------------------------------
+!
+! define mean flow and dissipation for OGW-kx spectrum
+!
+!-------------------------------------------------------------
+ subroutine oro_meanflow(nz, nzi, u1, v1, t1, pint, pmid, &
+ & delp, rho, bn2, uzi, rhoi, ktur, kalp, dzi, xn, yn)
+
+ use ugwp_common_v1 , only : grav, rgrav, rdi, velmin, dw2min
Constants should come from the host model through the argument list, if possible.
In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
> +! init gw-background dissipation
+!==============================
+!
+! Part-0 specifications of common constants, limiters and "criiical" values
+!
+!
+
+ module ugwp_common_v1
+!
+! use machine, only : kind_phys
+! use physcons, only : pi => con_pi, grav => con_g, rd => con_rd, &
+! rv => con_rv, cpd => con_cp, fv => con_fvirt,&
+! arad => con_rerth
+ implicit none
+
+ real, parameter :: grav =9.81, cpd = 1004.
Constants should come from the host model through the argument list, if possible. Most if not all of these are defined in fv3atm already.
In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
> + real, parameter :: omega2 = 2.*omega1, omega3 = 3.*omega1
+ real, parameter :: hpscale= 7000., rhp=1./hpscale, rhp2=.5*rhp, rh4 = 0.25*rhp
+ real, parameter :: mkzmin = pi2/80.0e3, mkz2min = mkzmin*mkzmin
+ real, parameter :: mkzmax = pi2/500., mkz2max = mkzmax*mkzmax
+ real, parameter :: cdmin = 2.e-2/mkzmax
+ end module ugwp_common_v1
+!
+!
+!===================================================
+!
+!Part-1 init => wave dissipation + RFriction
+!
+!===================================================
+ subroutine init_global_gwdis_v1(levs, zkm, pmb, kvg, ktg, krad, kion, pa_rf, tau_rf, me, master)
+
+ use ugwp_common_v1, only : pih
Ditto
In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
> + if (pmb(k) .le. pa_alp) then
+ rf_fv3=rtau_fv3*sin(pih_dlog*log(pa_alp/pmb(k)))**2
+ krad(k) = krad(k) + rf_fv3
+ kion(k) = kion(k) + rf_fv3
+
+ endif
+
+! write(6,132) zkm(k), kvg(k), kvg(k)*(6.28/5000.)**2, kion(k)
+ enddo
+
+ k= levs+1
+ kion(k) = kion(k-1)
+ krad(k) = krad(k-1)
+ kvg(k) = kvg(k-1)
+ ktg(k) = ktg(k-1)
+ if (me == master) then
This is probably ok, only from master and only at init time.
In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
> + integer :: nazoro ! number of azimuths
+ integer :: nstoro ! flag for stochastic launch above SG-peak
+
+ integer, parameter :: mdir = 8
+ real, parameter :: fdir=.5*mdir/pi
+
+ integer nwdir(mdir)
+ data nwdir/6,7,5,8,2,3,1,4/
+ save nwdir
+
+ real, parameter :: odmin = 0.1, odmax = 10.0
+!------------------------------------------------------------------------------
+! small-scale orography parameters for TOFD of Beljaars et al., 2004, QJRMS
+!------------------------------------------------------------------------------
+
+ integer, parameter :: n_tofd = 2 ! depth of SSO for TOFD compared with Zpbl
Should some of these be added to the host model?
In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
> +
+ real, parameter :: cmin = 5 !2.5
+ real, parameter :: cmax = 95. !82.5
+ real, parameter :: cmid = 22.5
+ real, parameter :: cwid = cmid
+ real, parameter :: bns = 2.e-2, bns2 = bns*bns, bns4=bns2*bns2
+ real, parameter :: mstar = 6.28e-3/2. ! 2km
+ real :: dc
+
+ real, allocatable :: ch_conv(:), spf_conv(:)
+ real, allocatable :: xaz_conv(:), yaz_conv(:)
+ contains
+!
+ subroutine init_conv_gws(nwaves, nazdir, nstoch, effac, &
+ lonr, kxw, cgwf)
+ use ugwp_common_v1, only : pi2, arad
Ditto
In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
> +!
+ nwav = nwaves
+ nazd = nazdir
+ nst = nstoch
+ gw_eff = effac
+ endif
+
+
+ v_kxw = pi2/lhmet ; v_kxw2 = v_kxw*v_kxw
+ rv_kxw = 1./v_kxw
+
+ allocate ( zci(nwav), zci4(nwav), zci3(nwav),zci2(nwav), zdci(nwav) )
+ allocate ( zcosang(nazd), zsinang(nazd) )
+ allocate (lzmet(nwav), czmet(nwav), mkzmet(nwav), dczmet(nwav), dmkz(nwav) )
+
+ if (me == master) then
Again, ok for now since only at init time and from master
In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
> +!
+ dlzmet = (lzmax-lzmin)/ real(nwav-1)
+ do inc=1, nwav
+ lzmet(inc) = lzmin + (inc-1)*dlzmet
+ mkzmet(inc) = pi2/lzmet(inc)
+ zci(inc) =lzmet(inc)/(pi2/bnfix)
+ zci4(inc) = (zms*zci(inc))**4
+ zci2(inc) = (zms*zci(inc))**2
+ zci3(inc) = (zms*zci(inc))**3
+
+ enddo
+
+ zdx = (zci(nwav)-zci(1))/ real(nwav-1)
+
+
+ if (me == master) then
Same here, reduce in the future?
In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
> +! character(len=255) :: file_limb_tab='ugwp_limb_tau.nc'
+
+! integer, parameter :: ny_tab=73, nt_tab=14
+! real, parameter :: rdy_tab = 1./2.5, rdd_tab = 1./30.
+! real :: days_tab(nt_tab), lat_tab(ny_tab)
+! real :: abmf_tab(ny_tab,nt_tab)
+
+ integer :: ugwp_azdir
+ integer :: ugwp_stoch
+
+ integer :: ugwp_src
+ integer :: ugwp_nws
+ real :: ugwp_effac
+
+!
+ data knob_ugwp_source / 1,0, 1, 0 / ! oro-conv-fjet-okw-taub_lat: 1-active 0-off
Can you use modern Fortran and combine those lines with lines 43-47?
In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
> + real, intent (in) :: cdmvgwd(2), cgwf(2) ! "scaling" controls for "old" GFS-GW dims(2) !!!
+ real, intent (in) :: pa_rf_in, tau_rf_in
+
+ character(len=64), intent (in) :: fn_nml2
+ character(len=64), parameter :: fn_nml='input.nml'
+
+! character, intent (in) :: input_nml_file
+! integer, parameter :: logunit = 6
+ integer :: ios
+ logical :: exists
+ real :: dxsg
+
+ integer :: ncid, iernc, vid, dimid, status
+ integer :: k
+ integer :: ddd_ugwp, curday_ugwp
+ real :: avqbo(6)
Possible to combine lines 218 and 219?
In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
> + write (6, *) 'separate ugwp :: namelist file: ', trim (fn_nml), ' does not exist'
+ else
+ open (unit = nlunit, file = trim(fn_nml), action = 'read', status = 'old', iostat = ios)
+ endif
+ rewind (nlunit)
+ read (nlunit, nml = cires_ugwp_nml)
+ close (nlunit)
+!
+ strsolver= knob_ugwp_orosolv
+ pa_rf = pa_rf_in
+ tau_rf = tau_rf_in
+
+ curday_ugwp = jdat_gfs(1)*10000 + jdat_gfs(2)*100 +jdat_gfs(3)
+ call calendar_ugwp(jdat_gfs(1), jdat_gfs(2), jdat_gfs(3), ddd_ugwp)
+
+! write version number and namelist to log file
Quite a lot of output from this routine. Can we reduce it after the initial development and testing phase?
In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
> + knob_ugwp_stoch(2), knob_ugwp_effac(2), lonr, kxw, cgwf )
+ if (me == master) &
+ print *, ' init_convective GWs cgwf', knob_ugwp_wvspec(2), knob_ugwp_azdir(2)
+
+ endif
+
+ ENDIF !IF (do_physb_gwsrcs)
+!
+!
+! Tabulated sources
+!
+! goto 121
+
+ iernc=NF90_OPEN(trim(ugwp_taufile), nf90_nowrite, ncid)
+
+ if(iernc.ne.0) then
Do not stop the model. Instead, pass the CCPP errmsg and errflg down to this routine, assign a meaningful error message to errmsg, set errflg=1 and return. In the calling functions, make sure that an errflg/=1 cascades up immediately so that CCPP can tell the host model to print the error message in its way and stop the execution.
In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
> + iernc=nf90_inq_varid( ncid, 'DAYS', vid )
+ iernc= nf90_get_var( ncid, vid, days_limb)
+ iernc=nf90_inq_varid( ncid, 'LATS', vid )
+ iernc= nf90_get_var( ncid, vid, ugwp_taulat)
+ iernc=nf90_inq_varid( ncid, 'ABSMF', vid )
+ iernc= nf90_get_var( ncid, vid, tau_limb)
+
+ iernc=nf90_close(ncid)
+
+ endif
+!
+ iernc=NF90_OPEN(trim(ugwp_qbofile), nf90_nowrite, ncid)
+
+ if(iernc.ne.0) then
+ write(6,*)
+ write(6,*) ' cannot open qbofile data-file', trim(ugwp_qbofile)
see above, do not stop the model
In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
> +
+
+!=============================================
+
+
+ subroutine cires_ugwp_advance
+!-----------------------------------------------------------------------
+!
+! options for the day-to-day variable sources/spectra + diagnostics
+! for stochastic "triggers"
+! diagnose GW-source functions * FGF + OKWP + SGO/CONV from IAU-fields
+! or use for stochastic GWP-sources "memory"
+!-----------------------------------------------------------------------
+ implicit none
+!
+! update sources
Not doing a lot, this routine ;-)
In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
> +!
+ deallocate( kvg, ktg )
+ deallocate( krad, kion )
+ deallocate( zkm, pmb )
+ deallocate( rfdis, rfdist)
+ deallocate(ugwp_taulat, ugwp_qbolat)
+ deallocate(tau_limb, uzmf_merra)
+ deallocate(days_limb, days_merra, pmb127)
+
+ end subroutine cires_ugwp_finalize
+
+!
+!
+!
+!
+ subroutine calendar_ugwp(yr, mm, dd, ddd_ugwp)
The Julian day is available from the host model, no need to calculate again. From GFS_typdefs.meta
[julian]
standard_name = julian_day
long_name = julian day
units = days
dimensions = ()
type = real
kind = kind_phys
Just need to make sure when the call to calendar_ugwp occurs in your current code. In the init phase? If so, we may have to add the julian day calculation to the init phase of GFS_phys_time_vary_init subroutine.
In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
> + endif
+ w1_j1tau(j) = 1.0 - w2_j2tau(j)
+
+ enddo
+ widqbo2 =1./widqbo/widqbo
+ do j=1,npts
+ dexp_latqbo(j) =0.
+ xabs =abs(dlat(j))
+ if (xabs .le. latqbo) then
+ dexp_latqbo(j) = exp(-xabs*xabs*widqbo2)
+ if (xabs .le. 4.0 ) dexp_latqbo(j) =1.
+! print *, ' indx_ugwp dexp=', dexp_latqbo(j), nint(dlat(j))
+ endif
+ enddo
+
+ if (me == master ) then
Is this called every time step? Or is this a debugging routine that is not active by default?
In physics/cires_ugwp_ngw_utils.F90 <#55 (comment)>:
> + it1 = max(it2-1,1)
+ if (it2 > ntau_d2t ) then
+ print *, ' it1, it2, ntau_d2t ', it1, it2, ntau_d2t
+ stop
+ endif
+ w2 = (float(ddd)-days_limb(it1))/(days_limb(it2)-days_limb(it1))
+ w1 = 1.0-w2
+ do i=1, im
+ j1 = j1_tau(i)
+ j2 = j2_tau(i)
+ tx1 = tau_limb(j1, it1)*ddy_j1tau(i)+tau_limb(j2, it1)*ddy_j2tau(i)
+ tx2 = tau_limb(j1, it2)*ddy_j1tau(i)+tau_limb(j2, it2)*ddy_j2tau(i)
+ tau_sat(i) = tx1*w1 + w2*tx2
+ enddo
+
+ if (me == master ) then
More output that is not needed?
In physics/cires_ugwp_orolm97_v1.F90 <#55 (comment)>:
> + oc, oa4, clx4, theta, sigmad, gammad, elvmaxd, sgh30, &
+ dusfc, dvsfc, xlatd, sinlat, coslat, sparea, &
+ cdmbgwd, me, master, rdxzb, &
+ zmtb, zogw, tau_mtb, tau_ogw, tau_tofd, &
+ dudt_mtb, dudt_ogw, dudt_tms)
+!----------------------------------------
+! ugwp_v1: gwdps_oro_v1 following recent updates of Lott & Miller 1997
+! eventually will be replaced with more "advanced"LLWB
+! and multi-wave solver that produce competitive FV3GFS-skills
+!
+! computation of kref for ogw + coorde diagnostics
+! all constants/parameters inside cires_ugwp_initialize.f90
+!----------------------------------------
+
+ use machine , only : kind_phys
+ use ugwp_common_v1, only : rgrav, grav, cpd, rd, rv, rcpd, rcpd2, &
Constants should come from the host model through the argument list, if possible.
In physics/cires_ugwp_solv2_v1_mod.F90 <#55 (comment)>:
> +!
+!--------------------------------------------------------------------------------
+! nov 2015 alternative gw-solver for nggps-wam
+! nov 2017 nh/rotational gw-modes for nh-fv3gfs
+! oct 2019 adding empirical satellite-based
+! source function and *F90 CIRES-style of the code
+! --------------------------------------------------------------------------------
+!
+
+ use machine, only : kind_phys
+
+ use cires_ugwp_module_v1,only : krad, kvg, kion, ktg
+
+ use cires_ugwp_module_v1,only : knob_ugwp_doheat, knob_ugwp_dokdis, idebug_gwrms
+
+ use ugwp_common_v1 , only : rgrav, grav, cpd, rd, rv, rcpdl, grav2cpd, &
Constants should come from the host model through the argument list, if possible.
In physics/cires_ugwp_triggers_v1.F90 <#55 (comment)>:
> @@ -0,0 +1,576 @@
+module cires_ugwp_triggers_v1
+
+
+contains
+
+
+ subroutine ugwp_triggers
+ implicit none
+ write(6,*) ' physics-based triggers for UGWP '
Not sure what this does ...
In physics/cires_ugwp_triggers_v1.F90 <#55 (comment)>:
> @@ -0,0 +1,576 @@
+module cires_ugwp_triggers_v1
+
+
+contains
+
+
+ subroutine ugwp_triggers
+ implicit none
+ write(6,*) ' physics-based triggers for UGWP '
+ end subroutine ugwp_triggers
+!
+ SUBROUTINE subs_diag_geo(nx, ny, lat, lon, rlat, rlon, dy, dx, &
+ cosv, rlatc, brcos, brcos2, dlam1, dlam2, dlat, divJp, divJm)
+ use ugwp_common_v1 , only : deg_to_rad
Constants should come from the host model through the argument list, if possible.
In physics/cires_ugwp_triggers_v1.F90 <#55 (comment)>:
> + use ugwp_common_v1 , only : deg_to_rad
+
+ implicit none
+ integer :: nx, ny
+ real :: lon(nx), lat(ny)
+ real :: rlon(nx), rlat(ny) , cosv(ny), tanlat(ny)
+ real :: rlatc(ny-1), brcos(ny), brcos2(ny)
+ real :: earth_r, ra1, ra2, dx, dy, dlat
+ real :: dlam1(ny), dlam2(ny), divJp(ny), divJm(ny)
+ integer :: j
+!
+! specify common constants and
+! geometric factors to compute deriv-es etc ...
+! coriolis coslat tan etc...
+!
+ earth_r = 6370.e3
radius of the earth should be defined by the host model (any host model). You want to be consistent, i.e. get it from the host model via the argument list.
In physics/cires_ugwp_triggers_v1.F90 <#55 (comment)>:
> + flat_gw = 0.10
+ else if (latdeg > -60. .and. latdeg <= -31.) then
+ flat_gw = 0.50*exp(-((abs(latdeg)-60.)/23.)**2)
+ else if (latdeg < 60. .and. latdeg >= 31.) then
+ flat_gw = 0.50*exp(-((abs(latdeg)-60.)/23.)**2)
+ else if (latdeg <= -60.) then
+ flat_gw = 0.50*exp(-((abs(latdeg)-60.)/70.)**2)
+ else if (latdeg >= 60.) then
+ flat_gw = 0.50*exp(-((abs(latdeg)-60.)/70.)**2)
+ end if
+ tau_gw(i) = tau_amp*flat_gw
+ enddo
+!
+ end subroutine slat_geos5
+ subroutine init_nazdir(naz, xaz, yaz)
+ use ugwp_common_v1 , only : pi2
Constants should come from the host model through the argument list, if possible.
In physics/cires_vert_orodis_v1.F90 <#55 (comment)>:
> +! output : taulin, taufrb, taulee, xn, yn, umag, kxw/kxridge
+! print *, krefj, levs, tauogw, tautot , ' ugwp_taub_oro '
+!
+ end subroutine ugwp_taub_oro
+!
+!--------------------------------------
+!
+! call ugwp_oro_lsatdis( krefj, levs, tauogw(j), tautot(j), tau_src, kxw, &
+! fcor(j), c2f2(j), up, vp, tp, qp, dp, zpm, zpi, pmid1, pint1, &
+! xn, yn, umag, drtau, kdis_oro)
+
+ subroutine ugwp_oro_lsatdis( krefj, levs, tauogw, tautot, tau_src, &
+ kxw, fcor, kxridge, up, vp, tp, qp, dp, zpm, zpi, pmid, pint, &
+ xn, yn, umag, drtau, kdis)
+
+ use ugwp_common_v1, only : bnv2min, grav, pi, pi2, dw2min, velmin, rgrav
Constants should come from the host model through the argument list, if possible.
In physics/cires_vert_orodis_v1.F90 <#55 (comment)>:
> + kds = 0.
+ endif
+ enddo
+
+ do k=krefj+1, kcrit
+ drtau(k) = rgrav*(tauz(k+1)-tauz(k))/dp(k)
+ enddo
+!
+!
+ end subroutine ugwp_oro_lsatdis
+!
+!
+ subroutine ugwp_tofd(im, levs, sigflt, elvmax, zpbl, u, v, zmid, &
+ utofd, vtofd, epstofd, krf_tofd)
+ use machine , only : kind_phys
+ use ugwp_common_v1 , only : rcpd2
Constants should come from the host model through the argument list, if possible.
In physics/cires_vert_orodis_v1.F90 <#55 (comment)>:
> + krf = const_tofd* a12_tofd *sgh2* zmet ** (-1.2) *zexp
+ utofd(i,k) = -krf*u(i,k)
+ vtofd(i,k) = -krf*v(i,k)
+ epstofd(i,k)= rcpd2*krf*ekin ! more accurate heat/mom form using "implicit tend-solver"
+ ! to update momentum and temp-re
+ krf_tofd(i,k) = krf
+ enddo
+ enddo
+!
+ end subroutine ugwp_tofd
+!
+!
+ subroutine ugwp_tofd1d(levs, sigflt, elvmax, zsurf, zpbl, u, v, &
+ zmid, utofd, vtofd, epstofd, krf_tofd)
+ use machine , only : kind_phys
+ use ugwp_common_v1 , only : rcpd2
Constants should come from the host model through the argument list, if possible.
In physics/unified_ugwp_post.F90 <#55 (comment)>:
> @@ -0,0 +1,83 @@
+!> \file unified_ugwp_post.F90
+!! This file contains
+module unified_ugwp_post
+
+contains
+
+!>\defgroup unified_ugwp_post unified_UGWP Scheme Post
+!! @{
+!> \section arg_table_unified_ugwp_post_init Argument Table
+!!
+ subroutine unified_ugwp_post_init ()
+ end subroutine unified_ugwp_post_init
+
***@***.*** The subroutine initializes the unified UGWP
+#if 0
We don't need lines 15 (#if 0) and 19 (#endif) anymore.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#55 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFMODLD3XLJPIKE3CVQGQ4LSK45QNANCNFSM4SPGK67Q>.
|
|
No, constants should come in via the argument list. physcons.F90 sits there for legacy reasons and should be moved over to the host model once all physics cleanly get their constants through the argument list.
… On Oct 19, 2020, at 9:36 PM, mdtoy ***@***.***> wrote:
Hi Dom,
I’m in the process of passing the physical constants through the chain of subroutine calls, but I was curious if instead of doing this, would it be okay to use a “use” statement, such as: “use physcons, only: con_grav, con_omega”, instead of using “ugwp_common_v1”.
— Mike
> On Oct 15, 2020, at 12:29 PM, DomHeinzeller ***@***.***> wrote:
>
>
> @DomHeinzeller requested changes on this pull request.
>
> This is a beautiful, clean implementation of the combined gravity wave drag schemes. I was very picky along the way to request passing constants used deep inside the physics from the host model, and to reduce clutter to stdout/stderr if possible. Plus a few other, small changes.
>
> Here is my suggestion to address the constants and stdout/stderr concerns:
>
> Constants
>
> It seems everything draws constants from module ugwp_common_v1. If we could find a way to set those constants as part of/first task of the init call, then we can leave all code unchanged and still get the host model constants.
>
> This will most likely change the answer, so we should make this change in a controlled way: first run the tests to verify that all existing regression tests pass when using internal constants. Then swap to use host model constants (maybe even using #ifdef statements in ugwp_common_v1 and make sure by manual inspection that the results are still acceptable.
>
> stdout/stderr
>
> There is a large amount of output written to stdout and stderr in the current code. This may be usedul for development, testing and debugging. I would wrap all but the absolutely necessary print statements in an if (debug) block and then have a single debug variable, maybe in ugwp_common_v1, that can be set to .true. by the user (or automatically using
>
> #ifdef DEBUG
> logical, parameter :: debug = .true.
> #else
> logical, parameter :: debug = .false.
> #endif
> Either way, the compiler knows at compile time what the value is and can optimize out all these blocks if no needed.
>
> Lastly, I would suggest to remove any existing cires_ugwp files (CCPP entry points and dependencies) that are no longer needed.
>
> In physics/cires_orowam2017.F90 <#55 (comment)>:
>
> > @@ -0,0 +1,347 @@
> +module cires_orowam2017
> +
> +
> +contains
> +
> +
> + subroutine oro_wam_2017(im, levs,npt,ipt, kref,kdt,me,master, &
> + & dtp,dxres, taub, u1, v1, t1, xn, yn, bn2, rho, prsi, prsL, &
> + & del, sigma, hprime, gamma, theta, &
> + & sinlat, xlatd, taup, taud, pkdis)
> +!
> + USE MACHINE , ONLY : kind_phys
> + use ugwp_common_v1 , only : grav, omega2
> These constants should be provided by the host model and passed in via the argument list, if possible.
>
> In physics/cires_orowam2017.F90 <#55 (comment)>:
>
> > + nzi = levs+1
> +
> + do iw = 1, nw
> +! !kxw = 0.25/(dxres)*iw
> + kxw = kxmin+(iw-1)*dkx
> + akx(iw) = kxw
> + akx2(iw) = kxw*kxw
> + aspkx(iw) = kxw ** (kx_slope)
> + tau_kx(iw) = aspkx(iw)*dkx
> + enddo
> +
> + tau_norm = sum(tau_kx)
> + tau_kx(:) = tau_kx(:)/tau_norm
> +
> + if (kdt == 1) then
> +771 format( 'vay-oro19 ', 3(2x,F8.3))
> We would like to reduce the amount of clutter written to stdout, but for the time being and while this is under development, it is ok for me.
>
> In physics/cires_orowam2017.F90 <#55 (comment)>:
>
> > +! if (taud(i,k) .gt. 0)taud(i,k)=taud(i,k)*.01
> +! if (abs(taud(i,k)).ge.axmax)taud(i,k)=sign(taud(i,k),axmax)
> + enddo
> + endif ! taub > 0
> + enddo ! oro-points (i, j, ipt)
> +!23456
> + end subroutine oro_wam_2017
> +!-------------------------------------------------------------
> +!
> +! define mean flow and dissipation for OGW-kx spectrum
> +!
> +!-------------------------------------------------------------
> + subroutine oro_meanflow(nz, nzi, u1, v1, t1, pint, pmid, &
> + & delp, rho, bn2, uzi, rhoi, ktur, kalp, dzi, xn, yn)
> +
> + use ugwp_common_v1 , only : grav, rgrav, rdi, velmin, dw2min
> Constants should come from the host model through the argument list, if possible.
>
> In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
>
> > +! init gw-background dissipation
> +!==============================
> +!
> +! Part-0 specifications of common constants, limiters and "criiical" values
> +!
> +!
> +
> + module ugwp_common_v1
> +!
> +! use machine, only : kind_phys
> +! use physcons, only : pi => con_pi, grav => con_g, rd => con_rd, &
> +! rv => con_rv, cpd => con_cp, fv => con_fvirt,&
> +! arad => con_rerth
> + implicit none
> +
> + real, parameter :: grav =9.81, cpd = 1004.
> Constants should come from the host model through the argument list, if possible. Most if not all of these are defined in fv3atm already.
>
> In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
>
> > + real, parameter :: omega2 = 2.*omega1, omega3 = 3.*omega1
> + real, parameter :: hpscale= 7000., rhp=1./hpscale, rhp2=.5*rhp, rh4 = 0.25*rhp
> + real, parameter :: mkzmin = pi2/80.0e3, mkz2min = mkzmin*mkzmin
> + real, parameter :: mkzmax = pi2/500., mkz2max = mkzmax*mkzmax
> + real, parameter :: cdmin = 2.e-2/mkzmax
> + end module ugwp_common_v1
> +!
> +!
> +!===================================================
> +!
> +!Part-1 init => wave dissipation + RFriction
> +!
> +!===================================================
> + subroutine init_global_gwdis_v1(levs, zkm, pmb, kvg, ktg, krad, kion, pa_rf, tau_rf, me, master)
> +
> + use ugwp_common_v1, only : pih
> Ditto
>
> In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
>
> > + if (pmb(k) .le. pa_alp) then
> + rf_fv3=rtau_fv3*sin(pih_dlog*log(pa_alp/pmb(k)))**2
> + krad(k) = krad(k) + rf_fv3
> + kion(k) = kion(k) + rf_fv3
> +
> + endif
> +
> +! write(6,132) zkm(k), kvg(k), kvg(k)*(6.28/5000.)**2, kion(k)
> + enddo
> +
> + k= levs+1
> + kion(k) = kion(k-1)
> + krad(k) = krad(k-1)
> + kvg(k) = kvg(k-1)
> + ktg(k) = ktg(k-1)
> + if (me == master) then
> This is probably ok, only from master and only at init time.
>
> In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
>
> > + integer :: nazoro ! number of azimuths
> + integer :: nstoro ! flag for stochastic launch above SG-peak
> +
> + integer, parameter :: mdir = 8
> + real, parameter :: fdir=.5*mdir/pi
> +
> + integer nwdir(mdir)
> + data nwdir/6,7,5,8,2,3,1,4/
> + save nwdir
> +
> + real, parameter :: odmin = 0.1, odmax = 10.0
> +!------------------------------------------------------------------------------
> +! small-scale orography parameters for TOFD of Beljaars et al., 2004, QJRMS
> +!------------------------------------------------------------------------------
> +
> + integer, parameter :: n_tofd = 2 ! depth of SSO for TOFD compared with Zpbl
> Should some of these be added to the host model?
>
> In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
>
> > +
> + real, parameter :: cmin = 5 !2.5
> + real, parameter :: cmax = 95. !82.5
> + real, parameter :: cmid = 22.5
> + real, parameter :: cwid = cmid
> + real, parameter :: bns = 2.e-2, bns2 = bns*bns, bns4=bns2*bns2
> + real, parameter :: mstar = 6.28e-3/2. ! 2km
> + real :: dc
> +
> + real, allocatable :: ch_conv(:), spf_conv(:)
> + real, allocatable :: xaz_conv(:), yaz_conv(:)
> + contains
> +!
> + subroutine init_conv_gws(nwaves, nazdir, nstoch, effac, &
> + lonr, kxw, cgwf)
> + use ugwp_common_v1, only : pi2, arad
> Ditto
>
> In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
>
> > +!
> + nwav = nwaves
> + nazd = nazdir
> + nst = nstoch
> + gw_eff = effac
> + endif
> +
> +
> + v_kxw = pi2/lhmet ; v_kxw2 = v_kxw*v_kxw
> + rv_kxw = 1./v_kxw
> +
> + allocate ( zci(nwav), zci4(nwav), zci3(nwav),zci2(nwav), zdci(nwav) )
> + allocate ( zcosang(nazd), zsinang(nazd) )
> + allocate (lzmet(nwav), czmet(nwav), mkzmet(nwav), dczmet(nwav), dmkz(nwav) )
> +
> + if (me == master) then
> Again, ok for now since only at init time and from master
>
> In physics/cires_ugwp_initialize_v1.F90 <#55 (comment)>:
>
> > +!
> + dlzmet = (lzmax-lzmin)/ real(nwav-1)
> + do inc=1, nwav
> + lzmet(inc) = lzmin + (inc-1)*dlzmet
> + mkzmet(inc) = pi2/lzmet(inc)
> + zci(inc) =lzmet(inc)/(pi2/bnfix)
> + zci4(inc) = (zms*zci(inc))**4
> + zci2(inc) = (zms*zci(inc))**2
> + zci3(inc) = (zms*zci(inc))**3
> +
> + enddo
> +
> + zdx = (zci(nwav)-zci(1))/ real(nwav-1)
> +
> +
> + if (me == master) then
> Same here, reduce in the future?
>
> In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
>
> > +! character(len=255) :: file_limb_tab='ugwp_limb_tau.nc'
> +
> +! integer, parameter :: ny_tab=73, nt_tab=14
> +! real, parameter :: rdy_tab = 1./2.5, rdd_tab = 1./30.
> +! real :: days_tab(nt_tab), lat_tab(ny_tab)
> +! real :: abmf_tab(ny_tab,nt_tab)
> +
> + integer :: ugwp_azdir
> + integer :: ugwp_stoch
> +
> + integer :: ugwp_src
> + integer :: ugwp_nws
> + real :: ugwp_effac
> +
> +!
> + data knob_ugwp_source / 1,0, 1, 0 / ! oro-conv-fjet-okw-taub_lat: 1-active 0-off
> Can you use modern Fortran and combine those lines with lines 43-47?
>
> In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
>
> > + real, intent (in) :: cdmvgwd(2), cgwf(2) ! "scaling" controls for "old" GFS-GW dims(2) !!!
> + real, intent (in) :: pa_rf_in, tau_rf_in
> +
> + character(len=64), intent (in) :: fn_nml2
> + character(len=64), parameter :: fn_nml='input.nml'
> +
> +! character, intent (in) :: input_nml_file
> +! integer, parameter :: logunit = 6
> + integer :: ios
> + logical :: exists
> + real :: dxsg
> +
> + integer :: ncid, iernc, vid, dimid, status
> + integer :: k
> + integer :: ddd_ugwp, curday_ugwp
> + real :: avqbo(6)
> Possible to combine lines 218 and 219?
>
> In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
>
> > + write (6, *) 'separate ugwp :: namelist file: ', trim (fn_nml), ' does not exist'
> + else
> + open (unit = nlunit, file = trim(fn_nml), action = 'read', status = 'old', iostat = ios)
> + endif
> + rewind (nlunit)
> + read (nlunit, nml = cires_ugwp_nml)
> + close (nlunit)
> +!
> + strsolver= knob_ugwp_orosolv
> + pa_rf = pa_rf_in
> + tau_rf = tau_rf_in
> +
> + curday_ugwp = jdat_gfs(1)*10000 + jdat_gfs(2)*100 +jdat_gfs(3)
> + call calendar_ugwp(jdat_gfs(1), jdat_gfs(2), jdat_gfs(3), ddd_ugwp)
> +
> +! write version number and namelist to log file
> Quite a lot of output from this routine. Can we reduce it after the initial development and testing phase?
>
> In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
>
> > + knob_ugwp_stoch(2), knob_ugwp_effac(2), lonr, kxw, cgwf )
> + if (me == master) &
> + print *, ' init_convective GWs cgwf', knob_ugwp_wvspec(2), knob_ugwp_azdir(2)
> +
> + endif
> +
> + ENDIF !IF (do_physb_gwsrcs)
> +!
> +!
> +! Tabulated sources
> +!
> +! goto 121
> +
> + iernc=NF90_OPEN(trim(ugwp_taufile), nf90_nowrite, ncid)
> +
> + if(iernc.ne.0) then
> Do not stop the model. Instead, pass the CCPP errmsg and errflg down to this routine, assign a meaningful error message to errmsg, set errflg=1 and return. In the calling functions, make sure that an errflg/=1 cascades up immediately so that CCPP can tell the host model to print the error message in its way and stop the execution.
>
> In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
>
> > + iernc=nf90_inq_varid( ncid, 'DAYS', vid )
> + iernc= nf90_get_var( ncid, vid, days_limb)
> + iernc=nf90_inq_varid( ncid, 'LATS', vid )
> + iernc= nf90_get_var( ncid, vid, ugwp_taulat)
> + iernc=nf90_inq_varid( ncid, 'ABSMF', vid )
> + iernc= nf90_get_var( ncid, vid, tau_limb)
> +
> + iernc=nf90_close(ncid)
> +
> + endif
> +!
> + iernc=NF90_OPEN(trim(ugwp_qbofile), nf90_nowrite, ncid)
> +
> + if(iernc.ne.0) then
> + write(6,*)
> + write(6,*) ' cannot open qbofile data-file', trim(ugwp_qbofile)
> see above, do not stop the model
>
> In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
>
> > +
> +
> +!=============================================
> +
> +
> + subroutine cires_ugwp_advance
> +!-----------------------------------------------------------------------
> +!
> +! options for the day-to-day variable sources/spectra + diagnostics
> +! for stochastic "triggers"
> +! diagnose GW-source functions * FGF + OKWP + SGO/CONV from IAU-fields
> +! or use for stochastic GWP-sources "memory"
> +!-----------------------------------------------------------------------
> + implicit none
> +!
> +! update sources
> Not doing a lot, this routine ;-)
>
> In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
>
> > +!
> + deallocate( kvg, ktg )
> + deallocate( krad, kion )
> + deallocate( zkm, pmb )
> + deallocate( rfdis, rfdist)
> + deallocate(ugwp_taulat, ugwp_qbolat)
> + deallocate(tau_limb, uzmf_merra)
> + deallocate(days_limb, days_merra, pmb127)
> +
> + end subroutine cires_ugwp_finalize
> +
> +!
> +!
> +!
> +!
> + subroutine calendar_ugwp(yr, mm, dd, ddd_ugwp)
> The Julian day is available from the host model, no need to calculate again. From GFS_typdefs.meta
>
> [julian]
> standard_name = julian_day
> long_name = julian day
> units = days
> dimensions = ()
> type = real
> kind = kind_phys
> Just need to make sure when the call to calendar_ugwp occurs in your current code. In the init phase? If so, we may have to add the julian day calculation to the init phase of GFS_phys_time_vary_init subroutine.
>
> In physics/cires_ugwp_module_v1.F90 <#55 (comment)>:
>
> > + endif
> + w1_j1tau(j) = 1.0 - w2_j2tau(j)
> +
> + enddo
> + widqbo2 =1./widqbo/widqbo
> + do j=1,npts
> + dexp_latqbo(j) =0.
> + xabs =abs(dlat(j))
> + if (xabs .le. latqbo) then
> + dexp_latqbo(j) = exp(-xabs*xabs*widqbo2)
> + if (xabs .le. 4.0 ) dexp_latqbo(j) =1.
> +! print *, ' indx_ugwp dexp=', dexp_latqbo(j), nint(dlat(j))
> + endif
> + enddo
> +
> + if (me == master ) then
> Is this called every time step? Or is this a debugging routine that is not active by default?
>
> In physics/cires_ugwp_ngw_utils.F90 <#55 (comment)>:
>
> > + it1 = max(it2-1,1)
> + if (it2 > ntau_d2t ) then
> + print *, ' it1, it2, ntau_d2t ', it1, it2, ntau_d2t
> + stop
> + endif
> + w2 = (float(ddd)-days_limb(it1))/(days_limb(it2)-days_limb(it1))
> + w1 = 1.0-w2
> + do i=1, im
> + j1 = j1_tau(i)
> + j2 = j2_tau(i)
> + tx1 = tau_limb(j1, it1)*ddy_j1tau(i)+tau_limb(j2, it1)*ddy_j2tau(i)
> + tx2 = tau_limb(j1, it2)*ddy_j1tau(i)+tau_limb(j2, it2)*ddy_j2tau(i)
> + tau_sat(i) = tx1*w1 + w2*tx2
> + enddo
> +
> + if (me == master ) then
> More output that is not needed?
>
> In physics/cires_ugwp_orolm97_v1.F90 <#55 (comment)>:
>
> > + oc, oa4, clx4, theta, sigmad, gammad, elvmaxd, sgh30, &
> + dusfc, dvsfc, xlatd, sinlat, coslat, sparea, &
> + cdmbgwd, me, master, rdxzb, &
> + zmtb, zogw, tau_mtb, tau_ogw, tau_tofd, &
> + dudt_mtb, dudt_ogw, dudt_tms)
> +!----------------------------------------
> +! ugwp_v1: gwdps_oro_v1 following recent updates of Lott & Miller 1997
> +! eventually will be replaced with more "advanced"LLWB
> +! and multi-wave solver that produce competitive FV3GFS-skills
> +!
> +! computation of kref for ogw + coorde diagnostics
> +! all constants/parameters inside cires_ugwp_initialize.f90
> +!----------------------------------------
> +
> + use machine , only : kind_phys
> + use ugwp_common_v1, only : rgrav, grav, cpd, rd, rv, rcpd, rcpd2, &
> Constants should come from the host model through the argument list, if possible.
>
> In physics/cires_ugwp_solv2_v1_mod.F90 <#55 (comment)>:
>
> > +!
> +!--------------------------------------------------------------------------------
> +! nov 2015 alternative gw-solver for nggps-wam
> +! nov 2017 nh/rotational gw-modes for nh-fv3gfs
> +! oct 2019 adding empirical satellite-based
> +! source function and *F90 CIRES-style of the code
> +! --------------------------------------------------------------------------------
> +!
> +
> + use machine, only : kind_phys
> +
> + use cires_ugwp_module_v1,only : krad, kvg, kion, ktg
> +
> + use cires_ugwp_module_v1,only : knob_ugwp_doheat, knob_ugwp_dokdis, idebug_gwrms
> +
> + use ugwp_common_v1 , only : rgrav, grav, cpd, rd, rv, rcpdl, grav2cpd, &
> Constants should come from the host model through the argument list, if possible.
>
> In physics/cires_ugwp_triggers_v1.F90 <#55 (comment)>:
>
> > @@ -0,0 +1,576 @@
> +module cires_ugwp_triggers_v1
> +
> +
> +contains
> +
> +
> + subroutine ugwp_triggers
> + implicit none
> + write(6,*) ' physics-based triggers for UGWP '
> Not sure what this does ...
>
> In physics/cires_ugwp_triggers_v1.F90 <#55 (comment)>:
>
> > @@ -0,0 +1,576 @@
> +module cires_ugwp_triggers_v1
> +
> +
> +contains
> +
> +
> + subroutine ugwp_triggers
> + implicit none
> + write(6,*) ' physics-based triggers for UGWP '
> + end subroutine ugwp_triggers
> +!
> + SUBROUTINE subs_diag_geo(nx, ny, lat, lon, rlat, rlon, dy, dx, &
> + cosv, rlatc, brcos, brcos2, dlam1, dlam2, dlat, divJp, divJm)
> + use ugwp_common_v1 , only : deg_to_rad
> Constants should come from the host model through the argument list, if possible.
>
> In physics/cires_ugwp_triggers_v1.F90 <#55 (comment)>:
>
> > + use ugwp_common_v1 , only : deg_to_rad
> +
> + implicit none
> + integer :: nx, ny
> + real :: lon(nx), lat(ny)
> + real :: rlon(nx), rlat(ny) , cosv(ny), tanlat(ny)
> + real :: rlatc(ny-1), brcos(ny), brcos2(ny)
> + real :: earth_r, ra1, ra2, dx, dy, dlat
> + real :: dlam1(ny), dlam2(ny), divJp(ny), divJm(ny)
> + integer :: j
> +!
> +! specify common constants and
> +! geometric factors to compute deriv-es etc ...
> +! coriolis coslat tan etc...
> +!
> + earth_r = 6370.e3
> radius of the earth should be defined by the host model (any host model). You want to be consistent, i.e. get it from the host model via the argument list.
>
> In physics/cires_ugwp_triggers_v1.F90 <#55 (comment)>:
>
> > + flat_gw = 0.10
> + else if (latdeg > -60. .and. latdeg <= -31.) then
> + flat_gw = 0.50*exp(-((abs(latdeg)-60.)/23.)**2)
> + else if (latdeg < 60. .and. latdeg >= 31.) then
> + flat_gw = 0.50*exp(-((abs(latdeg)-60.)/23.)**2)
> + else if (latdeg <= -60.) then
> + flat_gw = 0.50*exp(-((abs(latdeg)-60.)/70.)**2)
> + else if (latdeg >= 60.) then
> + flat_gw = 0.50*exp(-((abs(latdeg)-60.)/70.)**2)
> + end if
> + tau_gw(i) = tau_amp*flat_gw
> + enddo
> +!
> + end subroutine slat_geos5
> + subroutine init_nazdir(naz, xaz, yaz)
> + use ugwp_common_v1 , only : pi2
> Constants should come from the host model through the argument list, if possible.
>
> In physics/cires_vert_orodis_v1.F90 <#55 (comment)>:
>
> > +! output : taulin, taufrb, taulee, xn, yn, umag, kxw/kxridge
> +! print *, krefj, levs, tauogw, tautot , ' ugwp_taub_oro '
> +!
> + end subroutine ugwp_taub_oro
> +!
> +!--------------------------------------
> +!
> +! call ugwp_oro_lsatdis( krefj, levs, tauogw(j), tautot(j), tau_src, kxw, &
> +! fcor(j), c2f2(j), up, vp, tp, qp, dp, zpm, zpi, pmid1, pint1, &
> +! xn, yn, umag, drtau, kdis_oro)
> +
> + subroutine ugwp_oro_lsatdis( krefj, levs, tauogw, tautot, tau_src, &
> + kxw, fcor, kxridge, up, vp, tp, qp, dp, zpm, zpi, pmid, pint, &
> + xn, yn, umag, drtau, kdis)
> +
> + use ugwp_common_v1, only : bnv2min, grav, pi, pi2, dw2min, velmin, rgrav
> Constants should come from the host model through the argument list, if possible.
>
> In physics/cires_vert_orodis_v1.F90 <#55 (comment)>:
>
> > + kds = 0.
> + endif
> + enddo
> +
> + do k=krefj+1, kcrit
> + drtau(k) = rgrav*(tauz(k+1)-tauz(k))/dp(k)
> + enddo
> +!
> +!
> + end subroutine ugwp_oro_lsatdis
> +!
> +!
> + subroutine ugwp_tofd(im, levs, sigflt, elvmax, zpbl, u, v, zmid, &
> + utofd, vtofd, epstofd, krf_tofd)
> + use machine , only : kind_phys
> + use ugwp_common_v1 , only : rcpd2
> Constants should come from the host model through the argument list, if possible.
>
> In physics/cires_vert_orodis_v1.F90 <#55 (comment)>:
>
> > + krf = const_tofd* a12_tofd *sgh2* zmet ** (-1.2) *zexp
> + utofd(i,k) = -krf*u(i,k)
> + vtofd(i,k) = -krf*v(i,k)
> + epstofd(i,k)= rcpd2*krf*ekin ! more accurate heat/mom form using "implicit tend-solver"
> + ! to update momentum and temp-re
> + krf_tofd(i,k) = krf
> + enddo
> + enddo
> +!
> + end subroutine ugwp_tofd
> +!
> +!
> + subroutine ugwp_tofd1d(levs, sigflt, elvmax, zsurf, zpbl, u, v, &
> + zmid, utofd, vtofd, epstofd, krf_tofd)
> + use machine , only : kind_phys
> + use ugwp_common_v1 , only : rcpd2
> Constants should come from the host model through the argument list, if possible.
>
> In physics/unified_ugwp_post.F90 <#55 (comment)>:
>
> > @@ -0,0 +1,83 @@
> +!> \file unified_ugwp_post.F90
> +!! This file contains
> +module unified_ugwp_post
> +
> +contains
> +
> +!>\defgroup unified_ugwp_post unified_UGWP Scheme Post
> +!! @{
> +!> \section arg_table_unified_ugwp_post_init Argument Table
> +!!
> + subroutine unified_ugwp_post_init ()
> + end subroutine unified_ugwp_post_init
> +
> ***@***.*** The subroutine initializes the unified UGWP
> +#if 0
> We don't need lines 15 (#if 0) and 19 (#endif) anymore.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub <#55 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFMODLD3XLJPIKE3CVQGQ4LSK45QNANCNFSM4SPGK67Q>.
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#55 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AN7FF5BCLXNJG7YR5DIOP6DSLUAVNANCNFSM4SPGK67Q>.
|
|
@mdtoy I can't follow when you think the PR is ready again. Can you please let me know? If you could resolve the comments that you addressed, leave the others open and create an issue in this repository to remind ourselves about them, that would be great. |
| real, parameter :: gr2 = grav*gor | ||
| real, parameter :: grcp = grav*rcpd, gocp = grcp | ||
| real, parameter :: rcpdl = cpd*rgrav ! 1/[g/cp] == cp/g | ||
| real, parameter :: grav2cpd = grav*grcp ! g*(g/cp)= g^2/cp |
There was a problem hiding this comment.
Even some of these derived constants have CCPP standard names, BTW. Where they don't, and they must be calculated locally, they will of course need to no longer be defined as parameters.
There was a problem hiding this comment.
I had found that the derived ratio "fv" had the CCPP standard name "con_fvirt", and I used it in my previous revision from last week, where I did recalculate them locally (not as parameters). For now I would like to leave ugwp_common_v1 as-is even though I'm not using any physical constants from it. We can get rid of unused constants for our final ugwp_v1 version. I hope that's okay.
I just found some more ratios in ccpp/physics/physics/physcons.F90 and I could pass them through GFS_typedefs.meta later if necessary. Is physcons.F90 correct source? Also, are we free to add our own ratios to physcons.F90? (BTW, why aren't more of the ratios already in GFS_typedefs.meta?)
| !----------------------------------------------------------- | ||
| use machine, only : kind_phys | ||
| use physcons, only : con_cp, con_g, con_rd, con_rv | ||
| use physcons, only : con_cp, con_g, con_rd, con_rv, & |
There was a problem hiding this comment.
Same comment about physical constants.
There was a problem hiding this comment.
This section of code is carried over from cires_ugwp_v0, already in the gsd/develop branch. The unified_ugwp (v1) code doesn't call this, so I'm wondering if we can keep it as-is.
grantfirl
left a comment
grantfirl
left a comment
There was a problem hiding this comment.
As someone outside of GSD, I should have zero influence on what goes on in this fork, but I was asked to review. I'm honestly very confused about the gravity wave drag code unification strategy. What purpose does it serve to push all of these schemes into one CCPP scheme? From a CCPP point-of-view, and a GWD layman, if there are differing algorithms for solving the same parameterization problem, why should they not be distinct CCPP schemes? If the goal is to coalesce on the best algorithm, why are multiple versions of an algorithm being kept around under the veil of one "unified" one? It could certainly be that I'm just not understanding the code, but if there are going to be a bunch of namelist options to use one of Valery's versions or the GSD drag suite or the older GFS scheme, I think that there needs to be REALLY good documentation (with flow diagrams) that help a user understand what is actually being executed with the various namelist options. I'm not seeing that right now. Also, does anyone else find it ironic that the word "unified" appears twice in the name "unified UGWP"?
Thanks, @grantfirl for taking the time to review the code and flagging the constants/stop issues that I missed in my first round of reviews. Regarding your general concern. My understanding was, but @mdtoy please correct me if I am wrong, that the goal of putting those various versions under one hood is to be able to experiment with them and agree on what is the best solution for implementation in GFSv17 (?). Once this has been decided, the code would be cleaned up and only what is needed will be left in the final, unified version (maybe with a triple "unified" in the name then???). |
|
That's correct Dom, this is a transitional scheme for testing the various NOAA drag schemes to select and tune the best combination. For the final scheme, we'll find a better name than unified^nth power. |
|
To follow up on my previous comment, the unified_ugwp scheme doesn't just put three separate schemes into one for calling each one individually (which is an option, of course), but instead different pieces of the schemes can be combined with the others to come up with new possibilities (e.g., ugwp_v0 GWD+blocking combined with GSL small-scaleGWD + turbulent orographic form drag). This is the testing framework which will help with deciding on the final combination. |
|
I pushed my changes for your review. |
|
Before notifying that my PR changes were ready for review, I had forgotten to test if the new code would compile. There was a slight bug, which was fixed and pushed just now. The bug was in cires_vert_orodis_v1.F90. |
DomHeinzeller
left a comment
DomHeinzeller
left a comment
There was a problem hiding this comment.
What kind of testing have you done with this code after addressing the reviewer comments? Did you run (some of) the rt.sh regression tests? Trying to figure out what tests I have to run. Thanks ...
|
I did an RT after your suggested changes last week, but I haven't done another since yesterday's changes suggested by Grant. If you like, I can perform them this evening. The fv3_ccpp_gsd_drag_suite fails because I haven't put in the required input.nml changes automatically. I redo it manually and it checks out. Let me know if you want me to create scripts that automate the tests, including three more tests that I do with the unified_ugwp scheme to test for reproducibility of the three schemes. |
That would be great, thank you! |
|
I performed regression tests of the unified_ugwp scheme, which consisted of the following 6 tests using the test script tests/rt_ccpp_gsd_unified_ugwp.conf (they all passed):
The baselines were created with the test script: Feel free to modify/delete my test scripts as needed. Thanks. |
|
@mdtoy, can you please make sure that your ccpp-physics PR is up to date, i.e. includes everything from NOAA-GSD gsd/develop ? I don't know if it is, just checking. In ccpp/physics, check that one of your remotes points to Then go back to FV3 and see if |
|
I just merged latest NOAA-GSD gsd/develop and pushed to my fork (PR). |
|
@mdtoy I am getting one unexpected regression test failure when running the full regression tests ( I will need to run this test in DEBUG mode to see where it crashes. So far, the other tests are looking good, but not all votes have been counted yet ;-) |
|
Thanks for the update. Let me know what the results of the debug-mode test are. |
|
This PR has been pulled into #61 and will be merged automatically. Do not merge manually. |
Wrapper for PR #55 (Unified ugwp)
5 participants
Hello,
This PR introduces the "unified_ugwp" gravity wave drag scheme, which combines three drag schemes: 1) the "cires_ugwp" scheme, 2) the GSL drag suite "drag_suite", and 3) the new "ugwp_v1" scheme developed by Valery Yudin of CIRES. The first two schemes can be regression-tested against their original respective schemes, while the "ugwp_v1" is still under development. To activate the various options, new namelist options are required, which are available from the developers.
Regards,
Mike Toy