-
Notifications
You must be signed in to change notification settings - Fork 157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Winterwx #614
Winterwx #614
Conversation
…restart file and changed nsradar_reset, radar_reset to something that makes more sense, that is, nsfullradar_diag and fullradar_diag. Also, added new namelist parameter to use new precip ice density with the NOAH LSM. Code changes to upp also made to include the new winter wx diags.
@ericaligo-NOAA Please update the hash for ccpp/framework. |
Thanks, Chunxi. For these diagnostics we would want nearest neighbor. I
was told not specifying intpl_method means the nearest neighbor will be
used.
…On 1/5/2023 2:29 PM, ChunxiZhang-NOAA wrote:
***@***.**** commented on this pull request.
------------------------------------------------------------------------
In ccpp/driver/GFS_diagnostics.F90
<#614 (comment)>:
> @@ -1642,6 +1642,89 @@ subroutine GFS_externaldiag_populate (ExtDiag, Model, Statein, Stateout, Sfcprop
ExtDiag(idx)%data(nb)%var2 => IntDiag(nb)%pratemax(:)
enddo
+ idx = idx + 1
+ ExtDiag(idx)%axes = 2
+ ExtDiag(idx)%name = 'frzr'
+ ExtDiag(idx)%desc = 'accumulated surface freezing rain'
+ ExtDiag(idx)%unit = 'kg/m**2'
+ ExtDiag(idx)%mod_name = 'gfs_phys'
+ ExtDiag(idx)%cnvfac = cn_th
+ allocate (ExtDiag(idx)%data(nblks))
Maybe add ExtDiag(idx)%intpl_method = 'bilinear'?
—
Reply to this email directly, view it on GitHub
<#614 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQ75MK5CUH7G2AK2U5ATW3WQ4OJ7ANCNFSM6AAAAAATMC6YIY>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
@ericaligo-NOAA I see, thanks. |
Here's what I have in my .gitmodules:
[submodule "ccpp/framework"]
path = ccpp/framework
url = https://github.com/NCAR/ccpp-framework
branch = main
What should I change this to?
Eric
…On 1/5/2023 2:32 PM, ChunxiZhang-NOAA wrote:
@ericaligo-NOAA <https://github.com/ericaligo-NOAA> Please update the
hash for ccpp/framework.
—
Reply to this email directly, view it on GitHub
<#614 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQ75MIZPPWSKVA3QL53COLWQ4OTXANCNFSM6AAAAAATMC6YIY>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ericaligo-NOAA Keep .gitmodules as is. do the following steps: cd ccpp/framework; git checkout d4a0003; cd ../..; git status; git add ccpp/framework; git commit; git push origin your branch. |
Done. Thanks for the help!
…On 1/5/2023 4:01 PM, ChunxiZhang-NOAA wrote:
@ericaligo-NOAA <https://github.com/ericaligo-NOAA> Keep .gitmodules
as is. do the following steps: cd ccpp/framework; git checkout
d4a0003; cd ../..; git status; git add ccpp/framework; git commit; git
push origin your branch.
—
Reply to this email directly, view it on GitHub
<#614 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQ75MORDLTUWDZH4JTKJJLWQ4ZD3ANCNFSM6AAAAAATMC6YIY>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ericaligo-NOAA You are very welcome! |
Okay, I removed the commented out lines in the RUC LSM and pushed those
changes to my branch. Hopefully I've addressed all of your
comments/suggestions.
…On 1/5/2023 4:15 PM, ChunxiZhang-NOAA wrote:
@ericaligo-NOAA <https://github.com/ericaligo-NOAA> You are very welcome!
—
Reply to this email directly, view it on GitHub
<#614 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQ75MKYKCMCYCFNM5LKXJ3WQ42X7ANCNFSM6AAAAAATMC6YIY>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@ericaligo-NOAA Please make all your changes, then commit your changes in one commit. |
The revision (e88dbea) of UPP submodule is an appropriate UPP version for this PR. |
Thanks for the confirmation. I think that my question was in reference to the fact that updating the UPP hash presumably has a lot more changes than just what was required for @ericaligo-NOAA 's winter weather diagnostics changes. I wanted to confirm with the UFS code managers that all of the code changes in UPP represented by the update to the hash are desired/expected. |
Eric recently made changes for post processing new winter weather variables in UPP repository. To generate these variables in GRIB2, updating revision of upp submodule is needed. |
For example, the previous UPP commit hash for the fv3atm develop branch was back in August 2022: upp @ e227247 |
I could help to verify the changes of UPP outputs in grib2 besides new winter weather changes. |
Thanks for the detailed analysis. I agree it's not doing anything at
this point. I don't know the plans for the future if someone decided to
use it after the call to SFCTMP where I saw:
RHOSNF(I,J) = RHOSNFALL (I,J).
It dies right there, but folks need to be careful about using the RHOSNF
in the future. I'm fine with leaving it alone.
Eric
…On 2/9/2023 1:58 PM, Grant Firl wrote:
***@***.**** commented on this pull request.
------------------------------------------------------------------------
In ccpp/data/GFS_typedefs.F90
<#614 (comment)>:
> @@ -1729,6 +1730,14 @@ module GFS_typedefs
real (kind=kind_phys), pointer :: toticeb(:) => null() !< accumulated ice precipitation in bucket (kg/m2)
real (kind=kind_phys), pointer :: totsnwb(:) => null() !< accumulated snow precipitation in bucket (kg/m2)
real (kind=kind_phys), pointer :: totgrpb(:) => null() !< accumulated graupel precipitation in bucket (kg/m2)
+ real (kind=kind_phys), pointer :: frzr (:) => null() !< accumulated surface freezing rain (m)
+ real (kind=kind_phys), pointer :: frzrb (:) => null() !< accumulated surface freezing rain in bucket (m)
+ real (kind=kind_phys), pointer :: frozr (:) => null() !< accumulated surface graupel (m)
+ real (kind=kind_phys), pointer :: frozrb (:) => null() !< accumulated surface graupel in bucket (m)
+ real (kind=kind_phys), pointer :: tsnowp (:) => null() !< accumulated surface snowfall (m)
+ real (kind=kind_phys), pointer :: tsnowpb(:) => null() !< accumulated surface snowfall in bucket (m)
+ real (kind=kind_phys), pointer :: rhonewsn1(:) => null() !< precipitation ice density outside RUC LSM (kg/m3)
+ real (kind=kind_phys), pointer :: rhosnf(:) => null() !< precipitation ice density inside RUC LSM (kg/m3)
Regarding the rhosnf question within module_sf_ruclsm, it is always
only a diagnostic, but you're right that there are instances where it
will be set to nonsense.
1. Let's look at when exticeden == T. |rhosnf| is intent(out) in the
lsmruc subroutine, so it should always be set to something, but it
is not. It is only initialized to -1.e3 conditionally since it is
in the |if(init .and. (lsm_cold_start) .and. iter == 1) then|
block. |rhosnfall| is initialized to rhosnf (which is
/uninitialized/ except for the first time step) prior to calling
SFCTMP. Then, it is unmodified in SFCTMP and is used to set
|rhosnf| upon return to lsmruc. So, this variable is an
uninitialized diagnostic when exticeden == T. Perhaps this is OK
since the externally-calculated variable (the one that is used)
still has its value and the internal one is meaningless at this point.
2. Let's look at when exticeden == F. The same situation applies
except that |rhosnfall| is conditionally given a value in SFCTMP.
Otherwise, |rhosnf| returns nonsense.
Critically, though, |rhosnf| potentially returning nonsense was the
behavior */before/* any of the modifications in this PR. It only ever
had a value for the following conditions:
* |if(init .and. (lsm_cold_start) .and. iter == 1)| in lsmruc
* |IF((XLAND(I,J)-1.5) < 0.)| in lsmruc
* |IF(NEWSN.GT.0.)| in sfctmp
The initialization that happens to |rhosnfr| in the CCPP cap,
lsm_ruc_run, is overridden by the intent(out) status of the variable
in lsmruc.
I think that the concerns can be removed if |rhosnf| is declared as
intent(inout) in lsmruc. That way, it should always at least have its
value given at the first timestep. But, like I said, since it is only
a diagnostic, it may not matter much.
—
Reply to this email directly, view it on GitHub
<#614 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQ75MKXHHHLQWEKH3B3GW3WWU45RANCNFSM6AAAAAATMC6YIY>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
@grantfirl Could you make a final review to PR at your convenience? |
@ericaligo-NOAA ccpp pr was merged. Please, update submodule pointer and revert the chnage in gitmodules. |
@jkbk2004 Done. |
Thanks! |
* Six new winter wx diags added to output, accumulated fields added to restart file and changed nsradar_reset, radar_reset to something that makes more sense, that is, nsfullradar_diag and fullradar_diag. Also, added new namelist parameter to use new precip ice density with the NOAH LSM. Code changes to upp also made to include the new winter wx diags.
* Six new winter wx diags added to output, accumulated fields added to restart file and changed nsradar_reset, radar_reset to something that makes more sense, that is, nsfullradar_diag and fullradar_diag. Also, added new namelist parameter to use new precip ice density with the NOAH LSM. Code changes to upp also made to include the new winter wx diags.
Description
Merging winterwx branch into develop branch
This PR adds six winter weather diagnostics to the FV3 history files to be read into the UPP. A variable precip ice density currently in the RUC LSM is moved to GFS_MP_generic_post.F90 with the density then passed into the RUC, NOAH and NOAH MP LSMs. The RUC LSM will use this precip ice density. The NOAH and NOAH MP LSMs have the option of using it. For NOAH, a new namlest variable is defined if the new density is to be used. For NOAH MP, an existing namelist variable is used with option 5 in order to use the new density (option 4 is default). The accumulated/continuous winter weather fields are passed into the restart files.
Also, the namelist parameter, nsradar_reset was more appropriately renamed nsfullradar_diag.
It also fixes a bug in the frequency over which the full radar reflectivity (water coated ice) is computed. With the bug, the full radar reflectivity was computed one time step before the top of the hour (when parameter was set to 3600s). The fix allows the full radar reflectivity to be computed at the top of the hour.
Upstream Issue(s) addressed
NOAA-EMC/UPP#568
FV3 Bug fixes:
PRs:
Results will change for any SDF that includes the RUC LSM.Results will change if lradar=.true. because of the bug fix. Results will change since UPP expects all six diagnostics to be read in and no diag_tables were updated.
Testing
No RTs were run. Code changes were tested in a slightly older verison of ufs-weather-model in RRFS as well as global configurations. New RTs will be run on hera.
[ x] hera.intel
Dependencies
None
Issue(s) addressed
ufs-community/ufs-weather-model#1449