Rework CNPhenology onset and offset triggers to be more robust to changes in dt

[billsacks]

1165 improves the robustness of the CNPhenology onset and offset triggers when there are changes in time step length (dt) between the restart file and the current run. However, I don't think it goes far enough to ensure that all of the phenology code will work correctly under this condition.

I haven't come to completely understand the relevant code, but my understanding is that, for both onset and offset, we have:

1. Many time steps during the onset / offset period when we are making gradual transfers from some pools to other pools.

2. A second-to-last time step when there is some special code to ensure that all of the final transfers happen.

3. A last time step that finalizes the period.

The key point, from what I can tell from my brief read of the code, is that it seems like (2) is designed to happen exactly once. However, I think that neither the code before or after #1165 ensures that (2) actually happens exactly once if we allow for changes in the time step length between the restart file and the current run.

Specific scenarios I can think of where this is violated are:

• If a counter on the restart file was exactly half-way between two time steps according to the new time step, then the checks if (abs(onset_counter(p) - dt) <= dt/2._r8) will trigger twice. For example, if the restart file was generated with a dt of 900 sec, the counter on the restart file was 4500, and then we use it in a run with a 1800 sec time step, then this will trigger at two times: onset_counter = 2700 and onset_counter = 900.

• If going from a long time step to a short time step: if the restart file is written when we have just finished the time when a counter was equal to dt, so we just did (2), then we switch to a significantly shorter time step: we'll get a few time steps back in (1), then another instance of (2).

• If going from a short time step to a long time step: if the restart file is written when we are still multiple short time steps away from 0, but less than 1/2 time step away from 0 in the long time step, we'll never trigger (2).

• I'm also concerned about whether crop grain and biofuel fluxes are set correctly (see the comment in CNOffsetLitterfall, "this assumes that offset_counter == dt for crops"), but I haven't thought about this carefully.

The solution I can see is to treat this more explicitly like a state machine, and add another state here. Currently, we have two values for the onset and offset flags: 0.0 or 1.0. We could add a third value (2.0) that indicates that we are in the final time step. Then we would change the logic as follows:

• Change the countdown counters to be counting down to the second to last time step rather than counting down to the last time step.

  • If we want to maintain answers the same as before, I think this could be done by: (a) initializing these counters as dtime less than they currently are initialized... I think this will be especially important to do for crop, which seems to depend on some exactness here, as per the comment, 'this assumes that offset_counter == dt for crops'; (b) changing the restart variable name; (c) if the new restart variable name isn't on the file, then using the old one, but subtracting dtime from it, along with some special treatment if we were in the last time step of the period (adjusting the onset and offset flags in this case). (We should use the issue_fixed_metadata for this purpose: this is needed for this backwards compatibility logic to work correctly with init_interp.)

• The current checks for near-equality to dt would be changed to check if the counter is < dt/2

• When we find ourselves in the second to last time step (based on counter < dt/2), then change the relevant flag to the new value (2.0).

• Then the current conditionals checking if the counter is < dt/2 should be changed to instead check whether the flag is set to this new value: if so, we're in the last time step. (As long as those checks aren't triggered until the next time around the driver loop - not the same time step where we set the flag to 2.)

[billsacks]

Although I think this will do the wrong thing in some scenarios when changing the time step, my sense is that we have bigger fish to fry, so I'm closing this as a wontfix; we can reopen it if it seems to actually be causing problems in practice.