Time dims

[lrennels]

This PR should handle issues #573 and #422 with the main goal of reenabling the first and last keyword arguments on add_comp! so that we can use components of different time lengths.

[lrennels]

From conversation 1/28 with David:

• For now, we will enforce that when you set_param! or update_param! the dimensions of the data must exactly match the dimensions of the model at the time. Note that if a dimension has been updated with set_dimension!, this may mean that the new parameter values have a different size than the old ones. Also note that this means that parameters for components with shorter time dims than the model will needed to be padded with default values or missings. We will try to relax this later.
• the update_timesteps keyword argument will now default to true, and setting it to false allows skipping the steps of creating a new TimestepArray with new labels if the user/internal call is sure the time dimension has not changed ... TODO: we may also be able to internally check if it has changed, but it is unclear if that check will really improve speed much ...

^ can simply check if time_labels(oldparams.values) == m.md.dim_dict[:time]

[codecov]

Codecov Report

Merging #793 (cbe3f35) into master (b830667) will increase coverage by 1.52%.
The diff coverage is 96.24%.

@@            Coverage Diff             @@
##           master     #793      +/-   ##
==========================================
+ Coverage   78.90%   80.43%   +1.52%     
==========================================
  Files          37       37              
  Lines        3077     3051      -26     
==========================================
+ Hits         2428     2454      +26     
+ Misses        649      597      -52     

Flag	Coverage Δ
unittests	80.43% <96.24%> (+1.52%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/core/model.jl	84.21% <ø> (ø)
src/core/time_arrays.jl	90.38% <93.54%> (+11.20%)	⬆️
src/core/defs.jl	84.57% <95.23%> (+1.61%)	⬆️
src/components/connector.jl	100.00% <100.00%> (+40.00%)	⬆️
src/core/build.jl	100.00% <100.00%> (ø)
src/core/connections.jl	86.87% <100.00%> (+1.47%)	⬆️
src/core/dimensions.jl	84.21% <100.00%> (+1.19%)	⬆️
src/core/instances.jl	77.86% <100.00%> (+1.82%)	⬆️
src/core/types/defs.jl	78.94% <100.00%> (+0.28%)	⬆️
src/core/types/time.jl	100.00% <100.00%> (ø)
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b830667...cbe3f35. Read the comment docs.

[lrennels]

The general story right now is:

• All provided data (with set_param! or update_param! must match the full dimensions of the model. You can pad with missings if you want, but the dimensions have to match even for :time. This is a change, but it really just clarifies previously buggy behavior. We will try to loosen this later!
• Each component def, noting that a ModelDef is a type of component def, holds both a :time dimension and a first and last. The :time dimension of all components at the time of a model running are the same, specific operations like set_dimension! and build! propagate the top-level time dimension keys through to ensure that fact. If first and last are not set explicitly, they will be at the bounds of the model :time dimension and change with the model if the dimension is reset with set_dimension!. If they are set explicitly, they are fixed and immutable, and apply to all subcomponents of the component on which they were fixed during add_comp!.
• The model can start before any component, and end after, you’ll just get missings in your output arrays. A note here is that if you used something like is_first(t) in your run_timestep to set an initial value… you might run into problems because that refers to the model time, not the compoent time. That can be fixed by using something like if t == TimestepValue(2020) which I tried out.
• Component run periods must fall within the model time dimension.

[lrennels]

Update: going to alter so that is_first refers to the first tilmestep a component instance is run, not always the model tilmestep

[lrennels]

From experimenting with coupling FAIR and FUND, I've realized an inconsistency with TimestepIndex. While comparison operators like if t == TimestepIndex(5) works correctly because it compares ts.t with 5 ... indexing into a TimstepArray with TimestepIndex(5) isn't going to work because there simply isn't the information we need to offset the 5 by the component since our storage story has the data the length of time.

... idea for a fix .. to discuss on Slack

"""
    AbsoluteTimestepIndex

 An internal type used to index into a `TimestepArray` in `run_timestep` functions,
 containing an Int `index` that indicates the position in the array in terms of timesteps
 which ignores relative run times between components and models and is always
 relative to the allocated data array.
 """
struct AbsoluteTimestepIndex
    index::Int
end

"""
     TimestepIndex

 A user-facing type used to index into a `TimestepArray` in `run_timestep` functions,
 containing an Int `index` that indicates the position in the array in terms of timesteps
 and a timestep `t` from the `run_timestep` function.
 """
struct TimestepIndex
    t::Union{FixedTimestep, VariableTimestep}
    index::Int

    function TimestepIndex(t::Union{FixedTimestep, VariableTimestep}, index::Int)
        return new(t, index)
    end

    function TimestepIndex(index::Int)
        @warn("Using TimestepIndex with a single argument index is deprecated since it causes unexpected behavior when a model uses components with differing run time horizons. Please replace a call to TimestepIndex($index) to TimestepIndex(t, $index) where t is the Timestep argument in run_timestep(p, v, d, t).")
        return AbsoluteTimestepIndex(index)
    end
end