JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.79k stars 5.49k forks source link

Reduce with dims is incompletely implemented #45566

Open LilithHafner opened 2 years ago

LilithHafner commented 2 years ago
julia> reduce(gcd, [1])
1

julia> reduce(gcd, [1]; dims=1)
ERROR: MethodError: no method matching reducedim_init(::typeof(identity), ::typeof(gcd), ::Vector{Int64}, ::Int64)
Closest candidates are:
  reducedim_init(::Any, ::typeof(min), ::AbstractArray, ::Any) at /Applications/Julia-1.7.3.app/Contents/Resources/julia/share/julia/base/reducedim.jl:129
  reducedim_init(::Any, ::typeof(max), ::AbstractArray, ::Any) at /Applications/Julia-1.7.3.app/Contents/Resources/julia/share/julia/base/reducedim.jl:129
  reducedim_init(::Any, ::typeof(&), ::Union{Base.AbstractBroadcasted, AbstractArray}, ::Any) at /Applications/Julia-1.7.3.app/Contents/Resources/julia/share/julia/base/reducedim.jl:171
  ...
Stacktrace:
 [1] _mapreduce_dim(f::Function, op::Function, #unused#::Base._InitialValue, A::Vector{Int64}, dims::Int64)
   @ Base ./reducedim.jl:336
 [2] #mapreduce#731
   @ ./reducedim.jl:322 [inlined]
 [3] #reduce#733
   @ ./reducedim.jl:371 [inlined]
 [4] top-level scope
   @ REPL[6]:1
LilithHafner commented 2 years ago

When init is not specified, we should construct inital values from the provided array.

andrewjradcliffe commented 2 years ago

The peculiarity with reduce(op, [1]) stems from it calling through a call chain which ends here. In fact, op never gets called; for example:

julia> mybinaryop(a, b) = (println("hello world!"); a + b)
mybinaryop (generic function with 1 method)

julia> reduce(mybinaryop, [1])
1

julia> reduce(mybinaryop, [1,2])
hello world!
3

julia> reduce(mybinaryop, [1,2,3])
hello world!
hello world!
6

To be fair, the docstring for reduce with kwarg dims states that the init kwarg is optional only for +, *, max and min.

Constructing initial values can be quite difficult -- the neutral element cannot necessarily be derived from the array and binary operator alone, hence the error thrown.

LilithHafner commented 2 years ago

To be fair, the docstring for reduce with kwarg dims states that the init kwarg is optional only for +, *, max and min.

reduce(f, A; dims=:, [init])

Reduce 2-argument function f along dimensions of A. dims is a vector specifying the dimensions to reduce, and the keyword argument init is the initial value to use in the reductions. For +, *, max and min the init argument is optional.

That's a good point, however init is still listed in square braces which indicates that it is optional. More importantly, we should never need an init argument for non-empty collections. We can simply use the first element of the collection as the initial value. This is worth supporting because, as you noted, it can be hard to come up with a neutral element. In some cases, there is no neutral element, but we should still support multidimensional reductions.