Skip to content

Commit 2247d0f

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 c3e7b1b commit 2247d0f

File tree

3 files changed

+19
-19
lines changed

3 files changed

+19
-19
lines changed

NEWS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ New library features
3333
Standard library changes
3434
------------------------
3535

36+
* Empty dimensional reductions (e.g., `reduce` and `mapreduce` with the `dims` keyword
37+
selecting one or more dimensions) now behave like their whole-array (`dims=:`) counterparts,
38+
only returning values in unambiguous cases and erroring otherwise.
39+
3640
#### JuliaSyntaxHighlighting
3741

3842
#### LinearAlgebra

base/reducedim.jl

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

340-
_mapreduce_dim(f, op, ::_InitialValue, A::AbstractArrayOrBroadcasted, dims) =
340+
function _mapreduce_dim(f, op, ::_InitialValue, A::AbstractArrayOrBroadcasted, dims)
341+
isempty(A) && return fill(mapreduce_empty(f, op, eltype(A)), reduced_indices(A, dims))
341342
mapreducedim!(f, op, reducedim_init(f, op, A, dims), A)
343+
end
342344

343345
"""
344346
reduce(f, A::AbstractArray; dims=:, [init])
@@ -1124,10 +1126,7 @@ findmin(f, A::AbstractArray; dims=:) = _findmin(f, A, dims)
11241126
function _findmin(f, A, region)
11251127
ri = reduced_indices0(A, region)
11261128
if isempty(A)
1127-
if prod(map(length, reduced_indices(A, region))) != 0
1128-
throw(ArgumentError("collection slices must be non-empty"))
1129-
end
1130-
similar(A, promote_op(f, eltype(A)), ri), zeros(eltype(keys(A)), ri)
1129+
_empty_reduce_error()
11311130
else
11321131
fA = f(first(A))
11331132
findminmax!(f, isgreater, fill!(similar(A, _findminmax_inittype(f, A), ri), fA),
@@ -1197,10 +1196,7 @@ findmax(f, A::AbstractArray; dims=:) = _findmax(f, A, dims)
11971196
function _findmax(f, A, region)
11981197
ri = reduced_indices0(A, region)
11991198
if isempty(A)
1200-
if prod(map(length, reduced_indices(A, region))) != 0
1201-
throw(ArgumentError("collection slices must be non-empty"))
1202-
end
1203-
similar(A, promote_op(f, eltype(A)), ri), zeros(eltype(keys(A)), ri)
1199+
_empty_reduce_error()
12041200
else
12051201
fA = f(first(A))
12061202
findminmax!(f, isless, fill!(similar(A, _findminmax_inittype(f, A), ri), fA),

test/reducedim.jl

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,19 +209,19 @@ end
209209

210210
for f in (minimum, maximum)
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" isequal(f(A, dims=2), zeros(Int, 0, 1))
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))
214+
@test_throws "reducing over an empty collection is not allowed" isequal(f(A, dims=3), zeros(Int, 0, 1))
215215
end
216216
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)))
217+
@test_throws "reducing over an empty collection is not allowed" f(A, dims=1)
218+
@test_throws "reducing over an empty collection is not allowed" isequal(f(A, dims=2), (zeros(Int, 0, 1), zeros(Int, 0, 1)))
219+
@test_throws "reducing over an empty collection is not allowed" f(A, dims=(1, 2))
220+
@test_throws "reducing over an empty collection is not allowed" isequal(f(A, dims=3), (zeros(Int, 0, 1), zeros(Int, 0, 1)))
221+
@test_throws "reducing over an empty collection is not allowed" f(abs2, A, dims=1)
222+
@test_throws "reducing over an empty collection is not allowed" isequal(f(abs2, A, dims=2), (zeros(Int, 0, 1), zeros(Int, 0, 1)))
223+
@test_throws "reducing over an empty collection is not allowed" f(abs2, A, dims=(1, 2))
224+
@test_throws "reducing over an empty collection is not allowed" isequal(f(abs2, A, dims=3), (zeros(Int, 0, 1), zeros(Int, 0, 1)))
225225
end
226226

227227
end

0 commit comments

Comments
 (0)