Fix sum(bc::Broadcasted; dims = 1, init = 0)

[N5N3]

This PR make has_fast_linear_indexing rely on IndexStyle/ndims to fix mapreduce for Broadcasted with dim > 1.
Before:

julia> a = randn(100,100);

julia> bc = Broadcast.instantiate(Base.broadcasted(+,a,a));

julia> sum(bc,dims = 1,init = 0.0) == sum(collect(bc), dims = 1)
ERROR: MethodError: no method matching LinearIndices(::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{2}, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}, typeof(+), Tuple{Matrix{Float64}, Matrix{Float64}}})

After:

julia> sum(bc,dims = 1,init = 0.0) == sum(collect(bc), dims = 1)
true

This should extend the optimized fallback to more AbstractArray. (e.g. SubArray)

Test added.

[N5N3]

The only usage of has_fast_linear_indexing in Base I could find:

julia/base/reducedim.jl

Lines 248 to 261 in 8404e45

	function _mapreducedim!(f, op, R::AbstractArray, A::AbstractArrayOrBroadcasted)
	lsiz = check_reducedims(R,A)
	isempty(A) && return R
	if has_fast_linear_indexing(A) && lsiz > 16
	# use mapreduce_impl, which is probably better tuned to achieve higher performance
	nslices = div(length(A), lsiz)
	ibase = first(LinearIndices(A))-1
	for i = 1:nslices
	@inbounds R[i] = op(R[i], mapreduce_impl(f, op, A, ibase+1, ibase+lsiz))
	ibase += lsiz
	end
	return R
	end

[mcabbott]

Note that this fix isn't quite broad enough for the title. A Broadcasted which involves a tuple will still fail: No. The following example would also be fixed by the PR:

julia> using Base.Broadcast: broadcasted, instantiate

julia> bc = instantiate(broadcasted(+, [1 2], (3, 4)))
Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{2}}(+, ([1 2], (3, 4)))

julia> sum(bc)
20

julia> sum(bc; dims=1, init=0)
ERROR: MethodError: no method matching has_fast_linear_indexing(::Tuple{Int64, Int64})

Broadcasts which materialise to a tuple still fail, elsewhere. Maybe that's OK since you can't sum a tuple with dims in any case:

julia> Base.has_fast_linear_indexing(_) = false  # or the PR, I think

julia> bc2 = instantiate(broadcasted(+, 1, (2, 3, 4)))
Base.Broadcast.Broadcasted(+, (1, (2, 3, 4)))

julia> sum(bc2; dims=1, init=0)
ERROR: MethodError: no method matching similar(::Base.Broadcast.Broadcasted{Base.Broadcast.Style{Tuple}, Nothing, typeof(+), Tuple{Int64, Tuple{Int64, Int64, Int64}}}, ::Type{Int64}, ::Tuple{Base.OneTo{Int64}})

julia> copy(bc2)
(3, 4, 5)

julia> sum(ans; dims=1, init=0)
ERROR: MethodError: no method matching mapfoldl(::typeof(identity), ::typeof(Base.add_sum), ::Tuple{Int64, Int64, Int64}; dims::Int64, init::Int64)

The array .+ tuple case came up in JuliaDiff/ChainRules.jl#705

[N5N3]

It works for me though:

julia> using Base.Broadcast: broadcasted, instantiate

julia> bc = instantiate(broadcasted(+, [1 2], (3, 4)))
Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{2}}(+, ([1 2], (3, 4)))

julia> sum(bc)
20

julia> sum(bc; dims=1, init=0)
1×2 Matrix{Int64}:
 9  11

I guess the patch didn't work because of the old has_fast_linear_indexing(bc::Broadcast.Broadcasted)

[mcabbott]

Oh I'm an idiot, sorry, that's right. The PR removes the recursive method which caused the problem.