Skip to content

Commit f3382b5

Browse files
lrennelslrennels
authored
Merge pull request #857 from mimiframework/timestepvalue-offset-fix
Fix a TimestepValue offset indexing bug
2 parents b74d386 + 0544445 commit f3382b5

File tree

2 files changed

+47
-16
lines changed

2 files changed

+47
-16
lines changed

‎src/core/time_arrays.jl‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -244,15 +244,15 @@ end
244244

245245
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}
246246
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
247-
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] + 1 : view_offset = 0
247+
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] - 1 : view_offset = 0
248248
t = t - view_offset
249249
data = mat.data[t, idx]
250250
_missing_data_check(data, t)
251251
end
252252

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

268268
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}
269269
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
270-
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] + 1 : view_offset = 0
270+
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] - 1 : view_offset = 0
271271
t = t - view_offset
272272
data = mat.data[idx, t]
273273
_missing_data_check(data, t)
274274
end
275275

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

307307
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}
308308
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
309-
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] + 1 : view_offset = 0
309+
mat.data isa SubArray ? view_offset = mat.data.indices[1][1] - 1 : view_offset = 0
310310
t = t - view_offset
311311
setindex!(mat.data, val, t, idx)
312312
end
313313

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

321321
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}
322322
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
323-
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] + 1 : view_offset = 0
323+
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] - 1 : view_offset = 0
324324
t = t - view_offset
325325
setindex!(mat.data, val, idx, t)
326326
end
327327

328328
function Base.setindex!(mat::TimestepMatrix{VariableTimestep{TIMES}, T_data, 2}, val, idx::AnyIndex, ts::TimestepValue{T_time}) where {T_data, TIMES, T_time}
329329
t = _get_time_value_position(TIMES, ts)
330-
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] + 1 : view_offset = 0
330+
mat.data isa SubArray ? view_offset = mat.data.indices[2][1] - 1 : view_offset = 0
331331
t = t - view_offset
332332
setindex!(mat.data, val, idx, t)
333333
end
@@ -397,7 +397,7 @@ function Base.getindex(arr::TimestepArray{FixedTimestep{FIRST, STEP, LAST}, T_da
397397
_single_index_check(arr.data, idxs)
398398
idxs1, ts, idxs2 = split_indices(idxs, ti)
399399
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
400-
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
400+
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
401401
t = t - view_offset
402402
return arr.data[idxs1..., t, idxs2...]
403403
end
@@ -406,7 +406,7 @@ function Base.getindex(arr::TimestepArray{VariableTimestep{TIMES}, T_data, N, ti
406406
_single_index_check(arr.data, idxs)
407407
idxs1, ts, idxs2 = split_indices(idxs, ti)
408408
t = _get_time_value_position(TIMES, ts)
409-
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
409+
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
410410
t = t - view_offset
411411
return arr.data[idxs1..., t, idxs2...]
412412
end
@@ -441,7 +441,7 @@ function Base.setindex!(arr::TimestepArray{FixedTimestep{FIRST, STEP, LAST}, T_d
441441
_single_index_check(arr.data, idxs)
442442
idxs1, ts, idxs2 = split_indices(idxs, ti)
443443
t = _get_time_value_position([FIRST:STEP:LAST...], ts)
444-
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
444+
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
445445
t = t - view_offset
446446
setindex!(arr.data, val, idxs1..., t, idxs2...)
447447
end
@@ -450,7 +450,7 @@ function Base.setindex!(arr::TimestepArray{VariableTimestep{TIMES}, T_data, N, t
450450
_single_index_check(arr.data, idxs)
451451
idxs1, ts, idxs2 = split_indices(idxs, ti)
452452
t = _get_time_value_position(TIMES, ts)
453-
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
453+
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
454454
t = t - view_offset
455455
setindex!(arr.data, val, idxs1..., t, idxs2...)
456456
end
@@ -475,15 +475,15 @@ end
475475
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}
476476
idxs1, ts_array, idxs2 = split_indices(idxs, ti)
477477
ts_idxs = _get_ts_indices(ts_array, [FIRST:STEP:LAST...])
478-
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
478+
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
479479
ts_idxs = ts_idxs .- view_offset
480480
return arr.data[idxs1..., ts_idxs, idxs2...]
481481
end
482482

483483
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}
484484
idxs1, ts_array, idxs2 = split_indices(idxs, ti)
485485
ts_idxs = _get_ts_indices(ts_array, TIMES)
486-
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
486+
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
487487
ts_idxs = ts_idxs .- view_offset
488488
return arr.data[idxs1..., ts_idxs, idxs2...]
489489
end
@@ -497,15 +497,15 @@ end
497497
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}
498498
idxs1, ts_array, idxs2 = split_indices(idxs, ti)
499499
ts_idxs = _get_ts_indices(ts_array, [FIRST:STEP:LAST...])
500-
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
500+
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
501501
ts_idxs = ts_idxs .- view_offset
502502
setindex!(arr.data, vals, idxs1..., ts_idxs, idxs2...)
503503
end
504504

505505
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}
506506
idxs1, ts_array, idxs2 = split_indices(idxs, ti)
507507
ts_idxs = _get_ts_indices(ts_array, TIMES)
508-
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] + 1 : view_offset = 0
508+
arr.data isa SubArray ? view_offset = arr.data.indices[ti][1] - 1 : view_offset = 0
509509
ts_idxs = ts_idxs .- view_offset
510510
setindex!(arr.data, vals, idxs1..., ts_idxs, idxs2...)
511511
end

‎test/test_timesteparrays.jl‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,3 +745,34 @@ reset_time_val(x, zeros(10))
745745
reset_time_val(y, collect(reshape(zeros(8), 4, 2)))
746746

747747
end #module
748+
749+
#------------------------------------------------------------------------------
750+
# 8. Test handling of offsets for TimestepValue and TimestepIndex with a TimestepMatrix
751+
# --> this is a very specific test to handle PR #857, specifically for methods
752+
# using offset - 1 in time.jl
753+
#------------------------------------------------------------------------------
754+
755+
@defcomp testcomp begin
756+
757+
var_tvalue = Variable(index=[time, regions])
758+
var_tindex = Variable(index=[time, regions])
759+
760+
function run_timestep(p, v, d, t)
761+
for r in d.regions
762+
tvalue = TimestepValue(2003)
763+
tindex = TimestepIndex(1)
764+
765+
v.var_tvalue[tvalue,r] = 999
766+
v.var_tindex[tindex,r] = 999
767+
end
768+
end
769+
end
770+
771+
m = Model()
772+
set_dimension!(m, :time, 2000:2005)
773+
set_dimension!(m, :regions, ["A", "B"])
774+
add_comp!(m, testcomp, first = 2003)
775+
run(m)
776+
777+
@test m[:testcomp, :var_tvalue][findfirst(i -> i == 2003, 2000:2005), :] == [999., 999.]
778+
@test m[:testcomp, :var_tindex][findfirst(i -> i == 2003, 2000:2005), :] == [999., 999.]

0 commit comments

Comments
 (0)