Skip to content

Commit

Permalink
fix JuliaLang#37199, indexing for 1-d views with offset ranges (Julia…
Browse files Browse the repository at this point in the history
…Lang#37204)

We had been assuming that IdentityUnitRange matched the indices of the parent (like Slices) but they define their own offset axes. Further, other `AbstractRanges` may be similarly offset. We do not need to consider other offset `AbstractArrays` as this code only applies to `IndexLinear` `SubArray`s.
  • Loading branch information
mbauman authored and simeonschaub committed Aug 29, 2020
1 parent 8dfd10b commit 216c4ed
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 5 deletions.
13 changes: 8 additions & 5 deletions base/subarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -366,17 +366,20 @@ end
# The running sum is `f`; the cumulative stride product is `s`.
# If the parent is a vector, then we offset the parent's own indices with parameters of I
compute_offset1(parent::AbstractVector, stride1::Integer, I::Tuple{AbstractRange}) =
(@_inline_meta; first(I[1]) - first(axes1(I[1]))*stride1)
(@_inline_meta; first(I[1]) - stride1*first(axes1(I[1])))
# If the result is one-dimensional and it's a Colon, then linear
# indexing uses the indices along the given dimension. Otherwise
# linear indexing always starts with 1.
# indexing uses the indices along the given dimension.
# If the result is one-dimensional and it's a range, then linear
# indexing might be offset if the index itself is offset
# Otherwise linear indexing always starts with 1.
compute_offset1(parent, stride1::Integer, I::Tuple) =
(@_inline_meta; compute_offset1(parent, stride1, find_extended_dims(1, I...), find_extended_inds(I...), I))
compute_offset1(parent, stride1::Integer, dims::Tuple{Int}, inds::Tuple{Union{Slice, IdentityUnitRange}}, I::Tuple) =
compute_offset1(parent, stride1::Integer, dims::Tuple{Int}, inds::Tuple{Slice}, I::Tuple) =
(@_inline_meta; compute_linindex(parent, I) - stride1*first(axes(parent, dims[1]))) # index-preserving case
compute_offset1(parent, stride1::Integer, dims, inds::Tuple{AbstractRange}, I::Tuple) =
(@_inline_meta; compute_linindex(parent, I) - stride1*first(axes1(inds[1]))) # potentially index-offsetting case
compute_offset1(parent, stride1::Integer, dims, inds, I::Tuple) =
(@_inline_meta; compute_linindex(parent, I) - stride1) # linear indexing starts with 1

function compute_linindex(parent, I::NTuple{N,Any}) where N
@_inline_meta
IP = fill_to_length(axes(parent), OneTo(1), Val(N))
Expand Down
27 changes: 27 additions & 0 deletions test/subarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,33 @@ end
@test _test_27632(view(ones(Int64, (1, 1, 1)), 1, 1, 1)) === nothing
end

@testset "issue #37199: 1-d views with offset range indices" begin
b = zeros(6, 3)
b[Base.IdentityUnitRange(4:6), 2] .= 3
@test b == [zeros(6, 1) [0,0,0,3,3,3] zeros(6,1)]
b[4, Base.IdentityUnitRange(2:3)] .= 4
@test b == [zeros(6,1) [0,0,0,4,3,3] [0,0,0,4,0,0]]
b[Base.IdentityUnitRange(2:3), :] .= 5
@test b == [zeros(1, 3); fill(5, 2, 3); [zeros(3) [4,3,3] [4,0,0]]]
b[:, Base.IdentityUnitRange(3:3)] .= 6
@test b == [[zeros(1, 2); fill(5, 2, 2); [zeros(3) [4,3,3]]] fill(6, 6)]

A = reshape(1:5*7*11, 11, 7, 5)
inds = (1:4, 2:5, 2, :, fill(3))
offset(x) = x
offset(r::UnitRange) = Base.IdentityUnitRange(r)
for i1 in inds
for i2 in inds
for i3 in inds
vo = @view A[offset(i1), offset(i2), offset(i3)]
v = @view A[i1, i2, i3]
@test first(vo) == first(v) == first(A[i1, i2, i3])
@test collect(A[i1, i2, i3]) == collect(vo) == collect(v)
end
end
end
end

@testset "issue #29608; contiguousness" begin
@test Base.iscontiguous(view(ones(1), 1))
@test Base.iscontiguous(view(ones(10), 1:10))
Expand Down

0 comments on commit 216c4ed

Please sign in to comment.