Weird behavior of get_time_series_values

[raphaelsaavedra]

If we have two components load1 and load2, and both have a time series called "load".

The following will return the values of time series in load1:

load_ts = get_time_series(SingleTimeSeries, load1, "load")
get_time_series_values(load2, load_ts)

I imagine this should error, since load_ts is not in load2, but in load1.
Also, wouldn't it be convenient to have a method get_time_series_values that receives just the time series, without the component? If the goal is to just return the values of the specified time series, then the component shouldn't be relevant (we already specified the component when we got the time series).

[daniel-thom]

The component is required because the user may be using the scaling_factor_multiplier concept. In that case the time series instance stores a reference to a component method that will be applied when the user calls get_time_series_values.

"""
Return an Array of values from a cached StaticTimeSeries instance for the requested time
series parameters.
"""
function get_time_series_values(
    component::InfrastructureSystemsComponent,
    time_series::StaticTimeSeries,
    start_time::Union{Nothing, Dates.DateTime} = nothing;
    len::Union{Nothing, Int} = nothing,
    ignore_scaling_factors = false,
)

If you already have the time series instance and aren't using scaling_factor_multiplier then you could extract the data with get_data, but then you lose the start_time/len interfaces.

We could consider adding

function get_time_series_values(
    time_series::StaticTimeSeries,
    start_time::Dates.DateTime;
    len::Union{Nothing, Int} = nothing,
)

Secondly, we should add a check for the case you mentioned to ensure that the component actually contains the passed time_series instance.

[raphaelsaavedra]

Thanks for the clarification!

[raphaelsaavedra]

On the get_data topic: is there a single function that does that? get_data returns the TimeArray, so fetching the values requires values(get_data(load_ts)) – which is completely fine, was just wondering if there's a simpler way

[daniel-thom]

No, that's what you would have to do. It's even more cumbersome for subtypes of Forecast because the data is actually stored in a SortedDict. The user would have to pass in the key (start time of each window).

The change I mentioned above would be the simplification we would make.

[claytonpbarrows]

Also, wouldn't it be convenient to have a method get_time_series_values that receives just the time series, without the component?

Yes, that would be convenient. However, I think it could also cause some confusion. Sinceget_time_series_values typically applies the scaling_factor_multiplier to transform the data it returns, it requires the component as well as the time_series argument. If we added the method with only the time_series, I think it could create confusion if an unknowing user receives the transformed data with one method and the transformed data with the other method.
I think we should stick with the extra required step to extract the time series to avoid this confusion.

[jd-lara]

I think we should document this behavior better to avoid annoyances from the users.

[kdayday]

Secondly, we should add a check for the case you mentioned to ensure that the component actually contains the passed time_series instance.

@daniel-thom @jd-lara Think the best way to resolve this is with this check. Has it been implemented? We typically haven't taken approach of documenting how not to use a function, and otherwise, I think the docstring improvements for these functions in IS has resolved this issue, so we could close this.

[jd-lara]

Secondly, we should add a check for the case you mentioned to ensure that the component actually contains the passed time_series instance.

@daniel-thom @jd-lara Think the best way to resolve this is with this check. Has it been implemented? We typically haven't taken approach of documenting how not to use a function, and otherwise, I think the docstring improvements for these functions in IS has resolved this issue, so we could close this.

agree we should close. It is also stale for 4 years almost.