From bf66e74220558a3a94093d83d216dd8e38942781 Mon Sep 17 00:00:00 2001 From: Jishnu Bhattacharya Date: Fri, 16 Aug 2024 17:13:45 +0530 Subject: [PATCH 1/3] Fix indexing in _mapreducedim for OffsetArrays --- base/reducedim.jl | 6 ++++-- test/offsetarray.jl | 7 +++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/base/reducedim.jl b/base/reducedim.jl index e74fe2b765277..b5ff827f89827 100644 --- a/base/reducedim.jl +++ b/base/reducedim.jl @@ -258,8 +258,10 @@ 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 = range(firstindex(R), length=nslices) + v = mapreduce_impl(f, op, A, ibase+1, ibase+lsiz) + r = op(@inbounds(R[i]), v) + @inbounds R[i] = r ibase += lsiz end return R diff --git a/test/offsetarray.jl b/test/offsetarray.jl index 5ee918e85faf7..fb5855dfbaa0d 100644 --- a/test/offsetarray.jl +++ b/test/offsetarray.jl @@ -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 From 4403aaa872622b9611140b712a06008f2cf00073 Mon Sep 17 00:00:00 2001 From: Jishnu Bhattacharya Date: Fri, 16 Aug 2024 17:46:12 +0530 Subject: [PATCH 2/3] Avoid intermediate variable --- base/reducedim.jl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/base/reducedim.jl b/base/reducedim.jl index b5ff827f89827..809b36353f1ee 100644 --- a/base/reducedim.jl +++ b/base/reducedim.jl @@ -259,8 +259,7 @@ function _mapreducedim!(f, op, R::AbstractArray, A::AbstractArrayOrBroadcasted) nslices = div(length(A), lsiz) ibase = first(LinearIndices(A))-1 for i = range(firstindex(R), length=nslices) - v = mapreduce_impl(f, op, A, ibase+1, ibase+lsiz) - r = op(@inbounds(R[i]), v) + r = op(@inbounds(R[i]), mapreduce_impl(f, op, A, ibase+1, ibase+lsiz)) @inbounds R[i] = r ibase += lsiz end From f59501c1c6816946fd7a12a242f923efd6bb2652 Mon Sep 17 00:00:00 2001 From: Jishnu Bhattacharya Date: Thu, 22 Aug 2024 19:58:52 +0530 Subject: [PATCH 3/3] use `eachindex` in loop Co-authored-by: Matt Bauman --- base/reducedim.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/reducedim.jl b/base/reducedim.jl index 809b36353f1ee..0478afe1a46b6 100644 --- a/base/reducedim.jl +++ b/base/reducedim.jl @@ -258,7 +258,7 @@ 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 = range(firstindex(R), length=nslices) + 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