Hydraulic redistribution #1126
Hydraulic redistribution #1126
Conversation
ekluzek
added
PR status: ready
|
@djk2120 you want these new variables on the PPE work right? Is it required for that work or just helpful? |
|
I would say these are very helpful for the PPE work, but not strictly
required.
…On Thu, Aug 27, 2020 at 1:02 PM Erik Kluzek ***@***.***> wrote:
@djk2120 <https://github.com/djk2120> you want these new variables on the
PPE work right? Is it required for that work or just helpful?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1126 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADNHR5WON4VE4P63XCLMAZDSC2UTVANCNFSM4QNI6UFQ>
.
|
|
I think we should consider this a requirement for the PPE.
…On Thu, Aug 27, 2020 at 1:05 PM djk2120 ***@***.***> wrote:
I would say these are very helpful for the PPE work, but not strictly
required.
On Thu, Aug 27, 2020 at 1:02 PM Erik Kluzek ***@***.***>
wrote:
> @djk2120 <https://github.com/djk2120> you want these new variables on
the
> PPE work right? Is it required for that work or just helpful?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#1126 (comment)>, or
> unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/ADNHR5WON4VE4P63XCLMAZDSC2UTVANCNFSM4QNI6UFQ
>
> .
>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1126 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFABYVACNVEBEXTJ7VJMZ23SC2U6HANCNFSM4QNI6UFQ>
.
|
|
@negin513 since @dlawrenncar wants this for PPE work, I assigned it to you. You could combine this with the #1102 PR or make it as a separate tag, which ever you think is more efficient for you. We should make sure a few of us review this, but it is small and should be bit-for-bit so should be easy to handle. |
|
I merged this as part of #1187 but during the testing I've noticed FAILs in
Therefore, I conclude the changes here are not BFB and causing FAILS in I checked the outputs to figure out what are the fields that are not BFB.
Please let me know your thoughts on how to resolve this. |
|
Hi Negin,
Some of this is over my head as I am not familiar with the testing suite.
What is ERP/ERS?
Both VEGWPPD and VEGWPLN are new fields introduced by this pull request,
that would not have been in previous versions of the model. Does that
explain it?
…-Daniel
On Wed, Oct 14, 2020 at 2:43 PM Negin Sobhani ***@***.***> wrote:
I merged this as part of #1187 <#1187>
but during the testing I've noticed FAILs in COMPARE_base_rest phase.
So, I did the following:
1.
I brought this up to the master and tested it again with clm_short
test suite and I still receive FAILs in COMPARE_base_rest phase in ERP
and ERS tests.
2.
I used this branch as is and tested it with clm_short test suite and I
still receive FAILS in COMPARE_base_rest phase for ERP and ERS tests.
Therefore, I conclude the changes here are not BFB and causing FAILS in
COMPARE_base_rest.
I checked the outputs to figure out what are the fields that are not BFB.
- From my preliminary investigation, VEGWPPD and VEGWPLNare the two
fields that are not BFB. For example for
ERS_D_Ld3.f10_f10_musgs.I1850Clm50BgcCrop.cheyenne_intel.clm-default
test :
RMS VEGWPPD 1.1501E+04 NORMALIZED 3.6253E-02
RMS VEGWPLN 1.3917E+03 NORMALIZED 1.5362E-03
Please let me know your thoughts.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1126 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADNHR5VIWTM5673JOB5RBG3SKYENHANCNFSM4QNI6UFQ>
.
|
|
The ERS and ERP are exact restart tests. They are showing that the new history fields are showing different results after restart then when run all in one go. Usually, the easiest way to fix that is to just add these two to the restart file. I would do that and see if it fixes the problem. It is important to get these restart tests passing. That said, if we don't have to have these on the restart file that would be preferred. It seems like you might be able to look into it and see if it really has to be on the restart file or not. |
|
This is merged with #1187. |
Description of changes
Adding four new history fields relevant to plant hydraulic processes:
CTSM Issues Fixed (include github issue #):
1123
Are answers expected to change (and if so in what way)?
No.
Any User Interface Changes (namelist or namelist defaults changes)?
No, except for the availability of new history fields.
Testing performed, if any:
--compset I2000Clm50Sp --res f09_g17_gl4
Ran a five-day global simulation to confirm that the new history fields were writing out as expected. Compared local noon and predawn vegwp to half-hourly vegwp (at one point) and found agreement between the daily average and the corresponding timeslices in the half-hourly output.