Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checking the replacement value for variables #1016

Closed
Mastomaki opened this issue May 21, 2024 · 6 comments
Closed

Checking the replacement value for variables #1016

Mastomaki opened this issue May 21, 2024 · 6 comments
Labels
Type: improvement Improve something that already exists Zone: data & import Stuff going in and how it's processed

Comments

@Mastomaki
Copy link

Description of the issue

I'm thinking of the following function, for example considering the variable units_out_of_service. It seems that if the replacement_value is a time series, which has been defined only for some time indices, then a NaN will be returned for other indices and the model will crash.

function _add_variable!(m, name, ind, replacement_value)
    if replacement_value !== nothing
        ind_ = (analysis_time=_analysis_time(m), ind...)
        value = replacement_value(ind_)
        if value !== nothing
            return value
        end
    end
    @variable(m, base_name=_base_name(name, ind))
end

Describe the solution you'd like
In the case the replacement_value has not been defined for some time index, default value for the variable could be used.

@clizbe
Copy link
Collaborator

clizbe commented May 28, 2024

I agree this sounds like a desirable behavior. Until we're able to add it I think you'll have to work with filling the default value into your timeseries. Would that work?

@clizbe clizbe added Type: improvement Improve something that already exists Zone: data & import Stuff going in and how it's processed labels May 28, 2024
@Mastomaki
Copy link
Author

Yes. But actually there is the checking:

function Base.getindex(x::Union{TimeSeries,Map}, key)
    i = _searchsortedfirst(x.indexes, key)
    if get(x.indexes, i, nothing) == key
        x.values[i]
    else
        throw(BoundsError(x, key))
    end
end

For some reason this was not activated.

@Mastomaki
Copy link
Author

Or then it was this function, which is the culprit:

function _get_time_series_value(pv, t::TimeSlice, upd)
    adjusted_t = pv.value.ignore_year ? t - Year(start(t)) : t
    t_start, t_end = start(adjusted_t), end_(adjusted_t)
    a, b = _search_overlap(pv.value, t_start, t_end)
    if upd !== nothing
        timeout = _timeout(pv.value, t_start, t_end, a, b)
        _add_update(t, timeout, upd)
    end
    t_end <= pv.value.indexes[1] && return NaN
    t_start > pv.value.indexes[end] && !pv.value.ignore_year && return NaN
    mean(Iterators.filter(!isnan, pv.value.values[a:b]))
end

So at least add Boundserror there too.

@clizbe
Copy link
Collaborator

clizbe commented May 30, 2024

Can you create a pull request for this update?

@Mastomaki
Copy link
Author

I created a pull request in the SpineInterface repository.

@clizbe
Copy link
Collaborator

clizbe commented Jul 15, 2024

Great! Closing it here then.

@clizbe clizbe closed this as completed Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: improvement Improve something that already exists Zone: data & import Stuff going in and how it's processed
Projects
None yet
Development

No branches or pull requests

2 participants