Skip to content

Commit 94f24b8

Browse files
committed
unify reduce/reducedim empty case behaviors
Previously, Julia tried to guess at initial values for empty dimensional reductions in a slightly different way than whole-array reductions. This change unifies those behaviors, such that `mapreduce_empty` is called for empty dimensional reductions, just like it is called for empty whole-array reductions. Beyond the unification (which is valuable in its own right), this change enables some downstream simplifications to dimensional reductions and is likely to be the "most breaking" public behavior in a refactoring targeting more correctness improvements.
1 parent 1b16a39 commit 94f24b8

File tree

3 files changed

+69
-23
lines changed

3 files changed

+69
-23
lines changed

NEWS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ New library features
4141
Standard library changes
4242
------------------------
4343

44+
* Empty dimensional reductions (e.g., `reduce` and `mapreduce` with the `dims` keyword
45+
selecting one or more dimensions) now behave like their whole-array (`dims=:`) counterparts,
46+
only returning values in unambiguous cases and erroring otherwise.
47+
4448
#### JuliaSyntaxHighlighting
4549

4650
#### LinearAlgebra

base/reducedim.jl

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,10 @@ _mapreduce_dim(f, op, ::_InitialValue, A::AbstractArrayOrBroadcasted, ::Colon) =
341341
_mapreduce_dim(f, op, nt, A::AbstractArrayOrBroadcasted, dims) =
342342
mapreducedim!(f, op, reducedim_initarray(A, dims, nt), A)
343343

344-
_mapreduce_dim(f, op, ::_InitialValue, A::AbstractArrayOrBroadcasted, dims) =
344+
function _mapreduce_dim(f, op, ::_InitialValue, A::AbstractArrayOrBroadcasted, dims)
345+
isempty(A) && return fill(mapreduce_empty(f, op, eltype(A)), reduced_indices(A, dims))
345346
mapreducedim!(f, op, reducedim_init(f, op, A, dims), A)
347+
end
346348

347349
"""
348350
reduce(f, A::AbstractArray; dims=:, [init])
@@ -1128,10 +1130,7 @@ findmin(f, A::AbstractArray; dims=:) = _findmin(f, A, dims)
11281130
function _findmin(f, A, region)
11291131
ri = reduced_indices0(A, region)
11301132
if isempty(A)
1131-
if prod(map(length, reduced_indices(A, region))) != 0
1132-
throw(ArgumentError("collection slices must be non-empty"))
1133-
end
1134-
similar(A, promote_op(f, eltype(A)), ri), zeros(eltype(keys(A)), ri)
1133+
_empty_reduce_error()
11351134
else
11361135
fA = f(first(A))
11371136
findminmax!(f, isgreater, fill!(similar(A, _findminmax_inittype(f, A), ri), fA),
@@ -1201,10 +1200,7 @@ findmax(f, A::AbstractArray; dims=:) = _findmax(f, A, dims)
12011200
function _findmax(f, A, region)
12021201
ri = reduced_indices0(A, region)
12031202
if isempty(A)
1204-
if prod(map(length, reduced_indices(A, region))) != 0
1205-
throw(ArgumentError("collection slices must be non-empty"))
1206-
end
1207-
similar(A, promote_op(f, eltype(A)), ri), zeros(eltype(keys(A)), ri)
1203+
_empty_reduce_error()
12081204
else
12091205
fA = f(first(A))
12101206
findminmax!(f, isless, fill!(similar(A, _findminmax_inittype(f, A), ri), fA),

test/reducedim.jl

Lines changed: 60 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -207,23 +207,34 @@ end
207207
@test isequal(prod(A, dims=(1, 2)), fill(1, 1, 1))
208208
@test isequal(prod(A, dims=3), fill(1, 0, 1))
209209

210-
for f in (minimum, maximum)
210+
for f in (minimum, maximum, findmin, findmax)
211211
@test_throws "reducing over an empty collection is not allowed" f(A, dims=1)
212-
@test isequal(f(A, dims=2), zeros(Int, 0, 1))
212+
@test_throws "reducing over an empty collection is not allowed" f(A, dims=2)
213213
@test_throws "reducing over an empty collection is not allowed" f(A, dims=(1, 2))
214-
@test isequal(f(A, dims=3), zeros(Int, 0, 1))
215-
end
216-
for f in (findmin, findmax)
217-
@test_throws ArgumentError f(A, dims=1)
218-
@test isequal(f(A, dims=2), (zeros(Int, 0, 1), zeros(Int, 0, 1)))
219-
@test_throws ArgumentError f(A, dims=(1, 2))
220-
@test isequal(f(A, dims=3), (zeros(Int, 0, 1), zeros(Int, 0, 1)))
221-
@test_throws ArgumentError f(abs2, A, dims=1)
222-
@test isequal(f(abs2, A, dims=2), (zeros(Int, 0, 1), zeros(Int, 0, 1)))
223-
@test_throws ArgumentError f(abs2, A, dims=(1, 2))
224-
@test isequal(f(abs2, A, dims=3), (zeros(Int, 0, 1), zeros(Int, 0, 1)))
214+
@test_throws "reducing over an empty collection is not allowed" f(A, dims=3)
215+
if f === maximum
216+
# maximum allows some empty reductions with abs/abs2
217+
z = f(abs, A)
218+
@test isequal(f(abs, A, dims=1), fill(z, (1,1)))
219+
@test isequal(f(abs, A, dims=2), fill(z, (0,1)))
220+
@test isequal(f(abs, A, dims=(1,2)), fill(z, (1,1)))
221+
@test isequal(f(abs, A, dims=3), fill(z, (0,1)))
222+
z = f(abs2, A)
223+
@test isequal(f(abs2, A, dims=1), fill(z, (1,1)))
224+
@test isequal(f(abs2, A, dims=2), fill(z, (0,1)))
225+
@test isequal(f(abs2, A, dims=(1,2)), fill(z, (1,1)))
226+
@test isequal(f(abs2, A, dims=3), fill(z, (0,1)))
227+
else
228+
@test_throws "reducing over an empty collection is not allowed" f(abs, A, dims=1)
229+
@test_throws "reducing over an empty collection is not allowed" f(abs, A, dims=2)
230+
@test_throws "reducing over an empty collection is not allowed" f(abs, A, dims=(1, 2))
231+
@test_throws "reducing over an empty collection is not allowed" f(abs, A, dims=3)
232+
@test_throws "reducing over an empty collection is not allowed" f(abs2, A, dims=1)
233+
@test_throws "reducing over an empty collection is not allowed" f(abs2, A, dims=2)
234+
@test_throws "reducing over an empty collection is not allowed" f(abs2, A, dims=(1, 2))
235+
@test_throws "reducing over an empty collection is not allowed" f(abs2, A, dims=3)
236+
end
225237
end
226-
227238
end
228239

229240
## findmin/findmax/minimum/maximum
@@ -720,3 +731,38 @@ end
720731
@test_broken @inferred(maximum(exp, A; dims = 1))[1] === missing
721732
@test_broken @inferred(extrema(exp, A; dims = 1))[1] === (missing, missing)
722733
end
734+
735+
some_exception(op) = try return (Some(op()), nothing); catch ex; return (nothing, ex); end
736+
reduced_shape(sz, dims) = ntuple(d -> d in dims ? 1 : sz[d], length(sz))
737+
738+
@testset "Ensure that calling, e.g., sum(empty; dims) has the same behavior as sum(empty)" begin
739+
@testset "$r(Array{$T}(undef, $sz); dims=$dims)" for
740+
r in (minimum, maximum, findmin, findmax, extrema, sum, prod, all, any, count),
741+
T in (Int, Union{Missing, Int}, Number, Union{Missing, Number}, Bool, Union{Missing, Bool}, Any),
742+
sz in ((0,), (0,1), (1,0), (0,0), (0,0,1), (1,0,1)),
743+
dims in (1, 2, 3, 4, (1,2), (1,3), (2,3,4), (1,2,3))
744+
745+
A = Array{T}(undef, sz)
746+
rsz = reduced_shape(sz, dims)
747+
748+
v, ex = some_exception() do; r(A); end
749+
if isnothing(v)
750+
@test_throws typeof(ex) r(A; dims)
751+
else
752+
actual = fill(something(v), rsz)
753+
@test isequal(r(A; dims), actual)
754+
@test eltype(r(A; dims)) === eltype(actual)
755+
end
756+
757+
for f in (identity, abs, abs2)
758+
v, ex = some_exception() do; r(f, A); end
759+
if isnothing(v)
760+
@test_throws typeof(ex) r(f, A; dims)
761+
else
762+
actual = fill(something(v), rsz)
763+
@test isequal(r(f, A; dims), actual)
764+
@test eltype(r(f, A; dims)) === eltype(actual)
765+
end
766+
end
767+
end
768+
end

0 commit comments

Comments
 (0)