d.time should return AbstractTimestep things

[davidanthoff]

I think that d.time inside run_timestep should return things that one can use to index into variables and parameters directly. Right now it seems to return Ints instead, which we should actively discourage for indexing use.

Ideally things like first(d.time) and last(d.time) would just work. And then we should try to get something like first(d.time):last(d.time) to work to get a range that includes every timestep.

[corakingdon]

This shouldn't be hard to implement, but I should spend time looking at the models to see what it will disrupt. I think this should also be changed for all dimensions too right? Such that d.region would return an array of the region labels, rather than Ints as well?

[davidanthoff]

The key thing is that whatever is returned from d.foo can be used to index into variables and parameters.

Right now we support indexing with Int into time dimensions, but I think only for backwards compat. We should actually remove that, IMO. But because it still supports indexing with Int, things work right now. If d.time returned some timestep type, things would of course still work, because we support indexing with those.

I'm less sure about other dimensions. I don't think we want to support indexing with the actual labels. Those could be anything, and that could be really slow. We might need something akin to the timestep type for other dimensions. So I think we should leave other dimensions alone for now :)

[corakingdon]

Ah right. Agreed we shouldn't do other dimensions right now.

Just to clarify, you want first(d.time) to return a timestep object that can be used as an index into the parameters and variables?

[davidanthoff]

Yep.

I'm unclear what d.time should actually be: a range, an array, an iterator? Unclear to me. But first(d.time) would work for any of these :)

[corakingdon]

I'll plan to work on this after we finish #551. After that is merged, we will have multiple types of Timestep things that can be used as indexes, so we'll have to decide which kind d.time should return

[corakingdon]

We decided that this should not be either of the new AbstractTimesteps (TimestepValue or TimestepIndex). This should return either a VariableTimestep or a FixedTimestep, whichever it is to match the t being passed to the run_timestep function. @davidanthoff, I think I will just implement this as an array, unless you choose otherwise! currently d.dimname returns vectors

[rjplevin]

Are the new indexers AbstractTimesteps? This seems odd to me, since they have to be resolved against an (old) AbstractTimestep. I think we need a new abstract type.

[corakingdon]

Oh I'm not sure, that implementation isn't finished yet. But maybe you should let Lisa know here: #551

[davidanthoff]

Shouldn't all four of these inherit from AbstractTimestep?

I think returning an array is fine for now. We can later still decide to change that to something more efficient.