RFC: Deprecate defaulting to scalar in broadcast

[mbauman]

This PR consists of two commits — both pass tests locally. The first commit changes the default behavior of broadcast to collect its arguments instead of treating them as scalars. The second layers on top a deprecation mechanism that makes this — largely — non-breaking.

I believe this to be a compromise that will fix #18618 once deprecations are removed. Note that I kept all the types (except Ptr) that are currently tested to behave as scalars as being a part of the Scalar trait.

I initially went even farther, and attempted to remove Scalar() from the broadcast machinery entirely. I like that it completely does away with the trait — you simply specialize broadcast_collect to determine if you want to wrap your object in a Ref. It has the major downside, though, in that it'd make 1 .+ 2 == fill(3, ()). As a drastic alternative, we could consider unwrapping all AbstractArray{T,0}s that result from broadcast and bringing them back into the scalar world.

[andyferris]

Cool, this looks like an interesting change! On a technical level this looks good, and I can see it could be useful, but I have a few comments.

Recently, I've been peddling a thought (e.g. here and here and elsewhere) which goes like "map is for iterables and broadcast is for indexables". Of course, that's a slight lie since broadcasting is also about "broadcasting" a scalar over a collection, which may be convenient even for containers that aren't indexable, but the general flavor is to match up indices (in interesting and powerful ways, in the case of arrays). As evidence of my claim I would say that:

• I would be comfortable with a lazy map which worked on any arbitrary iterable collection (even those that aren't indexable), and a lazy broadcast which worked on any arbitrary indexable collection (even those which aren't iterable, e.g. the indices may be a continuous set). A lazy map of container a might be indexable if a is, and similarly a lazy broadcast of container b might be iterable if b is.

I also happen to be in favor of strong, generic interfaces (i.e. generic interfaces that provide some guarantees on the behavior of their outputs) - for example we might say that broadcast over a single indexable always creates an indexable with the same keys (another example might perhaps be that map never, ever messes with iteration order). We've never explicitly written down these particular "rules" but they seem somewhat sensible to me and such guarantees are useful for others to write generic code which can provide other guarantees (like not crashing), and so-on.

Regarding AbstractDict, both before and after this PR we have what I personally see as undesirable behavior. Prior to this, dictionaries were treated as scalar. After this, it seems that dictionaries will be converted to a vector of key-value pairs and then broadcast based on their enumeration order rather than their keys. If two dictionaries iterate their keys in different orders, the elements will match up in unexpected ways, which seems bad. I guess if we merge this we should also consider #26158 to fix that up. (As an aside, regarding what I said about strong interfaces, I sometimes wonder whether we should avoid map over two dictionaries altogether in favor of broadcast since the iteration order could be different and it's weird (to me) that map can mess with that).

Also, with this, for broadcasting with Strings does broadcast_collect end up giving us a Vector{Char} and perform broadcasting over that? (There was some discussion about whether strings were treated as a scalar or a collection for the purposes of broadcast in #18618).

In conclusion - I'd just like to raise the question of whether we really want to tie the fallback behavior of broadcast to the iteration API? One alternative we might consider is that the fallback could try and iterate over the keys of the inputs, instead?

[andyferris]

I’ll also add that by working with keys we might be able to create a fallback definition of broadcast!, but I can’t see how we’d extend this PR to do that.

[mbauman]

Very good thoughts. A smattering of responses:

• The basic idea here is that we will collect objects whose BroadcastStyle is Unknown (the new default). Any type that has a specialized BroadcastStyle is assumed to "know" how to broadcast as-is and is not touched.
• String is still special-cased as a "Scalar" BroadcastStyle on this branch. It's just so useful (and contentious) that I didn't want to bog down this PR with that one special case.
• Dicts get tied up in the exact same deprecation here. I definitely agree that a vector of pairs is wholly unhelpful, and I'm imagining that your PR (or something along those lines) will come into place afterwards. That'll give Dict a known BroadcastStyle and prevent it from falling back to the collect of unknowns.
• Further, I do agree that if keys exists and is not an AbstractUnitRange, it'd be nice to allow ourselves space to change the output there in a future release. I'm not sure how to test for that beyond applicable.

I don't find the distinction between iterable and indexable all that compelling. We privilege broadcast with the dot-syntax, and — while it'd be a PITA — its implementation could lean more heavily on iteration for key-less objects. Even without that implementation change, though, it's so very easy to support an indexing implementation with iterable arguments by collecting them first. I'd say that broadcasting is more about shapes than anything else… and we do have objects that are iterable and have shape but aren't indexable (Iterators.product).

[vchuravy]

Recently, I've been peddling a thought (e.g. here and here and elsewhere) which goes like "map is for iterables and broadcast is for indexables". Of course, that's a slight lie since broadcasting is also about "broadcasting" a scalar over a collection, which may be convenient even for containers that aren't indexable, but the general flavor is to match up indices (in interesting and powerful ways, in the case of arrays). As evidence of my claim I would say that:

I would rather say "map is for iterables" and "broadcast is for things (iterables) with shapes"

I also happen to be in favor of strong, generic interfaces (i.e. generic interfaces that provide some guarantees on the behavior of their outputs) - for example we might say that broadcast over a single indexable always creates an indexable with the same keys (another example might perhaps be that map never, ever messes with iteration order). We've never explicitly written down these particular "rules" but they seem somewhat sensible to me and such guarantees are useful for others to write generic code which can provide other guarantees (like not crashing), and so-on.

I understand the desire for strong rules, but keep in mind that having those rules also limits what we can do. As an example currently a mapreduce implementation is free to change the order of operations based on the func and based on the objects it maps over. Which allows implementation based on SIMD, pairwise-addition, GPU implementations, parallel implementations, all examples where having an undefined iteration order are helpful. One could impose that for map messing with the iteration order should not be observable in the output, but that only works for pure functions.

[vtjnash]

change the order of operations

I think the proposed rule for map is that the order of the output vector should match the order of the iterator, but not necessarily that the output will be emitted in that order.

for broadcasting with Strings

Perhaps Strings are simply a second example (like Matt's Iterators.product) where the keys (EachStringIndex) aren't the shape (1:length)?

I definitely agree that a vector of pairs is wholly unhelpful

I disagree. In the limit case of one item, being able to operate on each element in iteration order is useful collect(f(k, v) for (k, v) in d) --> f.(d). While if you wanted to operate on just the values, you could just write that Dict(zip(keys(d), f(v) for v in d)) --> f.(values(d)). And many dictionaries are implemented with specific sort orders – including ordered sets (DataStructures.OrderedDict), trees (DataStructures. SortedDict), or linear scan implementations like NamedTuple and Base.ImmutableDict)).

[stevengj]

So, for example, floor.(T, array) will no longer work, because a type T won't be treated as a scalar?

[mbauman]

No, this is the current list of specially cased Scalar arguments:

BroadcastStyle(::Type{<:Union{Number,Symbol,String,Type,Function,Uninitialized,RoundingMode,Nothing,Missing}}) = Scalar()

The most dissatisfying ones are the singletons like nothing and missing… it'd be nice to just assume they were all scalar. But you know what else is a singleton? I.

[stevengj]

You could always make singletons default to being treated as scalars and then opt-in to being a container for I if you want broadcasting on I (though I doesn't have a length or shape). I'm not sure how to dispatch on whether a type is a singleton?

[mbauman]

My point is simply that it's possible to make meaningful and useful things that are array-ish and definitely not scalar but still singletons.

The whole idea here is to remove as many special cases as possible. Right now the rule is very simple: if a type doesn't define a BroadcastStyle, we collect it. The list of special cases that need to specifically add a Scalar() trait is relatively small. I'd be interested in discovering more that folks use.

[andyferris]

I totally agree we should make a simple rule where user's don't need to consider special cases. For comparison, my proposal was: if a type doesn't define a BroadcastStyle, we use it's keys along with getindex and setindex! (I guess for this, broadcast may need to output a Dict by default to handle the arbitrary keys, but that choice won't affect broadcast! in any way). If keys errors and there is no BroadcastStyle defined, then broadcast will error.

Using this we gain over this approach that indices match up correctly by default + a good fallback implementation for broadcast!. I really worry that it will be confusing for users if the indices are ever ignored. I would go so far as to say that with the example of product - I would still feel it would be desirable if we broadcast over multiple products of AbstractDicts (with different iteration orders), broadcast might still magically match up the indices for us (rather than have product hide the indices and present just an enumeration - we might even possibly make product indexable somehow? That may be getting speculative, but I have recently started entering the territory where multidimensional dictionaries might be useful to me).

As to what we should do for v1.0 - have we also considered just deprecating the Unknown as Scalar behavior in v0.7, and error-out whenever BroadcastStyle is Unknown in v1.0? That would leave some more time to discuss this without blocking an alpha release, we can turn #26158 into a package for exploration, and implement whatever behavior we want in Base in the future in a non-breaking way.

Jameson wrote:

I disagree. In the limit case of one item, being able to operate on each element in iteration order is useful collect(f(k, v) for (k, v) in d) --> f.(d). While if you wanted to operate on just the values, you could just write that Dict(zip(keys(d), f(v) for v in d)) --> f.(values(d)). And many dictionaries are implemented with specific sort orders – including ordered sets (DataStructures.OrderedDict), trees (DataStructures. SortedDict), or linear scan implementations like NamedTuple and Base.ImmutableDict)).

I do wonder whether this is precisely where it's a good think that map and broadcast can be a bit different, so people can chose the behavior they need? For iteration behavior, you can do map(f, d) instead of f.(d). Yes, you lose the concise dot syntax, but still you get the choice. Not too bad of a trade off, perhaps? (off-topic, but I'd still really like that first example to become f.(pairs(d)) and the second to be f.(d), to match the array behavior :) ).

[vtjnash]

I really worry that it will be confusing for users if the indices are ever ignored

I agree, I think we just have different solutions for this. I've taken the position that it should be possible to broadcast over an iterable, so it should be OK to drop indices if the user has expressed that they are optional (for example, by calling values), but otherwise, must broadcast the elements (so that the user can't actually "lose" the keys). The behavior your propose (a strict join) is certainly also useful – but is it useful enough to justify continuing to prohibit using . syntax as a convenient shorthand for a generator (this PR)? And useful enough to justify adding new cases to the BroadcastType machinery, rather than finding ways of removing them (Matt's proposal to "remove Scalar() from the broadcast machinery entirely")?

[mbauman]

Closing in favor of #26435, which is based on that more radical proposal I linked to in the original post (remove Scalar() from the broadcast machinery entirely), but does away with the hugely breaking always-return-a-0-dim-array aspect. I like it much better. Details there.