interaction with snow bugfix causes restart failures with ED #74

bandre-ucar · 2016-06-10T17:04:48Z

Summary of Issue:

CLM 5 development included a series changes and bugfixes to snow, see clm4_5_x_r121, r141, r142, r141, r149, r153.

ED seems to be compatible with most of these, but a bugfix in r153 causes ED f10 and f45 restarts to break. See comments in #46. The exact change which causes restarts to break is:

components/clm/src/biogeophys/CanopyHydrologyMod.F90
 @@ -518,7 +518,7 @@ subroutine CanopyHydrology(bounds, &

              !======================  FSCA PARAMETERIZATIONS  ======================
               ! fsca parameterization based on *changes* in swe
               ! first compute change from melt during previous time step
 -             if(snowmelt(c) >= 0._r8) then
 +             if(snowmelt(c) > 0._r8) then

                  smr=min(1._r8,(h2osno(c))/(int_snow(c)))

Sean Swenson said that the changes is correct, because that if block should only be changing frac_sno when there is snow melt, not when snow melt is zero. If there is no melt, then the frac_sno should be left at the previous value until another process modifies it.

ED interacts with snow through the frac_sno_eff variable, used to determine the fraction_exposed at each height class. See EDCLMLinkMod.F90.

Expected behavior and actual behavior:

ED should restart correctly at f10 and f45 resolutions. f09 and f19 are not expected to work (#14 and #43).

Visualizing diffs in restart files, the error occurs at a single point in the antarctic, just north of McMurdo.

Steps to reproduce the problem (should include create_newcase or create_test command along with any user_nl or xml changes):

Checkout any version of ED after the merge with r153 and run a restart test at f45 or f10.

What is the changeset ID of the code, and the machine you are using:

56dfb03 and any later changes.

have you modified the code? If so, it must be committed and available for testing:

no

The text was updated successfully, but these errors were encountered:

rosiealice · 2016-06-14T13:12:26Z

Ben, thanks for narrowing this down.

It seems to me that there might now be a problem with frac_sno not actually
being set in the new version.

If there is snow (h2osno(c)>1, line 516 in canopyhydrology) then
previously, we always set frac_sno because we always went into the
snowmelt=true section of the code (line 512). Now, we -only- do that if
there is melting, but I think what is missing is a statement for working
out frac_sno if there is
a) existing snow,
b) no melting
c) no new snow.

I can't find that logic. Maybe it's in a different file. It seems wierd
that the code should work at all without it, which leads me to suspect that
it lives elsewhere. The form of the equation we should use also isn't
obvious to me because it seems to be calculated in the other two terms
(melting and new snow) from the delta in frac_sno, so the normal 'snow->
frac_sno' calculation doesn't seem to be in that section.

Sean, does this make sense?

Cheers,
-Rosie

On 10 June 2016 at 11:04, Ben Andre [email protected] wrote:

Summary of Issue:

CLM 5 development included a series changes and bugfixes to snow, see
clm4_5_x_r121, r141, r142, r141, r149, r153.

ED seems to be compatible with most of these, but a bugfix in r153 causes
ED f10 and f45 restarts to break. See comments in #46
#46. The exact change
56dfb03#diff-602d885aadee563b43fd78e088375899R521
which causes restarts to break is:

components/clm/src/biogeophys/CanopyHydrologyMod.F90
@@ -518,7 +518,7 @@ subroutine CanopyHydrology(bounds, &
          !======================  FSCA PARAMETERIZATIONS  ======================
           ! fsca parameterization based on *changes* in swe
           ! first compute change from melt during previous time step
        if(snowmelt(c) >= 0._r8) then
        if(snowmelt(c) > 0._r8) then

          smr=min(1._r8,(h2osno(c))/(int_snow(c)))
Sean Swenson said that the changes is correct, because that if block
should only be changing frac_sno when there is snow melt, not when snow
melt is zero. If there is no melt, then the frac_sno should be left at
the previous value until another process modifies it.

ED interacts with snow through the frac_sno_eff variable, used to
determine the fraction_exposed at each height class. See EDCLMLinkMod.F90
https://github.com/NGEET/ed-clm/blob/56dfb03035031389d99c89057ec12fe4bfeb6546/components/clm/src/ED/main/EDCLMLinkMod.F90#L1388
.
Expected behavior and actual behavior:

ED should restart correctly at f10 and f45 resolutions. f09 and f19 are
not expected to work (#14 #14 and
#43 #43).

Visualizing diffs in restart files, the error occurs at a single point in
the antarctic, just north of McMurdo.
Steps to reproduce the problem (should include create_newcase or
create_test command along with any user_nl or xml changes):

Checkout any version of ED after the merge with r153 and run a restart
test at f45 or f10.
What is the changeset ID of the code, and the machine you are using:

56dfb03
56dfb03
and any later changes.
have you modified the code? If so, it must be committed and available for
testing:

no

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#74, or mute the thread
https://github.com/notifications/unsubscribe/AMWsQx-rfMcWLG2oBN6NfmhHvibo4-4rks5qKZkxgaJpZM4IzJDQ
.

Dr Rosie A. Fisher

Terrestrial Sciences Section
Climate and Global Dynamics
National Center for Atmospheric Research
1850 Table Mesa Drive
Boulder, Colorado, 80305
USA.
+1 303-497-1706

http://www.cgd.ucar.edu/staff/rfisher/

bandre-ucar · 2016-06-14T14:48:49Z

From my discussion with Sean Swenson and tracing through the code, I think the logic is that that frac_sno is being updated by canopy hydrology. For the conditions you described (existing snow, no melt, no new), it was set either by cold start, restart or the previous time step. There is no update for the current step because nothing changed.

rosiealice · 2016-06-14T16:13:15Z

That sounds right. Apologies for my earlier missing of that train of
thought.

In the previous world, when there was snow, it was actually being updated
every timestep in this routine, and now it isn't, so I wonder if there's
something weird with the restarting logic that this exposes. Having said
that, it seems perfectly normal. I think I'd test whether it was
frac_sno_eff or snowdp breaking by setting one or other of them to zero in
the ED snow depth function. I'd do that myself, but I don't have the
restart tests ready to go....

On 14 June 2016 at 09:48, Ben Andre [email protected] wrote:

From my discussion with Sean Swenson and tracing through the code, I think
the logic is that that frac_sno is being updated by canopy hydrology.
For the conditions you described (existing snow, no melt, no new), it was
set either by cold start, restart or the previous time step. There is no
update for the current step because nothing changed.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#74 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AMWsQ6dtfZB0n_5fnHCtRZecYpHT4q35ks5qLr9TgaJpZM4IzJDQ
.

Dr Rosie A. Fisher

Terrestrial Sciences Section
Climate and Global Dynamics
National Center for Atmospheric Research
1850 Table Mesa Drive
Boulder, Colorado, 80305
USA.
+1 303-497-1706

http://www.cgd.ucar.edu/staff/rfisher/

rosiealice · 2016-06-14T16:36:51Z

Two things from conversation with Sean....

The 'normal' ELAI code uses only snow_depth and not
snow_depth*frac_sno_eff to determine the height of the snow covering the
leaves. I think the ED version is more correct, but it might be worth
trying only snow_depth in the term to see if thats the answer.
It -might- be something to do with actually setting the snowdp
variable, Sean thinks. If we used the term snow_depth*frac_sno_eff
instead of the resultant snowdp term does that do anything?

We'll keep thinking about it.
thanks all...

On 14 June 2016 at 11:12, rosie fisher [email protected] wrote:

That sounds right. Apologies for my earlier missing of that train of
thought.

In the previous world, when there was snow, it was actually being updated
every timestep in this routine, and now it isn't, so I wonder if there's
something weird with the restarting logic that this exposes. Having said
that, it seems perfectly normal. I think I'd test whether it was
frac_sno_eff or snowdp breaking by setting one or other of them to zero in
the ED snow depth function. I'd do that myself, but I don't have the
restart tests ready to go....

On 14 June 2016 at 09:48, Ben Andre [email protected] wrote:

From my discussion with Sean Swenson and tracing through the code, I
think the logic is that that frac_sno is being updated by canopy
hydrology. For the conditions you described (existing snow, no melt, no
new), it was set either by cold start, restart or the previous time step.
There is no update for the current step because nothing changed.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#74 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AMWsQ6dtfZB0n_5fnHCtRZecYpHT4q35ks5qLr9TgaJpZM4IzJDQ
.

Dr Rosie A. Fisher

Terrestrial Sciences Section
Climate and Global Dynamics
National Center for Atmospheric Research
1850 Table Mesa Drive
Boulder, Colorado, 80305
USA.
+1 303-497-1706

http://www.cgd.ucar.edu/staff/rfisher/

Dr Rosie A. Fisher

Terrestrial Sciences Section
Climate and Global Dynamics
National Center for Atmospheric Research
1850 Table Mesa Drive
Boulder, Colorado, 80305
USA.
+1 303-497-1706

http://www.cgd.ucar.edu/staff/rfisher/

bandre-ucar · 2016-06-14T16:52:07Z

You can clone my version of the repo:

git clone [email protected]:bandre-ucar/ed-clm.git
cd ed-clm
git checkout -b i74-ed-snow-bugfix 56dfb030
cd cime/scripts
./create_test -testname ERS_D_Ld5.f10_f10.ICLM45ED.yellowstone_intel.clm-edTest -testid i74

go into into the test directory, compile, submit, check 'TestStatus' file.

bandre-ucar · 2016-06-15T19:57:24Z

Note from Sean Swenson:

I think that the ed restart value has to do with the fact that the ed_clm_link 
is called during the restart from EDRestVectorMod.  This seems to occur 
prior to the running of the biogeophysics.  In a normal timestep, the biogeophysics 
occurs prior to the ED calls.  But during a restart, the ed_clm_link routine is 
called, so calculates various things based on what's on the restart, not on 
what the value is after  biogeophysics.  Not sure what the proper fix is though.

This probably affects #14 and #43.

bandre-ucar · 2016-06-16T22:15:51Z

I ran Rosie's proposed change from i74-ed-snow-bugfix at revision 9cf97cf through the test suite.

This appears to have resolved almost all the restart problems in #74, #14, #43 . There is only a single remaining restart failure:

    FAIL ERS_D_Ld5.f09_g16.ICLM45ED.yellowstone_intel.clm-edTest.cpl.hi.nc : test compare cpl.hi (.base and .rest files) 
    FAIL ERS_D_Ld5.f09_g16.ICLM45ED.yellowstone_intel.clm-edTest : test functionality summary (ERS_test)

How do we want to proceed? Do @ckoven or @rgknox want to review it? Do we want to bring this into master as a separate PR first, then I can merge it into my branch?

rgknox · 2016-06-18T00:16:13Z

This is great news guys. Congrats on getting this stuff squared away.

I took a look through Rosie's changes, and the seem plenty large enough to merit their own PR.

rgknox · 2017-03-01T23:51:24Z

This looks closed, have all these problems been resolved?

bandre-ucar · 2017-03-02T04:22:57Z

@rgknox I think this has been resolved.

bandre-ucar mentioned this issue Jun 10, 2016

merge to clm > r17x-ish and stable cime tag #46

Closed

bandre-ucar added the bug label Jun 10, 2016

bandre-ucar mentioned this issue Jun 15, 2016

ed-clm fails bit for bit restart for f19 and f09 grids #14

Closed

bandre-ucar mentioned this issue Aug 3, 2016

ed r182 reintegration #90

Merged

bandre-ucar closed this as completed Mar 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

interaction with snow bugfix causes restart failures with ED #74

interaction with snow bugfix causes restart failures with ED #74

bandre-ucar commented Jun 10, 2016

rosiealice commented Jun 14, 2016

bandre-ucar commented Jun 14, 2016

rosiealice commented Jun 14, 2016

rosiealice commented Jun 14, 2016

bandre-ucar commented Jun 14, 2016 •

edited

Loading

bandre-ucar commented Jun 15, 2016

bandre-ucar commented Jun 16, 2016

rgknox commented Jun 18, 2016 •

edited

Loading

rgknox commented Mar 1, 2017

bandre-ucar commented Mar 2, 2017

interaction with snow bugfix causes restart failures with ED #74

interaction with snow bugfix causes restart failures with ED #74

Comments

bandre-ucar commented Jun 10, 2016

Summary of Issue:

Expected behavior and actual behavior:

Steps to reproduce the problem (should include create_newcase or create_test command along with any user_nl or xml changes):

What is the changeset ID of the code, and the machine you are using:

have you modified the code? If so, it must be committed and available for testing:

rosiealice commented Jun 14, 2016

bandre-ucar commented Jun 14, 2016

rosiealice commented Jun 14, 2016

rosiealice commented Jun 14, 2016

bandre-ucar commented Jun 14, 2016 • edited Loading

bandre-ucar commented Jun 15, 2016

bandre-ucar commented Jun 16, 2016

rgknox commented Jun 18, 2016 • edited Loading

rgknox commented Mar 1, 2017

bandre-ucar commented Mar 2, 2017

bandre-ucar commented Jun 14, 2016 •

edited

Loading

rgknox commented Jun 18, 2016 •

edited

Loading