Dead code in base/broadcast.jl

Internals & Design
1

If I read correctly some code in base/broadcast.jl is never executed now.

To be specific:

# Indices utilities
@inline combine_axes(A, B...) = broadcast_shape(axes(A), combine_axes(B...))
combine_axes(A) = axes(A)

broadcast_shape(shape::Tuple) = shape
broadcast_shape(shape::Tuple, shape1::Tuple, shapes::Tuple...) = broadcast_shape(_bcs(shape, shape1), shapes...)
_bcs(::Tuple{}, ::Tuple{}) = ()
_bcs(::Tuple{}, newshape::Tuple) = (newshape[1], _bcs((), tail(newshape))...) # FIX: =newshape ?
_bcs(shape::Tuple, ::Tuple{}) = (shape[1], _bcs(tail(shape), ())...) # FIX: =shape?
function _bcs(shape::Tuple, newshape::Tuple)
    return (_bcs1(shape[1], newshape[1]), _bcs(tail(shape), tail(newshape))...)
end
_bcs1(a::Integer, b::Integer) = a == 1 ? b : (b == 1 ? a : (a == b ? a : throw(DimensionMismatch("arrays could not be broadcast to a common size")))) # never reached?
_bcs1(a::Integer, b) = a == 1 ? b : (first(b) == 1 && last(b) == a ? b : throw(DimensionMismatch("arrays could not be broadcast to a common size")))# never reached?
_bcs1(a, b::Integer) = _bcs1(b, a)# never reached?
_bcs1(a, b) = _bcsm(b, a) ? axistype(b, a) : (_bcsm(a, b) ? axistype(a, b) : throw(DimensionMismatch("arrays could not be broadcast to a common size")))
_bcsm(a, b) = a == b || length(b) == 1
_bcsm(a, b::Number) = b == 1# never reached?
_bcsm(a::Number, b::Number) = a == b || b == 1# never reached?
axistype(a::T, b::T) where T = a
axistype(a, b) = UnitRange{Int}(a)

is as good as

# Indices utilities
@inline combine_axes(A, B...) = broadcast_shape(axes(A), combine_axes(B...))
combine_axes(A) = axes(A)

broadcast_shape(shape::Tuple) = shape
broadcast_shape(shape::Tuple, shape1::Tuple, shapes::Tuple...) = broadcast_shape(_bcs(shape, shape1), shapes...)
_bcs(::Tuple{}, ::Tuple{}) = ()
_bcs(::Tuple{}, newshape::Tuple) = newshape
_bcs(shape::Tuple, ::Tuple{}) = shape
function _bcs(shape::Tuple, newshape::Tuple)
    return (_bcs1(shape[1], newshape[1]), _bcs(tail(shape), tail(newshape))...)
end
_bcs1(a, b) = _bcsm(b, a) ? axistype(b, a) : (_bcsm(a, b) ? axistype(a, b) : throw(DimensionMismatch("arrays could not be broadcast to a common size")))
_bcsm(a, b) = a == b || length(b) == 1
axistype(a::T, b::T) where T = a
axistype(a, b) = UnitRange{Int}(a)

Am I missing something?

2

Did you try that change and run the test suite?

3 Likes
3

Yeah, that seems better

I think this is easily reached: this states that a is not an Integer and b is

Similarly, this appears to state that a is not a Number, and b is not an Integer but is a Number

4

But can b be an integer at all? It is an element of the return value of axes. I think it is always a range-like?

5

Ah, OK. That sounds right then: it seems like we can now change that to throw an error (“axes defined incorrectly”) if either a or b is a Number. Would you like to make a PR? That’ll get more eyes on this too, and confirm that CI agrees it is not reached.

6

Yup, looks like this can be simplified significantly. Looking through the blame, that code has survived at least three major refactors wherein it was simply incrementally extended/refined. A PR to simplify the shape calculation would be welcome — and it’d be a great opportunity to improve the error messages, too! (ref https://github.com/JuliaLang/julia/issues/30438)

7

Thank you both for looking into this and for your kind words. @jameson @mbauman

Recently I am thinking on and off about extending the current broadcasting mechanism to make generalizing it easier, reread base/broadcast.jl is just part of that process. So I don’t think I will make a PR just for the sake of cleaner code, because that code will be refactored if the “generalized” version comes into being anyway.

There are still a lot of uncertainties now but I think I can try to describe my goal here, so that I can learn from more educated minds.

Broadcasting as is currently implemented by julia is a special case of a common pattern seen in Dagger.jl, Dask, Tenserflow, etc. Namely, the user specifies a list of data and operations along with their dependencies (call graph building), the library turns the specification into an execution plan and execute it (transformation and execution). The special thing about broadcasting in julia is a) the execution planning is done in the type domain (by walking up the call graph to agree on a BroadcastStyle), b) a do-what-i-mean-ish semantic is laid down as broadcasting rules.

My goal is to a) making violating a) less hackish (currently one has to either use a non singleton BroadcastStyle or manually walk a Broadcasted object.) b) leave more sockets in the broadcast mechanism to leave more room for even more aggressive DWIM style broadcasting, more on this later.

Some examples of the “more aggressive DWIM style broadcasting” I can think of:

  1. Patch broadcasting: Allow an offset array of axes (-1:1, -1:1) to be broadcasted with another of axes (0:2, 0:2), resulting an offset array of axes (0:1, 0:1). This style is appealing if you know each index has a common meaning for all participating arrays.
  2. broadcasting over dictionaries/NamedTuples (already reserved now but I can’t find the discussion).
  3. broadcasting over infinite generators yielding a new generator.

Generally nothing revolutionary, just making the @. syntactic sugar sweeter (and more error prone).

To achieve this I think it is necessary to generalize from axes to a concept that represents an arbitrary collection of indices, because we have more things to consider than simply shape. One should also be able to control the way to iterate through the indices (currently one is bound to eachindex).

If you are still not bored, I have tried to formalize some API here.

Any suggestions/criticism/pointer to existing discussions very appreciated.

1 Like