Skip to content

Conversation

@vtjnash
Copy link
Member

@vtjnash vtjnash commented Nov 2, 2023

Relands #29919

@vtjnash vtjnash marked this pull request as ready for review April 16, 2024 19:31
@vtjnash vtjnash force-pushed the revert-52003-revert-29919-sb/reduce-empty branch from b87fdeb to 6ca85cd Compare April 16, 2024 20:48
@vtjnash
Copy link
Member Author

vtjnash commented Apr 16, 2024

Looks like this still runs into this SparseArray problem:

SparseArrays/sparsematrix_ops                    (10) |         failed at 2024-04-16T20:47:52.441
Test Failed at /home/vtjnash/julia/usr/share/julia/stdlib/v1.12/SparseArrays/test/sparsematrix_ops.jl:248
  Expression: minimum(sparse(Int[]))
    Expected: errchecker
  No exception thrown

Test Failed at /home/vtjnash/julia/usr/share/julia/stdlib/v1.12/SparseArrays/test/sparsematrix_ops.jl:249
  Expression: maximum(sparse(Int[]))
    Expected: errchecker
  No exception thrown

@mbauman mbauman added the fold sum, maximum, reduce, foldl, etc. label Sep 3, 2024
@mbauman
Copy link
Member

mbauman commented Sep 4, 2024

OK, I have two good examples of why I don't like these behaviors. With this patch:

julia> minimum(Union{Missing,Int}[])
ERROR: MethodError: no method matching typemax(::Type{Union{Missing, Int64}})

julia> minimum(abs, Union{Missing,Int}[])
9223372036854775807

julia> maximum(Union{Missing,Int}[])
ERROR: MethodError: no method matching typemin(::Type{Union{Missing, Int64}})

julia> maximum(abs, Union{Missing,Int}[])
0

While I think it makes some sense that zero(Union{Missing, Int}) would be 0, I don't find it near as defensible that typemin or typemax should work — with or without an abs thrown into the mix. It seems like maximum(abs, Union{Missing,Int}[]) could be either missing or 0.

It also just doesn't seem like this is necessarily true in the abstract — for example:

julia> maximum(abs, Int8[])
0

julia> maximum(abs, Int8[-128]) # But there _does exist_ a lesser least element!
-128

That last example doesn't even require this patch — that's the current behavior of Julia. Still bad, though, and representative of what this patch wants to do more of.

@adienes
Copy link
Member

adienes commented Sep 4, 2024

julia> maximum(identity, [])
ERROR: MethodError: reducing over an empty collection is not allowed; consider supplying `init` to the reducer

julia> maximum(abs, [])
ERROR: MethodError: no method matching zero(::Type{Any})

julia> maximum(abs, Int8[])
0

😬 that's unfortunate.

maybe that can be fixed in your PR which is already slightly breaking? after all (slightly breaking ---> slightly more breaking) is a much smaller jump than (not breaking ---> slightly breaking)

@mbauman mbauman changed the title Reland "add more methods and tests for reductions over empty arrays" Define minimum and maximum of empty arrays to return typemin(T) May 1, 2025
@mbauman mbauman changed the title Define minimum and maximum of empty arrays to return typemin(T) Define minimum and maximum of empty arrays to return typemax and typemin May 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fold sum, maximum, reduce, foldl, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants