2020.02 candidate into NOAA-GFDL/dev/gfdl

[bensonr]

This merge request is to push the updated nesting infrastructure into the dev/gfdl branch tracking FV3 for GFDL model components. It has been checked against GFDL models and does not change answers.

Almost every file has been touched as this update removes whitespace, tab characters, etc.

[underwoo]

@bensonr I managed to get through about 1/2 of the files for review. I'll still work through the others, but I wanted to give you a chance to comment on what I found so far. I will finish my review tomorrow.

As this has a lot of changes, I am unable to look real close at the code changes. I don't think I will mark this a "request changes", but if the items will be addressed, I think it would make the code easier to maintain.

[underwoo]

Just want to verify that this change from 0.2 to 0.1 is desired. I also hope the change, or some explanation is described in a commit message.

[bensonr]

Some portions of FV3 are still undergoing development, testing, and evaluation. This particular section of code is currently used for short-term prediction and this change is most likely due to the tuning aspects of testing and evaluation.

@lharris4 would need to comment on how this change is documented.

[lharris4]

Hi, Seth. Much of the commented-out area is indeed comment and not code. The terminal velocity diagnostics are leftovers from the old 3d version of the GFDL microphysics; this implementation won't work in the 2d (vectorized) GFDL MP code. They are here to remind us how to implement them if we do restore this diagnostic in the new version of the code. Lucas

…

On Thu, Mar 19, 2020 at 6:04 PM Seth Underwood ***@***.***> wrote: ***@***.**** commented on this pull request. @bensonr <https://github.com/bensonr> I managed to get through about 1/2 of the files for review. I'll still work through the others, but I wanted to give you a chance to comment on what I found so far. I will finish my review tomorrow. As this has a lot of changes, I am unable to look real close at the code changes. I don't think I will mark this a "request changes", but if the items will be addressed, I think it would make the code easier to maintain. ------------------------------ In driver/SHiELD/atmosphere.F90 <#19 (comment)> : > +!--- Need to know the formulation of the mixing ratio being imported into FV3 +!--- in order to adjust it in a consistent manner for advection +!rab subroutine atmosphere_tracer_postinit (IPD_Data, Atm_block) +!rab !--- interface variables --- +!rab type(IPD_data_type), intent(in) :: IPD_Data(:) +!rab type(block_control_type), intent(in) :: Atm_block +!rab !--- local variables --- +!rab integer :: i, j, ix, k, k1, n, nwat, nb, blen +!rab real(kind=kind_phys) :: qwat +!rab +!rab if( nq<3 ) call mpp_error(FATAL, 'GFS phys must have 3 interactive tracers') +!rab +!rab n = mygrid +!rab nwat = Atm(n)%flagstruct%nwat +!rab +!rab!$OMP parallel do default (none) & +!rab!$OMP shared (Atm_block, Atm, IPD_Data, npz, nq, ncnst, n, nwat) & +!rab!$OMP private (nb, k, k1, ix, i, j, qwat) +!rab do nb = 1,Atm_block%nblks +!rab do k = 1, npz +!rab k1 = npz+1-k !reverse the k direction +!rab do ix = 1, Atm_block%blksz(nb) +!rab i = Atm_block%index(nb)%ii(ix) +!rab j = Atm_block%index(nb)%jj(ix) +!rab qwat = sum(Atm(n)%q(i,j,k1,1:nwat)) +!rab Atm(n)%q(i,j,k1,1:nq) = Atm(n)%q(i,j,k1,1:nq) + IPD_Data(nb)%Stateout%gq0(ix,k,1:nq) * (1.0 - qwat) +!rab if (nq .gt. ncnst) then +!rab Atm(n)%qdiag(i,j,k1,nq+1:ncnst) = Atm(n)%qdiag(i,j,k1,nq+1:ncnst) + IPD_Data(nb)%Stateout%gq0(ix,k,nq+1:ncnst) +!rab endif +!rab enddo +!rab enddo +!rab enddo +!rab +!rab call mpp_update_domains (Atm(n)%q, Atm(n)%domain, complete=.true.) +!rab +!rab return +!rab end subroutine atmosphere_tracer_postinit @bensonr <https://github.com/bensonr> is there a reason to keep this commented subroutine? ------------------------------ In driver/SHiELD/atmosphere.F90 <#19 (comment)> : > +!GFDL if ( dnats > 0 ) then +!GFDL do iq = nq-dnats+1, nq +!GFDL do k = 1, npz +!GFDL k1 = npz+1-k !reverse the k direction +!GFDL do ix = 1,blen +!GFDL i = Atm_block%index(nb)%ii(ix) +!GFDL j = Atm_block%index(nb)%jj(ix) +!GFDL Atm(n)%q(i,j,k1,iq) = Atm(n)%q(i,j,k1,iq) + (IPD_Data(nb)%Stateout%gq0(ix,k,iq)-IPD_Data(nb)%Statein%qgrs(ix,k,iq))*rdt +!GFDL! * (IPD_Data%Statein%prsi(ix,k)-IPD_Data(nb)%Statein%prsi(ix,k+1))/Atm(n)%delp(i,j,k1) +!GFDL enddo +!GFDL enddo +!GFDL enddo +!GFDL endif + Another commented section. If there is a reason to keep these, a comment on why would be useful. ------------------------------ In driver/SHiELD/atmosphere.F90 <#19 (comment)> : > +#if defined(OVERLOAD_R4) +#define _DBL_(X) DBLE(X) +#define _RL_(X) REAL(X,KIND=4) +#else +#define _DBL_(X) X +#define _RL_(X) X +#endif This is just a note, and if others agree we should create an issue in NOAA-GFDL/FMS. The CPP macros used when building FMS should be stored in a file (e.g. fms_config.h) that other model components could then #include in the files. This will help when building the components to ensure the same set of macros are used, and is a way we can standardize some of the CPP macros. ------------------------------ In driver/SHiELD/gfdl_cloud_microphys.F90 <#19 (comment)> : > + ! if (id_cond > 0) then + ! do j = js, je + ! do i = is, ie + ! cond (i, j) = cond (i, j) * rgrav + ! enddo + ! enddo + ! used = send_data (id_cond, cond, time, is_in = iis, js_in = jjs) + ! endif + + ! if (id_snow > 0) then + ! used = send_data (id_snow, snow, time, iis, jjs) + ! used = send_data (id_snow, snow, time, is_in = iis, js_in = jjs) + ! if (mp_print .and. seconds == 0) then + ! tot_prec = g_sum (snow, is, ie, js, je, area, 1) + ! if (master) write (*, *) 'mean snow = ', tot_prec + ! endif + ! endif + ! + ! if (id_graupel > 0) then + ! used = send_data (id_graupel, graupel, time, iis, jjs) + ! used = send_data (id_graupel, graupel, time, is_in = iis, js_in = jjs) + ! if (mp_print .and. seconds == 0) then + ! tot_prec = g_sum (graupel, is, ie, js, je, area, 1) + ! if (master) write (*, *) 'mean graupel = ', tot_prec + ! endif + ! endif + ! + ! if (id_ice > 0) then + ! used = send_data (id_ice, ice, time, iis, jjs) + ! used = send_data (id_ice, ice, time, is_in = iis, js_in = jjs) + ! if (mp_print .and. seconds == 0) then + ! tot_prec = g_sum (ice, is, ie, js, je, area, 1) + ! if (master) write (*, *) 'mean ice_mp = ', tot_prec + ! endif + ! endif + ! + ! if (id_rain > 0) then + ! used = send_data (id_rain, rain, time, iis, jjs) + ! used = send_data (id_rain, rain, time, is_in = iis, js_in = jjs) + ! if (mp_print .and. seconds == 0) then + ! tot_prec = g_sum (rain, is, ie, js, je, area, 1) + ! if (master) write (*, *) 'mean rain = ', tot_prec + ! endif + ! endif + ! + ! if (id_rh > 0) then !not used? + ! used = send_data (id_rh, rh0, time, iis, jjs) + ! used = send_data (id_rh, rh0, time, is_in = iis, js_in = jjs) + ! endif + ! + ! + ! if (id_prec > 0) then + ! used = send_data (id_prec, prec_mp, time, iis, jjs) + ! used = send_data (id_prec, prec_mp, time, is_in = iis, js_in = jjs) + ! endif + + ! if (mp_print) then + ! prec1 (:, :) = prec1 (:, :) + prec_mp (:, :) + ! if (seconds == 0) then + ! prec1 (:, :) = prec1 (:, :) * dt_in / 86400. + ! tot_prec = g_sum (prec1, is, ie, js, je, area, 1) + ! if (master) write (*, *) 'daily prec_mp = ', tot_prec + ! prec1 (:, :) = 0. + ! endif + ! endif + + ! call mpp_clock_end (gfdl_mp_clock) More commented code. ------------------------------ In driver/SHiELD/gfdl_cloud_microphys.F90 <#19 (comment)> : > + ! ----------------------------------------------------------------------- + ! fms diagnostics: + ! ----------------------------------------------------------------------- + + ! if (id_cond > 0) then + ! do k = ktop, kbot ! total condensate + ! cond (i) = cond (i) + dp1 (k) * (qlz (k) + qrz (k) + qsz (k) + qiz (k) + qgz (k)) + ! enddo + ! endif + ! + ! if (id_vtr > 0) then + ! do k = ktop, kbot + ! vt_r (i, j, k) = vtrz (k) + ! enddo + ! endif + ! + ! if (id_vts > 0) then + ! do k = ktop, kbot + ! vt_s (i, j, k) = vtsz (k) + ! enddo + ! endif + ! + ! if (id_vtg > 0) then + ! do k = ktop, kbot + ! vt_g (i, j, k) = vtgz (k) + ! enddo + ! endif + ! + ! if (id_vts > 0) then + ! do k = ktop, kbot + ! vt_i (i, j, k) = vtiz (k) + ! enddo + ! endif + ! + ! if (id_droplets > 0) then + ! do k = ktop, kbot + ! qn2 (i, j, k) = ccn (k) + ! enddo + ! endif Any reason to *not* want these diagnostics? Are they for debugging only? ------------------------------ In driver/SHiELD/gfdl_cloud_microphys.F90 <#19 (comment)> : > + if (tc .ge. 0.) then + + ! ----------------------------------------------------------------------- + ! melting of snow + ! ----------------------------------------------------------------------- + + dqs0 = ces0 / p1 (k) - qv + + if (qs > qcmin) then + + ! ----------------------------------------------------------------------- + ! psacw: accretion of cloud water by snow + ! only rate is used (for snow melt) since tc > 0. + ! ----------------------------------------------------------------------- + + if (ql > qrmin) then + factor = denfac (k) * csacw * exp (0.8125 * log (qs * den (k))) + psacw = factor / (1. + dts * factor) * ql ! rate + else + psacw = 0. + endif + + ! ----------------------------------------------------------------------- + ! psacr: accretion of rain by melted snow + ! pracs: accretion of snow by rain + More commented out code. ------------------------------ In driver/SHiELD/gfdl_cloud_microphys.F90 <#19 (comment)> : > + lhi (k) = li00 + dc_ice * tzk (k) + lcpk (k) = lhl (k) / cvm (k) + icpk (k) = lhi (k) / cvm (k) + tcpk (k) = lcpk (k) + icpk (k) + enddo + + do k = ktop, kbot + + ! ----------------------------------------------------------------------- + ! do nothing above p_min + ! ----------------------------------------------------------------------- + + if (p1 (k) < p_min) cycle + + tz = tzk (k) + qv = qvk (k) + ql = qlk (k) + qi = qik (k) + qr = qrk (k) + qs = qsk (k) + qg = qgk (k) + + pgacr = 0. + pgacw = 0. + tc = tz - tice + + if (tc .ge. 0.) then + + ! ----------------------------------------------------------------------- + ! melting of snow + ! ----------------------------------------------------------------------- + + dqs0 = ces0 / p1 (k) - qv + + if (qs > qcmin) then + + ! ----------------------------------------------------------------------- + ! psacw: accretion of cloud water by snow + ! only rate is used (for snow melt) since tc > 0. + ! ----------------------------------------------------------------------- + + if (ql > qrmin) then + factor = denfac (k) * csacw * exp (0.8125 * log (qs * den (k))) + psacw = factor / (1. + dts * factor) * ql ! rate + else + psacw = 0. + endif + + ! ----------------------------------------------------------------------- + ! psacr: accretion of rain by melted snow + ! pracs: accretion of snow by rain + And again. ------------------------------ In driver/SHiELD/gfdl_cloud_microphys.F90 <#19 (comment)> : > + psacw = factor / (1. + dts * factor) * ql ! rate + else + psacw = 0. + endif + + ! ----------------------------------------------------------------------- + ! psacr: accretion of rain by melted snow + ! pracs: accretion of snow by rain + And again. ------------------------------ In driver/SHiELD/gfdl_cloud_microphys.F90 <#19 (comment)> : > + else + psacw = 0. + endif + + ! ----------------------------------------------------------------------- + ! psacr: accretion of rain by melted snow + ! pracs: accretion of snow by rain + This looks like old debug code. I'd suggest using #ifdef if this debug code could be useful in the future. ------------------------------ In driver/SHiELD/gfdl_cloud_microphys.F90 <#19 (comment)> : > + if (p1 (k) < p_min) cycle + + tz = tzk (k) + qv = qvk (k) + ql = qlk (k) + qi = qik (k) + qr = qrk (k) + qs = qsk (k) + qg = qgk (k) + + pgacr = 0. + pgacw = 0. + tc = tz - tice + + if (tc .ge. 0.) then + + ! ----------------------------------------------------------------------- + ! melting of snow + ! ----------------------------------------------------------------------- + + dqs0 = ces0 / p1 (k) - qv + + if (qs > qcmin) then + + ! ----------------------------------------------------------------------- + ! psacw: accretion of cloud water by snow + ! only rate is used (for snow melt) since tc > 0. + ! ----------------------------------------------------------------------- + + if (ql > qrmin) then + factor = denfac (k) * csacw * exp (0.8125 * log (qs * den (k))) + psacw = factor / (1. + dts * factor) * ql ! rate + else + psacw = 0. + endif + + ! ----------------------------------------------------------------------- + ! psacr: accretion of rain by melted snow + ! pracs: accretion of snow by rain + An entire function is commented out. ------------------------------ In driver/SHiELD/gfdl_cloud_microphys.F90 <#19 (comment)> : > + + ! ----------------------------------------------------------------------- + ! psacr: accretion of rain by melted snow + ! pracs: accretion of snow by rain + More. ------------------------------ In model/fv_cmp.F90 <#19 (comment)> : > +! 2050 at 0 deg C; 1972 at -15 C; 1818. at -40 C +! real, parameter:: c_ice = 2106. ! heat capacity of ice at 0.C (same as IFS) +! real, parameter:: c_liq = 4218. ! ECMWF-IFS at 0 deg C + real, parameter:: c_ice = 1972. ! -15 C + real, parameter:: c_liq = 4.1855e+3 ! GFS, at 15 deg C + real, parameter:: cp_vap = cp_vapor ! 4*rv_gas=1846. + real, parameter:: dc_vap = cp_vap - c_liq ! = -2344. isobaric heating/cooling + real, parameter:: dc_ice = c_liq - c_ice ! = 2084 + real, parameter:: tice = 273.16 + real, parameter:: t_wfr = tice - 40. +! Values at 0 Deg C + real, parameter:: hlv0 = 2.5e6 + real, parameter:: hlf0 = 3.3358e5 +! Latent heat at absolute zero: + real, parameter:: Lv0 = hlv0 - dc_vap*tice ! = 3.141264e6 + real, parameter:: li00 = hlf0 - dc_ice*tice ! = -2.355446e5 +! Li (T=113) ~ 0. +!!! real(kind=R_GRID), parameter:: e00 = 610.71 ! saturation vapor pressure at T0 More lines of commented out code. ------------------------------ In model/fv_sg.F90 <#19 (comment)> : > @@ -310,7 +311,7 @@ subroutine fv_subgrid_z( isd, ied, jsd, jed, is, ie, js, je, km, nq, dt, & ! top layer unphysically warm ri = 0. elseif ( tv2<t_min ) then - ri = min(ri, 0.2) + ri = min(ri, 0.1) Just want to verify that this change from 0.2 to 0.1 is desired. I also hope the change, or some explanation is described in a commit message. ------------------------------ In tools/fv_nggps_diag.F90 <#19 (comment)> : > call range_check('UA', Atm(n)%ua, isc, iec, jsc, jec, ngc, npz, Atm(n)%gridstruct%agrid, & - -220., 250., bad_range) + -250., 250., bad_range, Time) More changes to numbers I just want to verify are desired. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#19 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMUQRVGCYPAZEMUBSEPFLKDRIKJGNANCNFSM4LPRDB6Q> .

[underwoo]

As mentioned in previous comments, there are many places throughout the code that have sections of commented out code. I understand that some are references for possible future work, or to implement features that used to work. I suggest adding additional comments to indicate this in the code.

[bensonr]

@mlee03 @underwoo @laurenchilutti - thank you for the reviews. I am merging this code and tagging it so the 2020.02 release (for GFDL models) testing can commence.

[colingladueNOAA]

I will commence the test tagging.