some CLM history fields don't restart properly

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2014-11-30 20:12:04 -0700
Bugzilla Id: 2093
Bugzilla CC: andre@ucar.edu, erik@ucar.edu, muszala@ucar.edu, rfisher@ucar.edu,

Mariana found that some CLM history fields don't restart properly. This was documented in bug 2091, but I'm moving this here, because it's really a separate (and less critical) problem from 2091.

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2014-11-30 20:12:40 -0700

From Mariana (from bug 2091):

There are still two problems that I have to track down with the restarts. They are that NFIRE is not bfb on restart and that EFLX_GRND_LAKE has a fill pattern difference on restart. I am wondering how long these two bugs have been there - since neither of these fields appear in when the suffix
"clm-irrigOn_reduceOutput"
is added to the test suite. To discover this I actually ran the test
ERS_Ly5.f10_f10.ICLM45BGCCROP.yellowstone_intel
where a lot more fields are output to monthly clm history files. The above two fields were the only ones that were a problem on restart. I am wondering if we should move to this test in the future rather than
ERS_Ly5.f10_f10.ICLM45BGCCROP.yellowstone_intel.clm-irrigOn_reducedOutput

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2014-11-30 20:13:13 -0700

From Mariana (from bug 2091):

Committed

refactor_koven_bugfix_n01_clm4_5_1_r098 [I think this should say: refactor_koven_bugfix_n02_clm4_5_1_r098]

which fixed the restart problem for history variable NFIRE. Basically, it was to remove the following if clause in CNVegStateType.F90
old: if (nsrest == nsrStartup) this%nfire_col(c) = 0._r8
new: this%nfire_col(c) = 0._r8

Now only history variable EFLX_GRND_LAKE has a problem on restart.

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2014-11-30 20:13:34 -0700

From Mariana (from bug 2091):

I just confirmed that NFIRE was not bit-for-bit and EFLX_GRND_LAKE had fill pattern differences in clm4_5_1_r096 for the test: ERS_Ly5.f10_f10.ICLM45BGCCROP.yellowstone_intel
So the restart problems with those variables are outstanding issues not related to the clm4_5_1_r097 tag. As it turns out, I have fixed the NFIRE problem on the branch listed here - but the EFLX_GRND_LAKE problem is still outstanding.

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2014-11-30 20:16:04 -0700

As to Mariana's suggestion of changing the ERS_Ly5 test to include more history fields: I think this is a good idea. I'd suggest that, once the ERS test and others are set up to compare component history files as well as cpl hist files, we should change most or all of our reducedOutput tests to instead inherit from the "monthly" testmod, or something similar to it, so that they keep all of the default history fields, just with monthly rather than daily output.

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2014-12-01 05:26:10 -0700

One more comment that I forgot to include from bug 2091, from me:

At a glance, I'm not convinced that your fix for nfire is entirely correct. From a skim through CNFireMod, it looks like what's going on here is that nfire is only set in certain conditions, and when those conditions aren't met, it stays at its value from the previous time step. My intuition is that the correct thing to do is to either:

(1) Set nfire in every time step - setting it to 0 in conditions where it is currently not being touched.

(2) Keep the current logic (so nfire is only touched in certain conditions), and adding it as a restart field.

I don't understand the fire code well enough to know which of those is correct.

[billsacks]

Bill Sacks < sacks@ucar.edu > - 2016-07-07 11:35:59 -0600

To summarize the outstanding problems:

(1) EFLX_GRND_LAKE has fill pattern differences upon restart

(2) NFIRE had different values upon restart; this was fixed, but I think in the wrong way

[billsacks]

From clm-cmt discussion: it looks like the cases where nfire isn't set are ones based on landcover (trotr1_col(c)+trotr2_col(c)>0.6_r8). So the problem of nfire being set in one time step and then accidentally persisting at the old value into the next timestep should only occur in a transient landcover case. However, this also argues for going with my first proposed solution for nfire:

Set nfire in every time step - setting it to 0 in conditions where it is currently not being touched.

I think we should do this - i.e., ensuring nfire is set every time step. This may change answers for just the nfire diagnostic field, but not for anything else (and may not change answers even for nfire). I'm adding this to the simple bfb tag with this caveat.

(We haven't looked at EFLX_GRND_LAKE: we should look at that.)