PowerSystems.jl icon indicating copy to clipboard operation
PowerSystems.jl copied to clipboard

Weird behavior of `get_time_series_values`

Open raphaelsaavedra opened this issue 4 years ago • 6 comments

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).

raphaelsaavedra avatar Jun 03 '21 16:06 raphaelsaavedra

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.

daniel-thom avatar Jun 03 '21 17:06 daniel-thom

Thanks for the clarification!

raphaelsaavedra avatar Jun 03 '21 17:06 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

raphaelsaavedra avatar Jun 03 '21 17:06 raphaelsaavedra

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.

daniel-thom avatar Jun 03 '21 17:06 daniel-thom

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.

claytonpbarrows avatar Jun 08 '21 02:06 claytonpbarrows

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

jd-lara avatar Jun 09 '21 00:06 jd-lara