Skip to content

Commit 16ddf2c

Browse files
authored
Revert "Adjust initialization in maximum and minimum (#27845)" (#28103)
This reverts commit 1859a91.
1 parent b2b1b18 commit 16ddf2c

File tree

6 files changed

+45
-69
lines changed

6 files changed

+45
-69
lines changed

base/abstractarray.jl

+1-6
Original file line numberDiff line numberDiff line change
@@ -1926,12 +1926,7 @@ function mapslices(f, A::AbstractArray; dims)
19261926
Rsize = copy(dimsA)
19271927
# TODO: maybe support removing dimensions
19281928
if !isa(r1, AbstractArray) || ndims(r1) == 0
1929-
# If the result of f on a single slice is a scalar then we add singleton
1930-
# dimensions. When adding adding the dimensions, we have to respect the
1931-
# index type of the input array (e.g. in the case of OffsetArrays)
1932-
tmp = similar(Aslice, typeof(r1), reduced_indices(Aslice, 1:ndims(Aslice)))
1933-
tmp[first(CartesianIndices(tmp))] = r1
1934-
r1 = tmp
1929+
r1 = [r1]
19351930
end
19361931
nextra = max(0, length(dims)-ndims(r1))
19371932
if eltype(Rsize) == Int

base/reducedim.jl

+28-47
Original file line numberDiff line numberDiff line change
@@ -3,41 +3,28 @@
33
## Functions to compute the reduced shape
44

55
# for reductions that expand 0 dims to 1
6-
reduced_index(i::OneTo) = OneTo(1)
7-
reduced_index(i::Slice) = Slice(first(i):first(i))
8-
reduced_index(i::AbstractUnitRange) =
9-
throw(ArgumentError(
10-
"""
11-
No method is implemented for reducing index range of type $typeof(i). Please implement
12-
reduced_index for this index type or report this as an issue.
13-
"""
14-
))
156
reduced_indices(a::AbstractArray, region) = reduced_indices(axes(a), region)
167

178
# for reductions that keep 0 dims as 0
189
reduced_indices0(a::AbstractArray, region) = reduced_indices0(axes(a), region)
1910

20-
function reduced_indices(inds::Indices{N}, d::Int) where N
11+
function reduced_indices(inds::Indices{N}, d::Int, rd::AbstractUnitRange) where N
2112
d < 1 && throw(ArgumentError("dimension must be ≥ 1, got $d"))
2213
if d == 1
23-
return (reduced_index(inds[1]), tail(inds)...)
14+
return (oftype(inds[1], rd), tail(inds)...)
2415
elseif 1 < d <= N
25-
return tuple(inds[1:d-1]..., oftype(inds[d], reduced_index(inds[d])), inds[d+1:N]...)::typeof(inds)
16+
return tuple(inds[1:d-1]..., oftype(inds[d], rd), inds[d+1:N]...)::typeof(inds)
2617
else
2718
return inds
2819
end
2920
end
21+
reduced_indices(inds::Indices, d::Int) = reduced_indices(inds, d, OneTo(1))
3022

3123
function reduced_indices0(inds::Indices{N}, d::Int) where N
3224
d < 1 && throw(ArgumentError("dimension must be ≥ 1, got $d"))
3325
if d <= N
3426
ind = inds[d]
35-
rd = isempty(ind) ? ind : reduced_index(inds[d])
36-
if d == 1
37-
return (rd, tail(inds)...)
38-
else
39-
return tuple(inds[1:d-1]..., oftype(inds[d], rd), inds[d+1:N]...)::typeof(inds)
40-
end
27+
return reduced_indices(inds, d, (isempty(ind) ? ind : OneTo(1)))
4128
else
4229
return inds
4330
end
@@ -51,7 +38,7 @@ function reduced_indices(inds::Indices{N}, region) where N
5138
if d < 1
5239
throw(ArgumentError("region dimension(s) must be ≥ 1, got $d"))
5340
elseif d <= N
54-
rinds[d] = reduced_index(rinds[d])
41+
rinds[d] = oftype(rinds[d], OneTo(1))
5542
end
5643
end
5744
tuple(rinds...)::typeof(inds)
@@ -66,7 +53,7 @@ function reduced_indices0(inds::Indices{N}, region) where N
6653
throw(ArgumentError("region dimension(s) must be ≥ 1, got $d"))
6754
elseif d <= N
6855
rind = rinds[d]
69-
rinds[d] = isempty(rind) ? rind : reduced_index(rind)
56+
rinds[d] = oftype(rind, (isempty(rind) ? rind : OneTo(1)))
7057
end
7158
end
7259
tuple(rinds...)::typeof(inds)
@@ -92,6 +79,25 @@ end
9279
reducedim_initarray(A::AbstractArray, region, init, ::Type{R}) where {R} = fill!(similar(A,R,reduced_indices(A,region)), init)
9380
reducedim_initarray(A::AbstractArray, region, init::T) where {T} = reducedim_initarray(A, region, init, T)
9481

82+
function reducedim_initarray0(A::AbstractArray{T}, region, f, ops) where T
83+
ri = reduced_indices0(A, region)
84+
if isempty(A)
85+
if prod(length, reduced_indices(A, region)) != 0
86+
reducedim_initarray0_empty(A, region, f, ops) # ops over empty slice of A
87+
else
88+
R = f == identity ? T : Core.Compiler.return_type(f, (T,))
89+
similar(A, R, ri)
90+
end
91+
else
92+
R = f == identity ? T : typeof(f(first(A)))
93+
si = similar(A, R, ri)
94+
mapfirst!(f, si, A)
95+
end
96+
end
97+
98+
reducedim_initarray0_empty(A::AbstractArray, region, f, ops) = mapslices(x->ops(f.(x)), A, dims = region)
99+
reducedim_initarray0_empty(A::AbstractArray, region,::typeof(identity), ops) = mapslices(ops, A, dims = region)
100+
95101
# TODO: better way to handle reducedim initialization
96102
#
97103
# The current scheme is basically following Steven G. Johnson's original implementation
@@ -118,33 +124,8 @@ function _reducedim_init(f, op, fv, fop, A, region)
118124
return reducedim_initarray(A, region, z, Tr)
119125
end
120126

121-
# initialization when computing minima and maxima requires a little care
122-
for (f1, f2, initval) in ((:min, :max, :Inf), (:max, :min, :(-Inf)))
123-
@eval function reducedim_init(f, op::typeof($f1), A::AbstractArray, region)
124-
# First compute the reduce indices. This will throw an ArgumentError
125-
# if any region is invalid
126-
ri = reduced_indices(A, region)
127-
128-
# Next, throw if reduction is over a region with length zero
129-
any(i -> isempty(axes(A, i)), region) && _empty_reduce_error()
130-
131-
# Make a view of the first slice of the region
132-
A1 = view(A, ri...)
133-
134-
if isempty(A1)
135-
# If the slice is empty just return non-view version as the initial array
136-
return copy(A1)
137-
else
138-
# otherwise use the min/max of the first slice as initial value
139-
v0 = mapreduce(f, $f2, A1)
140-
141-
# but NaNs need to be avoided as intial values
142-
v0 = v0 != v0 ? typeof(v0)($initval) : v0
143-
144-
return reducedim_initarray(A, region, v0)
145-
end
146-
end
147-
end
127+
reducedim_init(f, op::typeof(max), A::AbstractArray{T}, region) where {T} = reducedim_initarray0(A, region, f, maximum)
128+
reducedim_init(f, op::typeof(min), A::AbstractArray{T}, region) where {T} = reducedim_initarray0(A, region, f, minimum)
148129
reducedim_init(f::Union{typeof(abs),typeof(abs2)}, op::typeof(max), A::AbstractArray{T}, region) where {T} =
149130
reducedim_initarray(A, region, zero(f(zero(T))))
150131

stdlib/SparseArrays/src/sparsematrix.jl

+2
Original file line numberDiff line numberDiff line change
@@ -1616,6 +1616,8 @@ end
16161616
# non-trivial, so use Arrays for output
16171617
Base.reducedim_initarray(A::SparseMatrixCSC, region, v0, ::Type{R}) where {R} =
16181618
fill(v0, Base.reduced_indices(A,region))
1619+
Base.reducedim_initarray0(A::SparseMatrixCSC, region, v0, ::Type{R}) where {R} =
1620+
fill(v0, Base.reduced_indices0(A,region))
16191621

16201622
# General mapreduce
16211623
function _mapreducezeros(f, op, ::Type{T}, nzeros::Int, v0) where T

stdlib/SparseArrays/test/higherorderfns.jl

-3
Original file line numberDiff line numberDiff line change
@@ -628,8 +628,5 @@ end
628628
@test Diagonal(spzeros(5)) \ view(rand(10), 1:5) == [Inf,Inf,Inf,Inf,Inf]
629629
end
630630

631-
@testset "Issue #27836" begin
632-
@test minimum(sparse([1, 2], [1, 2], ones(Int32, 2)), dims = 1) isa Matrix
633-
end
634631

635632
end # module

test/offsetarray.jl

+8-8
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ targets2 = ["(1.0, 1.0)",
213213
"([1.0], [1.0])",
214214
"([1.0], [1.0])",
215215
"([1.0], [1.0])"]
216-
@testset "printing of OffsetArray with n=$n" for n = 0:4
216+
for n = 0:4
217217
a = OffsetArray(fill(1.,ntuple(d->1,n)), ntuple(identity,n))
218218
show(IOContext(io, :limit => true), MIME("text/plain"), a)
219219
@test String(take!(io)) == targets1[n+1]
@@ -362,9 +362,9 @@ A = OffsetArray(rand(4,4), (-3,5))
362362
@test maximum(A) == maximum(parent(A))
363363
@test minimum(A) == minimum(parent(A))
364364
@test extrema(A) == extrema(parent(A))
365-
@test maximum(A, dims=1) == OffsetArray(maximum(parent(A), dims=1), A.offsets)
366-
@test maximum(A, dims=2) == OffsetArray(maximum(parent(A), dims=2), A.offsets)
367-
@test maximum(A, dims=1:2) == OffsetArray(maximum(parent(A), dims=1:2), A.offsets)
365+
@test maximum(A, dims=1) == OffsetArray(maximum(parent(A), dims=1), (0,A.offsets[2]))
366+
@test maximum(A, dims=2) == OffsetArray(maximum(parent(A), dims=2), (A.offsets[1],0))
367+
@test maximum(A, dims=1:2) == maximum(parent(A), dims=1:2)
368368
C = similar(A)
369369
cumsum!(C, A, dims=1)
370370
@test parent(C) == cumsum(parent(A), dims=1)
@@ -396,11 +396,11 @@ I = findall(!iszero, z)
396396
@test findall(x->x==0, h) == [2]
397397
@test mean(A_3_3) == median(A_3_3) == 5
398398
@test mean(x->2x, A_3_3) == 10
399-
@test mean(A_3_3, dims=1) == median(A_3_3, dims=1) == OffsetArray([2 5 8], A_3_3.offsets)
400-
@test mean(A_3_3, dims=2) == median(A_3_3, dims=2) == OffsetArray(reshape([4,5,6],(3,1)), A_3_3.offsets)
399+
@test mean(A_3_3, dims=1) == median(A_3_3, dims=1) == OffsetArray([2 5 8], (0,A_3_3.offsets[2]))
400+
@test mean(A_3_3, dims=2) == median(A_3_3, dims=2) == OffsetArray(reshape([4,5,6],(3,1)), (A_3_3.offsets[1],0))
401401
@test var(A_3_3) == 7.5
402-
@test std(A_3_3, dims=1) == OffsetArray([1 1 1], A_3_3.offsets)
403-
@test std(A_3_3, dims=2) == OffsetArray(reshape([3,3,3], (3,1)), A_3_3.offsets)
402+
@test std(A_3_3, dims=1) == OffsetArray([1 1 1], (0,A_3_3.offsets[2]))
403+
@test std(A_3_3, dims=2) == OffsetArray(reshape([3,3,3], (3,1)), (A_3_3.offsets[1],0))
404404
@test sum(OffsetArray(fill(1,3000), -1000)) == 3000
405405

406406
@test norm(v) norm(parent(v))

test/reducedim.jl

+6-5
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@ safe_sumabs2(A::Array{T}, region) where {T} = safe_mapslices(sum, abs2.(A), regi
1717
safe_maxabs(A::Array{T}, region) where {T} = safe_mapslices(maximum, abs.(A), region)
1818
safe_minabs(A::Array{T}, region) where {T} = safe_mapslices(minimum, abs.(A), region)
1919

20-
@testset "test reductions over region: $region" for region in Any[
20+
Areduc = rand(3, 4, 5, 6)
21+
for region in Any[
2122
1, 2, 3, 4, 5, (1, 2), (1, 3), (1, 4), (2, 3), (2, 4), (3, 4),
2223
(1, 2, 3), (1, 3, 4), (2, 3, 4), (1, 2, 3, 4)]
23-
Areduc = rand(3, 4, 5, 6)
24+
# println("region = $region")
2425
r = fill(NaN, map(length, Base.reduced_indices(axes(Areduc), region)))
2526
@test sum!(r, Areduc) safe_sum(Areduc, region)
2627
@test prod!(r, Areduc) safe_prod(Areduc, region)
@@ -329,10 +330,10 @@ end
329330
@test sum(Any[1 2;3 4], dims=1) == [4 6]
330331
@test sum(Vector{Int}[[1,2],[4,3]], dims=1)[1] == [5,5]
331332

332-
@testset "Issue #10461. region=$region" for region in Any[-1, 0, (-1, 2), [0, 1], (1,-2,3), [0 1;
333+
# issue #10461
334+
Areduc = rand(3, 4, 5, 6)
335+
for region in Any[-1, 0, (-1, 2), [0, 1], (1,-2,3), [0 1;
333336
2 3], "hello"]
334-
Areduc = rand(3, 4, 5, 6)
335-
336337
@test_throws ArgumentError sum(Areduc, dims=region)
337338
@test_throws ArgumentError prod(Areduc, dims=region)
338339
@test_throws ArgumentError maximum(Areduc, dims=region)

0 commit comments

Comments
 (0)