Collect PRs #613, #628, #634, #635 / update long names and vertical dimension for atmosphere_heat_diffusivity and atmosphere_momentum_diffusivity #641
Conversation
…_2017() call; also cires_orowam2017.F90 appears to be a duplicate and is not used by any scheme
… intel/debug build
standards. Fixes NCAR#625
pulling latest master into local ltp-bugfix
…g flags for ugwpv1_gsldrag and GFS_DCNV_generic.F90
…ure, temperature, and specific humidity for Thompson MP. Use extended arrays plyr, tlyr, and qlyr respectively.
…dkt) and atmosphere_momentum_diffusivity (aka dku), update long names
climbfuji
changed the title
…llect_various_prs_20210422
climbfuji
requested review from
grantfirl,
JulieSchramm and
llpcarson
as code owners
| write(6,*) ' error in opening file ',trim(fngrib) | ||
| print *,'error in opening file ',trim(fngrib) | ||
| write(6,*) ' FATAL ERROR: in opening file ',trim(fngrib) | ||
| print *,'FATAL ERROR: in opening file ',trim(fngrib) | ||
| call abort |
There was a problem hiding this comment.
This is not necessarily related to this PR, but I didn't notice until now -- sfcsub.F is full of "call abort" statements, but CCPP schemes are not supposed to stop the model. Was this "grandfathered" in with the GFS physics originally? I'm guessing that since the error-handling was touched for this file as part of this PR and that it was not modified to use CCPP error-handling that it was a "bridge too far"?
There was a problem hiding this comment.
I mentioned that in my review, too. UFS_UTILS is now also pointing to this file / using this file. We should definitely make the change in the near future (as I suggested). The much bigger picture here is that we need to make sure that all error messages created by CCPP are compliant with NCO error handling. Hopefully this will not be a problem for other users of the CCPP.
There was a problem hiding this comment.
Thanks. I see your comment in #628. I didn't review the combined PRs individually, so sorry for the duplication.
| @@ -116,7 +116,7 @@ subroutine hedmf_run (im,km,ntrac,ntcw,dv,du,tau,rtg, & | |||
| & dtsfc(im), dqsfc(im), & | |||
| & hpbl(im) | |||
| real(kind=kind_phys), intent(out) :: & | |||
| & dkt(im,km-1), dku(im,km-1) | |||
| & dkt(im,km), dku(im,km) | |||
There was a problem hiding this comment.
Is there any potential issue that the dkt and dku are defined as km levels but are only initialized below for km1 levels? This probably will not affect FV3ATM, since they are already initialized at the FV3ATM level before passing into ccpp/physics. But, if this is also used in other applications (SCM or other models), then they may get uninitialized values for the extra one vertical level.
!> - Initialize diffusion coefficients to 0 and calculate the total radiative heating rate (dku, dkt, radx)
do k = 1,km1
do i = 1,im
dku(i,k) = 0.
dkt(i,k) = 0.
dktx(i,k) = 0.
cku(i,k) = 0.
ckt(i,k) = 0.
tem = zi(i,k+1)-zi(i,k)
radx(i,k) = tem*(swh(i,k)*xmu(i)+hlw(i,k))
enddo
enddo
There was a problem hiding this comment.
To comment on the SCM, this should not be an issue. Both dku and dkt are in the interstitial DDT and both entire arrays get reset to zero during every physics timestep (FV3 and SCM behave identically here).
There was a problem hiding this comment.
It is always the responsibility of the component that defines/allocates data to initialize it properly. One cannot rely on some other component doing that. We will also talk about the definition of intent(out) in the ccpp framework meeting today, which is somewhat related.
There was a problem hiding this comment.
@BinLiu-NOAA I stand corrected to what I said beforehand, according to the Fortran standard, a variable declared as intent(out) must be set entirely, not just partially, inside the scheme (i.e. going back to our old definition in CCPP). A scheme cannot rely on the other pieces being initialized elsewhere. I am going to create an issue to follow up on this PR and initialize the arrays dku and dkt correctly in all schemes that have them declared as intent(out).
There was a problem hiding this comment.
Thanks, @climbfuji! Sounds reasonable to me.
This PR collects various small PRs (see there for a description of the changes):
sfcsub.Fto conform to NCEP/NCO standards from @GeorgeGayno-NOAAugwpv1_gsldragbug fixes, andGFS_typdefsupdates associated with diag flags forugwpv1_gsldrag.F90andGFS_DCNV_generic.F90from @mdtoyGFS_rrtmg_prefrom @matusmartiniAdditional changes:
atmosphere_heat_diffusivity(akadkt) andatmosphere_momentum_diffusivity(akadku) and update their long namesFixes #639
Fixes #632
Fixes #625
Associated PRs:
NCAR/ccpp-framework#368
#641
NOAA-EMC/fv3atm#291
ufs-community/ufs-weather-model#541
For regression testing, see ufs-community/ufs-weather-model#541