Skip to content

Commit

Permalink
Fix indexing in _mapreducedim for OffsetArrays (#55506)
Browse files Browse the repository at this point in the history
The destination array was being indexed incorrectly if it had offset
indices. This led to the following on nightly:
```julia
julia> using OffsetArrays

julia> r = 5:100;

julia> a = OffsetVector(r, 2);

julia> sum(a, dims=1)
1-element OffsetArray(::Vector{Int64}, 3:3) with eltype Int64 with indices 3:3:
 0

julia> sum(a)
5040
```
The indexing was marked `@inbounds`, so this was not throwing an error.
This PR also follows #55329 and only marks the indexing operations as
`@inbounds`, omitting the function calls.

---------

Co-authored-by: Matt Bauman <mbauman@juliahub.com>
  • Loading branch information
jishnub and mbauman authored Aug 23, 2024
1 parent 0c64283 commit 3d20a92
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 2 deletions.
5 changes: 3 additions & 2 deletions base/reducedim.jl
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,9 @@ function _mapreducedim!(f, op, R::AbstractArray, A::AbstractArrayOrBroadcasted)
# use mapreduce_impl, which is probably better tuned to achieve higher performance
nslices = div(length(A), lsiz)
ibase = first(LinearIndices(A))-1
for i = 1:nslices
@inbounds R[i] = op(R[i], mapreduce_impl(f, op, A, ibase+1, ibase+lsiz))
for i in eachindex(R)
r = op(@inbounds(R[i]), mapreduce_impl(f, op, A, ibase+1, ibase+lsiz))
@inbounds R[i] = r
ibase += lsiz
end
return R
Expand Down
7 changes: 7 additions & 0 deletions test/offsetarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -907,3 +907,10 @@ end
v = view([1,2,3,4], :)
@test v[Base.IdentityUnitRange(2:3)] == OffsetArray(2:3, 2:3)
end

@testset "mapreduce with OffsetRanges" begin
r = 5:100
a = OffsetArray(r, 2)
b = sum(a, dims=1)
@test b[begin] == sum(r)
end

0 comments on commit 3d20a92

Please sign in to comment.