Skip to content

Commit

Permalink
Merge pull request #857 from mimiframework/timestepvalue-offset-fix
Browse files Browse the repository at this point in the history 
Fix a TimestepValue offset indexing bug
  • Loading branch information
@lrennels
lrennels authored Oct 20, 2021
2 parents b74d386 + 0544445 commit f3382b5
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 16 deletions.
32 changes: 16 additions & 16 deletions src/core/time_arrays.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,15 @@ end

function Base.getindex(mat::TimestepMatrix{FixedTimestep{FIRST, STEP, LAST}, T_data, 1}, ts::TimestepValue{T_time}, idx::AnyIndex) where {T_data, FIRST, STEP, LAST, T_time}
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] + 1 : view_offset = 0
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] - 1 : view_offset = 0
t = t - view_offset
data = mat.data[t, idx]
_missing_data_check(data, t)
end

function Base.getindex(mat::TimestepMatrix{VariableTimestep{TIMES}, T_data, 1}, ts::TimestepValue{T_time}, idx::AnyIndex) where {T_data, TIMES, T_time}
t = _get_time_value_position(TIMES, ts)
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] + 1 : view_offset = 0
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] - 1 : view_offset = 0
t = t - view_offset
data = mat.data[t, idx]
_missing_data_check(data, t)
Expand All @@ -267,15 +267,15 @@ end

function Base.getindex(mat::TimestepMatrix{FixedTimestep{FIRST, STEP, LAST}, T_data, 2}, idx::AnyIndex, ts::TimestepValue{T_time}) where {T_data, FIRST, STEP, LAST, T_time}
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] + 1 : view_offset = 0
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] - 1 : view_offset = 0
t = t - view_offset
data = mat.data[idx, t]
_missing_data_check(data, t)
end

function Base.getindex(mat::TimestepMatrix{VariableTimestep{TIMES}, T_data, 2}, idx::AnyIndex, ts::TimestepValue{T_time}) where {T_data, TIMES, T_time}
t = _get_time_value_position(TIMES, ts)
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] + 1 : view_offset = 0
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] - 1 : view_offset = 0
t = t - view_offset
data = mat.data[idx, t]
_missing_data_check(data, t)
Expand Down Expand Up @@ -306,28 +306,28 @@ end

function Base.setindex!(mat::TimestepMatrix{FixedTimestep{FIRST, STEP, LAST}, T_data, 1}, val, ts::TimestepValue{T_time}, idx::AnyIndex) where {T_data, FIRST, STEP, LAST, T_time}
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] + 1 : view_offset = 0
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] - 1 : view_offset = 0
t = t - view_offset
setindex!(mat.data, val, t, idx)
end

function Base.setindex!(mat::TimestepMatrix{VariableTimestep{TIMES}, T_data, 1}, val, ts::TimestepValue{T_time}, idx::AnyIndex) where {T_data, TIMES, T_time}
t = _get_time_value_position(TIMES, ts)
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] + 1 : view_offset = 0
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] - 1 : view_offset = 0
t = t - view_offset
setindex!(mat.data, val, t, idx)
end

function Base.setindex!(mat::TimestepMatrix{FixedTimestep{FIRST, STEP, LAST}, T_data, 2}, val, idx::AnyIndex, ts::TimestepValue{T_time}) where {T_data, FIRST, STEP, LAST, T_time}
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] + 1 : view_offset = 0
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] - 1 : view_offset = 0
t = t - view_offset
setindex!(mat.data, val, idx, t)
end

function Base.setindex!(mat::TimestepMatrix{VariableTimestep{TIMES}, T_data, 2}, val, idx::AnyIndex, ts::TimestepValue{T_time}) where {T_data, TIMES, T_time}
t = _get_time_value_position(TIMES, ts)
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] + 1 : view_offset = 0
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] - 1 : view_offset = 0
t = t - view_offset
setindex!(mat.data, val, idx, t)
end
Expand Down Expand Up @@ -397,7 +397,7 @@ function Base.getindex(arr::TimestepArray{FixedTimestep{FIRST, STEP, LAST}, T_da
_single_index_check(arr.data, idxs)
idxs1, ts, idxs2 = split_indices(idxs, ti)
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
t = t - view_offset
return arr.data[idxs1..., t, idxs2...]
end
Expand All @@ -406,7 +406,7 @@ function Base.getindex(arr::TimestepArray{VariableTimestep{TIMES}, T_data, N, ti
_single_index_check(arr.data, idxs)
idxs1, ts, idxs2 = split_indices(idxs, ti)
t = _get_time_value_position(TIMES, ts)
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
t = t - view_offset
return arr.data[idxs1..., t, idxs2...]
end
Expand Down Expand Up @@ -441,7 +441,7 @@ function Base.setindex!(arr::TimestepArray{FixedTimestep{FIRST, STEP, LAST}, T_d
_single_index_check(arr.data, idxs)
idxs1, ts, idxs2 = split_indices(idxs, ti)
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
t = t - view_offset
setindex!(arr.data, val, idxs1..., t, idxs2...)
end
Expand All @@ -450,7 +450,7 @@ function Base.setindex!(arr::TimestepArray{VariableTimestep{TIMES}, T_data, N, t
_single_index_check(arr.data, idxs)
idxs1, ts, idxs2 = split_indices(idxs, ti)
t = _get_time_value_position(TIMES, ts)
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
t = t - view_offset
setindex!(arr.data, val, idxs1..., t, idxs2...)
end
Expand All @@ -475,15 +475,15 @@ end
function Base.getindex(arr::TimestepArray{FixedTimestep{FIRST, STEP, LAST}, T_data, N, ti}, idxs::Union{Array{TimestepValue{T_time},1}, AnyIndex}...) where {T_data, N, ti, FIRST, STEP, LAST, T_time}
idxs1, ts_array, idxs2 = split_indices(idxs, ti)
ts_idxs = _get_ts_indices(ts_array, [FIRST:STEP:LAST...])
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
ts_idxs = ts_idxs .- view_offset
return arr.data[idxs1..., ts_idxs, idxs2...]
end

function Base.getindex(arr::TimestepArray{VariableTimestep{TIMES}, T_data, N, ti}, idxs::Union{Array{TimestepValue{T_time},1}, AnyIndex}...) where {T_data, N, ti, TIMES, T_time}
idxs1, ts_array, idxs2 = split_indices(idxs, ti)
ts_idxs = _get_ts_indices(ts_array, TIMES)
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
ts_idxs = ts_idxs .- view_offset
return arr.data[idxs1..., ts_idxs, idxs2...]
end
Expand All @@ -497,15 +497,15 @@ end
function Base.setindex!(arr::TimestepArray{FixedTimestep{FIRST, STEP, LAST}, T_data, N, ti}, vals, idxs::Union{Array{TimestepValue{T_time},1}, AnyIndex}...) where {T_data, N, ti, FIRST, STEP, LAST, T_time}
idxs1, ts_array, idxs2 = split_indices(idxs, ti)
ts_idxs = _get_ts_indices(ts_array, [FIRST:STEP:LAST...])
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
ts_idxs = ts_idxs .- view_offset
setindex!(arr.data, vals, idxs1..., ts_idxs, idxs2...)
end

function Base.setindex!(arr::TimestepArray{VariableTimestep{TIMES}, T_data, N, ti}, vals, idxs::Union{Array{TimestepValue{T_time},1}, AnyIndex}...) where {T_data, N, ti, TIMES, T_time}
idxs1, ts_array, idxs2 = split_indices(idxs, ti)
ts_idxs = _get_ts_indices(ts_array, TIMES)
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
ts_idxs = ts_idxs .- view_offset
setindex!(arr.data, vals, idxs1..., ts_idxs, idxs2...)
end
Expand Down
31 changes: 31 additions & 0 deletions test/test_timesteparrays.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -745,3 +745,34 @@ reset_time_val(x, zeros(10))
reset_time_val(y, collect(reshape(zeros(8), 4, 2)))

end #module

#------------------------------------------------------------------------------
# 8. Test handling of offsets for TimestepValue and TimestepIndex with a TimestepMatrix
# --> this is a very specific test to handle PR #857, specifically for methods
# using offset - 1 in time.jl
#------------------------------------------------------------------------------

@defcomp testcomp begin

var_tvalue = Variable(index=[time, regions])
var_tindex = Variable(index=[time, regions])

function run_timestep(p, v, d, t)
for r in d.regions
tvalue = TimestepValue(2003)
tindex = TimestepIndex(1)

v.var_tvalue[tvalue,r] = 999
v.var_tindex[tindex,r] = 999
end
end
end

m = Model()
set_dimension!(m, :time, 2000:2005)
set_dimension!(m, :regions, ["A", "B"])
add_comp!(m, testcomp, first = 2003)
run(m)

@test m[:testcomp, :var_tvalue][findfirst(i -> i == 2003, 2000:2005), :] == [999., 999.]
@test m[:testcomp, :var_tindex][findfirst(i -> i == 2003, 2000:2005), :] == [999., 999.]

0 comments on commit f3382b5

Please sign in to comment.