Exact restart - classic driver

[yixinmao]

Fix inexact restart for classic driver:

• Currently the restart is inexact for vegetation with canopy, caused by a few variables being used before being assigned values.
• Have tested the following permutation of options to confirm the causes of inexact restart:
  • veg type: one grid cell of bare soil / evergreen needleleef
  • FULL_ENERGY = TRUE / FALSE
  • FROZEN_SOIL = TRUE / FALSE
  • CARBON = FALSE (have not tested for CARBON=TRUE yet)
  • LAKE=FALSE (have not tested for LAKE=TRUE yet)

Variables to fix:

• [PR Clean up and initial fix for exact restart - classic driver #481] energy->Tfoliage (this is a state variable that needs to be saved and read).
  This variable is calculated in "generate_default_states()" when no initial state is specified.
• energy->LongUnderOut (this is a flux variable whose old value from the last time step is used before being updated in the current time step)
  • This variable is calculated in "generate_default_states()" when no initial state is specified (so when there is no initial state, LongUnderOut would be calculated first; but when there is an initial state, LongUnderOut remains zero before the first time step calculation).
  • This variable is currently saved to state file to ensure exact restart [PR Fixed exact restart (temporarily) - classic and image drivers #507]. TODO: this is a temporary solution; a better way of handling this flux variable needs to be done (potential solutions: 1) modifying the calculation in vic_run so that the flux variable is always computed before used; 2) computing the flux variable from state variables before vic_run)
• energy->snow_flux (this is a flux variable which has the same problem, and is also temporarily saved to state file [PR Fixed exact restart (temporarily) - classic and image drivers #507])

Other changes related to exact restart:

• [PR Clean up and initial fix for exact restart - classic driver #481] Raise ascii state variable outputs to %.16g
• [PR Clean up and initial fix for exact restart - classic driver #481] Cleanly separate state and flux variables in vic_def.h and print_library_shared.c

Local variables used before initializing (these do not affect restart, but better to be fixed also):

• In surface_fluxes(), the local variable ShortUnderIn is used before initialized
• In func_surf_energy_bal(), line 662-663, "_fusion" is used; but "_fusion" is only initialized if FROZEN_SOIL=TRUE

[jhamman]

Thanks @yixinmao for the detailed update. Can you put together a PR with the cleanup changes listed above. We'll have to address the "Variables to fix" section separately.

@tbohn - any thoughts on the best way to fix the LongUnderOut and snow_flux problems listed above?

[yixinmao]

Thanks @jhamman for your comment. Will do, and I'll put in the read and write for energy->Tfoliage also if that sounds OK?

[jhamman]

Yes, that sounds good. Thanks.

[tbohn]

@jhamman I was discussing that with @yixinmao and was thinking that if we're just talking about a small number of fluxes, and they are easily computed from the known states, a quick fix could be to insert a computation of those fluxes into compute_derived_state_vars(). If their computation depends on other fluxes as well as states - e.g., requires running the whole energy balance computation - then perhaps it would be better to save them in the state file. The most labor intensive fix might be to re-order the various computations in surface_fluxes() so that there are no instances of variables being computed from the previous step's fluxes. Depending on how much work that is, it might be do-able for 5.0, or might end up being too much work and be more appropriate for 5.1. I guess we need to take a look and determine how much work is involved in computing these the "right" way, and then decide if we're willing to do that much work for 5.0. I can try to take a look at it over the weekend (i'm out of town right now, so only have a few short windows over the next 4 days in which I can do work).

[jhamman]

My initial thought here is that we shouldn't design around existing mistakes or bugs. In other words, I don't want to put fluxes in the state file. If there are variables being computed from the previous steps fluxes in surface_fluxes(), that sounds like a bug to me.

@yixinmao - can you point us to the lines in surface_fluxes where energy->LongUnderOut and energy->snow_flux are being calculated from fluxes from previous timesteps?

[tbohn]

@jhamman doesn't @yixinmao 's addition of Tfoliage to the state file fix the problems with LongUnderOut and snow_flux? I think that was the ultimate culprit. So my earlier comment is now out of date.

[yixinmao]

@tbohn Tfoliage is a separate issue from the two fluxes - Tfoliage is a prognostic state that needs to be read and written (which is already taken care of); the two fluxes are used for computing other variables in vic_run before their values get updated.

[yixinmao]

@jhamman - energy->LongUnderOut seems to be calculated in line 634 (develop branch) in calc_surf_energy_bal.c - its calculation seems to need other fluxes variables, which tracks back several levels of functions...

[yixinmao]

@jhamman - Whoops sorry hit send too soon...
energy->snow_flux seems to be calculated in the section around line 378 in func_surf_energy_bal.c - its calculation might only depend on some state variables it seems, so may be possible to derive it in compute_derived_state_vars. But I haven't looked further into it.

[jhamman]

As I'm thinking about it, the longwave fluxes should be fairly easy to compute from states at any point. We know how these terms are calculated - longwave = f(rad_temp) where rad_temp is a state variable that we are certainly saving. The challenge is going to be figuring out what the uninitialized field is made up of. Are we missing the downwelling longwave below the canopy? If so, longwave = f(canopy_rad_temp) + f(atmos_lw_down).

The snow_flux calculation looks fine to me and I'm not seeing where its using a flux from a previous timestep.

[yixinmao]

@jhamman Right the calculation of snow_flux is not using fluxes from the previous step, but the variable snow_flux gets used somewhere before in vic_run, which may affect the calculation of some other variables

[bartnijssen]

For now my recommendation is to simply store the small number of fluxes to which this applies as part of the state file and keep this issue open (or reopen a similar one) for 5.1. I think that to ensure exact restarts we need to make sure that the initial guess for these fluxes is the same no matter whether we are continuing a model run or restarting. The easiest fix for that is to include these fluxes in the state file, not as a long-term solution, but to keep moving on VIC 5.0.

[jhamman]

@yixinmao - can this be closed following #509?

[yixinmao]

@jhamman We probably want to leave the temporary fix for the two flux variables documented (in this issue or open another issue for it); also the last point - two uninitialized local variables - has not been fixed yet

[bartnijssen]

@yixinmao: Where are we on this? It's still open even though the milestone has passed. If you want to keep it open, please move the milestone, because this will get lost otherwise.

[yixinmao]

@bartnijssen Most of the things on the list here are done, except:

• We are saving fluxes energy->LongUnderOut and energy->snow_flux to state file for exact restart - this is not ideal but will not be fixed in 5.0.
• The two local variables used before initializing (these do not affect results)

Maybe we could close this issue and open another issue for the above remaining things?

[jhamman]

Yes, @yixinmao - please open a new issue and reference/close this one.

[yixinmao]

Most things in this issues are resolved. The remaining things are summarized in a new issue #580 .