Updates for ufs/dev PR#30 #1
Conversation
|
@ericaligo-NOAA I removed the rhosnf variable because with your changes, it isn't being used anymore in RUC LSM and it isn't being used anywhere else in UFS. So, to save memory and avoid code confusion by having similar variables in the code, I thought it best to remove. We should probably check with @tanyasmirnova if this is OK. @tanyasmiranova What do you think? I can always put it back in if you both prefer, but it seems better to me to remove it. |
|
After looking through the module_sf_ruclsm.f90, rhosnf, which is rhosnfall, is not used. in subroutine SFCTMP, it's commented out. I'm comfortable merging. It can be added back if Tanya believes it should be. What do you think? |
|
@ericaligo-NOAA @grantfirl Eric and Grant, did you run a test before/after change to RUC LSM? The results should be identical, then we could remove computation of rhosnfall from RUC LSM. If they are not identical, we should keep RUC LSM unchanged. |
|
It might be best just to leave that part alone?
…On 1/13/2023 1:47 PM, tanyasmirnova wrote:
@ericaligo-NOAA <https://github.com/ericaligo-NOAA> @grantfirl
<https://github.com/grantfirl> Eric and Grant, did you run a test
before/after change to RUC LSM? The results should be identical, then
we could remove computation of rhosnfall from RUC LSM. If they are not
identical, we should keep RUC LSM unchanged.
—
Reply to this email directly, view it on GitHub
<#1 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQ75MK23ALB2ORIE75NYYLWSGPNHANCNFSM6AAAAAATYYZFTI>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@ericaligo-NOAA I would prefer no changes in RUC LSM. The computation will be repeated, but it is not a big expense. I will get to this later and test using the rhonewsn1 from GFS_MP_generic_post.F90. As we discussed last week, the result will not actually be identical, because the 1-st level temperature could be different at the next time step when RUC LSM is called, right? |
|
You are correct. Just to be clear, today's question regarding rhosnf is
completely separate from the changes in my PR related to rhonewsn1. I
believe Grant saw rhosnf as not doing anything and wanted to remove it.
Eric
…On 1/13/2023 3:04 PM, tanyasmirnova wrote:
@ericaligo-NOAA <https://github.com/ericaligo-NOAA> I would prefer no
changes in RUC LSM. The computation will be repeated, but it is not a
big expense. I will get to this later and test using the rhonewsn1
from GFS_MP_generic_post.F90. As we discussed last week, the result
will not actually be identical, because the 1-st level temperature
could be different at the next time step when RUC LSM is called, right?
—
Reply to this email directly, view it on GitHub
<#1 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQ75MLAETS4Q6IOHHTQRATWSGYNPANCNFSM6AAAAAATYYZFTI>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@ericaligo-NOAA @grantfirl 'rhosnf' is the same as newsnow1, but computed internally in RUC LSM. It would be nice to have both of them: one computed internally - rhosnf, and another computed in GFS_MP_generic_post - rhonewsn1. I would like to see how close they are. |
|
@tanyasmirnova @ericaligo-NOAA Eric is correct that my intention was to create CCPP standard names that were consistent with previous ones and the rules, although the |
|
@grantfirl Yes, it is acceptable and desirable. it would like to keep two options within RUC LSM. Thank you very much. |
… use externally-calculated value
There was a problem hiding this comment.
@grantfirl @ericaligo-NOAA Thank you for putting the internal RUC LSM formulation back in. I have just one comment, otherwise, all changes look good to me.
physics/lsm_ruc.F90
Outdated
| @@ -747,6 +748,7 @@ subroutine lsm_ruc_run & ! inputs | |||
| acrunoff(i,j) = 0.0 | |||
| snfallac_lnd(i,j) = 0.0 | |||
| snfallac_ice(i,j) = 0.0 | |||
| rhosnfr(i,j) = 0.0 | |||
There was a problem hiding this comment.
@grantfirl could you please initialize rhosnfr as a flag rhosnfr(i,j) = -1.e3?
|
@tanyasmirnova I changed the initialization of rhosnfr in lsm_ruc_run as you suggested. Thanks for the review. This should be ready to merge if @ericaligo-NOAA is OK with it. I did run RTs on Hera/Intel and it compiles/runs at least. There seemed to be an issue with the regional_atmaq tests, however. I don't know if they're related to the winterwx changes, but it was complaining about an invalid namelist parameter. |
|
Speaking of namelist parameters, this is a reminder that with the latest changes, when using RUC LSM, one will also need to set the |
|
@grantfirl, vrbliceden_noah is only needed when running with the NOAH LSM with the new variable precip ice density. If it's not specified in the namelist, the default is the original precip ice density specified in the NOAH LSM. |
|
@grantfirl @ericaligo-NOAA I agree, we have to change vrbliceden_noah to simply vrbliceden or vrbliceden_mp (because it is computed directly after call to MP scheme). |
|
However, it's only used for the NOAH LSM, so I believe having noah as
part of the parameter is important.
…On 1/19/2023 12:39 PM, tanyasmirnova wrote:
@grantfirl <https://github.com/grantfirl> @ericaligo-NOAA
<https://github.com/ericaligo-NOAA> I agree, we have to change
vrbliceden_noah to simply vrbliceden or vrbliceden_mp (because it is
computed directly after call to MP scheme).
—
Reply to this email directly, view it on GitHub
<#1 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQ75MM7N5ZYMBDFITXKZH3WTF353ANCNFSM6AAAAAATYYZFTI>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@ericaligo-NOAA From @tanyasmirnova suggestion, the latest commits of my changes added back the RUC LSM internally calculated value of frozen precip density and the SAME flag that you used for NOAH LSM is now used to control whether RUC LSM uses the frozen precip density calculated in GFS_MP_generic or the original internal algorithm. So, the flag is no longer specific to NOAH and is applicable for both NOAA LSM and RUC LSM. |
|
One issue is that regardless of where the variable precip ice density is computed, it's always the variable precip ice density. Using vrbliceden = F with the RUC LSM suggests that it's not a variable precip ice density. That parameter was solely meant for NOAH LSM and if the user wanted to use the original approach vs the new variable precip ice density. I understand the desire to allow the RUC LSM to do both, but perhaps a totally different name then needs to be used. if ( lsm == lsm_ruc .or. (lsm == lsm_noahmp .and. iopt_snf == 5) .or. vrbliceden_noah == .true.) then to Eric |
|
@ericaligo-NOAA Excellent points. So, it seems that there are two choices: My preference, for simplicity's sake, is the first one. It seems a bit overkill to have two different flags controlling this for different LSMs (in addition to the integer option for NoahMP). For the first one, the IF statement would be: Yet another option is to have the newly proposed |
|
I like that new parameter name, exticeden. I like option 1 or 3. For
option 3, changes would need to be made to NOAH and NOAHMP. For example,
instead of vrbliceden_noah, we would have exticeden. For NOAH MP, would
we simply replace opt_snf == 5 with exticeden == T? See below. Also,
for NOAH MP, we'd have to pass exticeden into the driver and the
routine. As a result, perhaps option 1 is the best solution since it
doesn't involve adding any new parameters to the NOAH MP driver or
subroutine? For NOAH, we still need to replace vrbliceden_noah with the
new name (exticeden) in sflx.f and the driver.
NOAH
if(.not. *vrbliceden_noah*) then
if (tempc <= -15.0) then
dsnew = 0.05
else
dsnew = 0.05 + 0.0017*(tempc + 15.0)**1.5
endif
else
dsnew = rhonewsn*0.001
endif
NOAH MP:
! hedstrom nr and jw pomeroy (1998), hydrol. processes, 12, 1611-1625
! fresh snow density
bdfall = min(120.,67.92+51.25*exp((sfctmp-tfrz)/2.59)) !mb/an:
change to min
if(opt_snf == 4 .or. *opt_snf == 5*) then
prcp_frozen = prcpsnow + prcpgrpl + prcphail
if(prcpnonc > 0. .and. prcp_frozen > 0.) then
fpice = min(1.0,prcp_frozen/prcpnonc)
fpice = max(0.0,fpice)
if(opt_snf==4) bdfall = bdfall*(prcpsnow/prcp_frozen) +
rho_grpl*(prcpgrpl/prcp_frozen) + &
rho_hail*(prcphail/prcp_frozen)
if(*opt_snf==5*) bdfall = parameters%prcpiceden
else
fpice = 0.0
endif
…On 1/19/2023 3:36 PM, Grant Firl wrote:
@ericaligo-NOAA <https://github.com/ericaligo-NOAA> Excellent points.
So, it seems that there are two choices:
-- change vrbliceden_noah to something like exticeden to better
capture what the flag means (for "external" ice density with respect
to LSM)
-- add an additional flag, e.g. exticeden_ruc, to go along with
vrbliceden_noah.
My preference, for simplicity's sake, is the first one. It seems a bit
overkill to have two different flags controlling this for different
LSMs (in addition to the integer option for NoahMP). For the first
one, the IF statement would be:
|if ((lsm == lsm_noahmp .and. iopt_snf == 5) .or. exticeden == .true.)
then|
For the second option, the IF statement would be:
|if ( (lsm == lsm_ruc .and. exticeden) .or. (lsm == lsm_noahmp .and.
iopt_snf == 5) .or. (lsm == lsm_noah .and. vrbliceden_noah == .true.))
then|.
Yet another option is to have the newly proposed |exticeden| flag be a
"master" control for this functionality for all LSMs. E.g., we can
set/override iopt_snf to 5 if exticeden == T for NOAHMP. Then, the IF
statement only needs to check for |if (exticeden) then|. Thoughts?
—
Reply to this email directly, view it on GitHub
<#1 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQ75MIIMYO7LRCNHGEXII3WTGQTBANCNFSM6AAAAAATYYZFTI>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@grantfirl @ericaligo-NOAA Opinion 3 where "exticeden flag is a "master" control for this functionality for all LSMs" makes the most sense to me. |
|
I'm copying @barlage because we talked about using the opt_snf = 5 flag in order to avoid passing an additional parameter into the NOAH MP driver and module_sf_noahmplsm.f90, which we would have to do if we go with option 3, that is, using the parameter exticeden instead of iopt_snf to determine if NOAH MP uses the original density formulation or the new one. |
|
@ericaligo-NOAA We could continue to use opt_snf = 5 with NoahMP and option 3. In GFS_typedefs.F90/control_initialize(), we could add a statement like: and make sure to print this out when writing out NOAHMP options in GFS_typedefs.F90/control_print(). This avoids passing in a new flag to NOAHMP. |
|
That would work. Thanks!
…On 1/19/2023 4:23 PM, Grant Firl wrote:
@ericaligo-NOAA <https://github.com/ericaligo-NOAA> We could continue
to use opt_snf = 5 with NoahMP and option 3. In
GFS_typedefs.F90/control_initialize(), we could add a statement like:
|if (lsm == lsm_noahmp .and. iopt_snf == 4 .and. exticeden) then|
|iopt_snf = 5|
and make sure to print this out when writing out NOAHMP options in
GFS_typedefs.F90/control_print(). This avoids passing in a new flag to
NOAHMP.
—
Reply to this email directly, view it on GitHub
<#1 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALQ75MJAQKV42GTWIVQWUDDWTGWGNANCNFSM6AAAAAATYYZFTI>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@grantfirl very good solution, Grant! Thank you. |
|
OK, the solution that we discussed is coded up and pushed here. |
Remove MYNN SFC logic from noahmpdrv.F90 and fix CMakeLists.txt authors
update CODEOWNERS to replace Chunxi with Qingfu
Changes: