RFC: Deprecate multi-value non-scalar indexed assignment

[mbauman]

Updated Feb 9, 2018: This PR is now ready for review. Executive summary:

The current setindex! function (the A[i] = b syntax) does three very different operations:

• Given an index i that refers to only one location (scalar indexing), A[i] = b modifies A in that location to hold b.
• Given an index I that refers to more than one location (non-scalar indexing), A[I] = b can mean one of two things:
  • If b is an AbstractArray (multi-value), assign the values it contains to those locations I in A. That is, essentially, [A[i] = bᵢ for (i, bᵢ) in zip(I, b)].
  • If b is not an AbstractArray (scalar-value), then broadcast its value to every location selected by I in A.

These two different behaviors in the non-scalar indexing case basically make using this function in a generic way impossible. If you want to always set the value b to many indices of A, you cannot use this syntax because b might happen to be an array in some cases, radically changing the meaning of the expression. You need to manually iterate over the indices, using scalar setindex methods. But we now also have the new broadcast! syntax, A[I] .= B, which uses the usual broadcasting semantics to determine how B should fill into the values of A.

This PR deprecates the special "multi-value non-scalar indexing" behavior in favor of using .= broadcasted assignments. It's important to note, however, that these two behaviors are not equivalent, and in some cases the deprecation is quite convoluted. There are three reasons for this:

• Multivalue nonscalar setindex! is more permissive in its shape checks than broadcasting is (when both I and b contain the same number of elements).
• Broadcasting is more permissive in allowing, well, broadcasting across singleton dimensions.
• Broadcasting has the additional caveat that not only AbstractArrays will expand out to multiple indices, but tuples and other broadcasting collections will, too.

This was not a simple change to make; simply following the deprecation message won't always work. The places where I had the biggest difficulty adjusting to the new behavior was in cases where we still do this kind of "arrays are special" concatenating behavior: cat, mapslices, etc. The other place was in OffsetArrays — multi-value non-scalar setindex doesn't care about matching indices but broadcasting does.

---

Old message: This is still a ways from completion, but I wanted to post this WIP so folks could start getting a sense of what this deprecation would mean. This would address the 1.0 portion of #24086.

One interesting aspect of this is that x[I] .= … is much more permissive than the current setindex! method is… and some of our internals are relying on those tougher bounds. So we'll have to carefully vet the changes here. One example from this current branch:

julia> Int[1:3; [4 5]]
4×2 Array{Int64,2}:
 1  1
 2  2
 3  3
 4  5

[timholy]

Assuming this is the only reasonable path forward to A.[I] .= X, I like it even if it's a little scary in its permissiveness.

I changed the broadcast syntax since you started working on this. Do you need any help finishing this before feature-freeze?

[mbauman]

Yes, I am unable to work on this myself in time to hit the feature freeze. If you'd be able to resurrect and take this over I'd be very grateful.

[timholy]

I'm a little skeptical that I'll have time either; #25050 has my own priority, given that I know we want that but there's some chance we'll look at this and decide we don't want it. But time permitting I'll see what I can do.

[andyferris]

Just curious about the status here, and if this is something we still want?

[StefanKarpinski]

It is. Unfortunately, @mbauman is on vacation until January – we may need to allow a few changes after the alpha, unless someone else gets this done before then.

[andyferris]

I started looking into this on ajf/deprecatemultivaluenonscalarindexedassignment. So far it's rebased and switches to a (maybe bad?) implementation using the newer broadcast functions. Still got to fix the usage of .= in array construction as mentioned in the OP.

[timholy]

I'll take a look at some point at the broadcasting, but FYI we may change the API yet again. If we merge some combination of #24992 and jn/lazydotfuse then I don't think we'll need to exempt broadcasting from the 1.x API freeze.

[mbauman]

Thanks for the rebase, @andyferris! I'm back and this is my top priority right now. I'm getting back up to speed with all the changes in the past month and it's nice to be able to pick up on your branch.

[andyferris]

(@mbauman Sorry I didn't finish it - Christmas happened - but at least it's a start).

[Sacha0]

Why add these ::Number qualifications?

[mbauman]

Otherwise we don't hit the deprecated method. Eventually I think I'll need to add a deprecated method specifically for sparse matrices, but this got me a bit closer to getting things passing.

[Sacha0]

Cheers, thanks :). Exciting motion!

[JeffBezanson]

Bump. Adding to milestone.

[oschulz]

Does this mean that

A[a:b] = B::AbstractArray

would become illegal, but that

A[a:b] = x::Number

would still be allowed? So also

A[a:b] = A[a:b]

would no longer be allowed? That strikes me a weird - since A[a:b] returns an array, shouldn't it also be allowed to assign an array to it (but not necessarily a scalar). I always thought this to be very elegant.
If I can write A[a:b] == A[a:b], I should be able to write A[a:b] = A[a:b] too.

I also worry about performance implications - AFAIK, this kind of setindex! used optimized code for the basic Array. Also, A[:, n] = X was a common high-performance way to overwrite array columns without memory allocation (no views necessary). This change may affect quite a bit of low-level code in array packages. Of course one can use copyto!, but that's less generic.

[mbauman]

Yes, that's correct. Right now in the indexing expression A[I...] = X, we know when I selects multiple indices for arrays based upon the very limited set of supported index types. We don't know whether X should be treated as a single element or if its many elements should be unpacked since arrays can store nested arrays. This dual nature makes it a trap to try to assign a single x to many locations in A. For an example of where this was a problem, take a look at #22747.

This PR is paving the way towards a future where we can treat the RHS of the A[I...] = … expression as a single element. Always. I... can continue to select many indices, but we will simply assign the RHS to every single one of those indices. No "unpacking" will be performed.

Now, I did also consider going the other way. That is, we could demand that the syntax A[I...] = X always unpacks the many elements of X into the many locations in A, and deprecate the meaning where it assigns the same RHS to many locations. That would solve the same problem, and it would seem that the syntax A[I...] .= x is just as capable of broadcasting a scalar element to many locations. And you're right, it's not terribly obvious that there's a definite right choice here. I chose the current design in this PR for a confluence of reasons:

• As I wrote it, A[I...] .= x isn't the correct deprecation. You must "protect" the value x in a Ref(x) or (x,) to ensure that it is indeed always treated as a single element. That's just broadcasting, but if we're choosing to privilege one of these non-scalar operations I'd like to privilege the one that's either more fundamental or harder to otherwise write.
• I also find the simplicity of always treating the "value" that gets passed to setindex! as a single element quite compelling.
• As I was making this change, I found that there were a number of deprecation changes where the RHS could easily participate in complete in-place fusion, resulting in a big performance win. That's not an advantage in the single-element case.
• Finally, I have an eye on fully detangling these scalar vs nonscalar operations in the future in a way that could live alongside support for dictionaries and other collections. This is enumerated in What to do about multivalue setindex!? #24086 (and specifically What to do about multivalue setindex!? #24086 (comment)). The dots "flag" which parts of the expression are potentially containers that should participate in broadcasting. I don't think we'd ever require dots for non-scalar indexing for arrays, but if we ever support them for other containers it should be done in a consistent manner.

I've very carefully transformed all the performance specializations on setindex!(A,v,I...) to broadcast!(::typeof(identity), ::typeof(view(A, I...)), v). The fallback, completely general broadcast! implementation has a check to see if no broadcasting is required… and in that case it uses copyto!. In such a memory-based operation, the SubArray wrapper doesn't really amount to a significant portion of the time. On the left is master setindex! vs. this branch's broadcast! on the right:

julia> for n in 2 .^ (0:16)             |  julia> for n in 2 .^ (0:16)
           A = rand(n, 10)              |             A = rand(n, 10)
           v = rand(n)                  |             v = rand(n)
           @btime (for i in 1:10;       |             @btime (for i in 1:10;
                       $A[:, i] = $v;   |                         $A[:, i] .= $v;
                   end)                 |                     end)
       end                              |         end
  104.331 ns (0 allocations: 0 bytes)   |    98.043 ns (10 allocations: 480 bytes)
  108.791 ns (0 allocations: 0 bytes)   |    98.518 ns (10 allocations: 480 bytes)
  121.303 ns (0 allocations: 0 bytes)   |    104.465 ns (10 allocations: 480 bytes)
  119.218 ns (0 allocations: 0 bytes)   |    99.408 ns (10 allocations: 480 bytes)
  121.606 ns (0 allocations: 0 bytes)   |    112.935 ns (10 allocations: 480 bytes)
  167.481 ns (0 allocations: 0 bytes)   |    116.623 ns (10 allocations: 480 bytes)
  199.685 ns (0 allocations: 0 bytes)   |    171.674 ns (10 allocations: 480 bytes)
  251.065 ns (0 allocations: 0 bytes)   |    234.525 ns (10 allocations: 480 bytes)
  414.050 ns (0 allocations: 0 bytes)   |    356.610 ns (10 allocations: 480 bytes)
  795.935 ns (0 allocations: 0 bytes)   |    1.013 μs (10 allocations: 480 bytes)
  1.509 μs (0 allocations: 0 bytes)     |    2.890 μs (10 allocations: 480 bytes)
  3.510 μs (0 allocations: 0 bytes)     |    5.350 μs (10 allocations: 480 bytes)
  8.795 μs (0 allocations: 0 bytes)     |    12.389 μs (10 allocations: 480 bytes)
  15.226 μs (0 allocations: 0 bytes)    |    28.469 μs (10 allocations: 480 bytes)
  57.774 μs (0 allocations: 0 bytes)    |    56.630 μs (10 allocations: 480 bytes)
  119.952 μs (0 allocations: 0 bytes)   |    112.759 μs (10 allocations: 480 bytes)
  253.679 μs (0 allocations: 0 bytes)   |    240.559 μs (10 allocations: 480 bytes)

[oschulz]

Now, I did also consider going the other way. That is, we could demand that the syntax A[I...] = X always unpacks the many elements of X into the many locations in A, and deprecate the meaning where it assigns the same RHS to many locations. That would solve the same problem

IMHO, that would be the sane solution. I think it's completely unnatural, and surprising to the user, if A[a:b] retrieves multiple different values, but A[a:b] = ... treats it's RHS as a scalar and tries to assign it to all array elements in the range. Could we not reconsider this? This would be really, really asymmetric behavior. Array indexing is such a fundamental thing in Julia - languages tend to get judged on inconsistencies like this (deserved or not), and with 1.0 in sight ...

Think about this:

A[:] = x::eltype(A)

would be legal, even though it results in

A[:] != x

but

A[:] = B::typeof(A)

would be illegal, even though it would result in

A[:] == B

if still allowed. To interpret assignment and comparison so differently, that's quite weird, isn't it? I don't think this would make Julia appear elegant.

But if we really, really have to deprecate A[:] = B::typeof(A), then please let's deprecate A[:] = x::eltype(A) as well - the latter one is the inconsistent one, semantically. I understand there may be technical reasons to keep A[:] = x - but really, that's just a kind of hack, then, and in Julia v1.0, no less ...

I've very carefully transformed all the performance specializations [...]

Ah, sorry, didn't see that - thanks, that's really great!

One worry, though - did you benchmark this multi-threaded, on a modern CPU with a decent number of threads/cores (> 20/10)? In my recent tests, views are still (current state of v0.7) very often heap allocated, and frequent global heap locks kill multi-threading performance to an amazing degree. I know Julia is supposed to able to stack allocate views in the future (and maybe already now, sometimes?), but we don't seem to be there yet, and may not be for a while - though I'd be more than happy to learn otherwise.

[StefanKarpinski]

I think it's completely unnatural, and surprising to the user, if A[a:b] retrieves multiple different values, but A[a:b] = ... treats it's RHS as a scalar and tries to assign it to all array elements in the range.

A[a:b] does not return multiple values, it returns a single object that is a collection of values.

[oschulz]

A[a:b] does not retrieve multiple values, it returns a single object that is a collection of values.

Sorry, yes, that's what I meant to say. So it seems completely unnatural to me that A[a:b] should return an object that is a collection of values, but that A[a:b] = ... would assign an object that is a single value.

And, like i mentioned, it would make assignment and comparison semantically different. Except for exotic types with some special behavior, the natural expectation is that a successful LHS = RHS results in LHS == RHS.

[StefanKarpinski]

Sorry, yes, that's what I meant to say. So it seems completely unnatural to me that A[a:b] should return an object that is a collection of values, but that A[a:b] = ... would assign an object that is a single value.

It doesn't assign an object that is a single value, it assigns an object that is a collection of values, but a single object nevertheless.

[JeffBezanson]

@oschulz I think you've made a pretty good case. Something else smells a bit odd here --- we're saying it's inconsistent for A[:] = x to repeat scalar x but copy array x, and yet A[:] .= x does exactly the same thing. The idea of broadcasting is that it allows shapes to be mismatched in a certain way. Arguably, without broadcasting the shapes just have to match exactly.

Repeating a scalar is inherently broadcasting and so should require .=, but copying same-shaped arrays perhaps should not.

[oschulz]

but that A[a:b] = ... would assign an object that is a single value.
It doesn't assign an object that is a single value, it assigns an object that is a collection of values, but a single object nevertheless.

Uh, sorry, now I'm confused - maybe I misunderstood: I thought this PR was about deprecating A[range] = B, with B being a single object that's a collection of values (which I find natural), while A[range] = x, with x being a single object that's not a collection (which I find unnatural) would stay legal?

[oschulz]

@JeffBezanson : Repeating a scalar is inherently broadcasting and so should require .=, but copying same-shaped arrays perhaps should not.

Right, I fully agree. We deprecated this kind of implicit broadcasting in so many other places (like deprecating A + x::eltype(A) in favour of A .+ x, so why would we make it the only allowed behavior of array assignment?

[JeffBezanson]

Of course, the corner case is A[1] = [2,3], where you want to make A into [[2,3], ...], but this might also be a shape mismatch where the RHS should have 1 element. Maybe we can live with that?

[oschulz]

Of course, the corner case is A[1] = [2,3], where you want to make A into [[2,3], ...], but this might also be a shape mismatch where the RHS should have 1 element. Maybe we can live with that?

But 1 is not a range, so isn't this completely different from A[a:b] = ...? I thought A[1] = [2,3] should always be interpreted as trying make A into [[2,3], ...], and it's legality would depend on the element type of A.

[oschulz]

Here's my proposal - let the rules/semantics for implementations of setindex! be:

• A[I...] = X should leave A in the same state as view(A, I...) .= X. No matter whether the indices are integers, ranges/arrays or colons (even works for single integer indices like in view([1,2,3], 1) .= 42).
• A[I...] = X should result in A[I...] == X (for element types with well-defined equality behavior).

This is, IMHO, simple and elegant. It should also clearly define RHS shape requirements, etc.

It would mean, of course, that A[I...] = x::eltype(A) would become deprecated.

[mbauman]

That's tempting enough to try it the other way. I definitely don't think both of these behaviors can stand, but I think we really only need to deprecate one or the other.

[oschulz]

Thanks for considering this, @mbauman - I realize you put a lot of work into this already. I'd be perfectly happy with deprecating the one-to-many assignment (and keeping the many-to-many).

Elegance of the language aside, maybe this would also break less user code? It's hard to judge, of course, but I'd guess things like A[:, i] = B::Vector are more common than A[:, i] = x::Number, in practice. And for A[:] = x::Number there was always fill!, hopefully most people used that.

Hm, here's an idea: Why don't we use fill!(A, x, idxs...) to replace setindex!(A, x, idxs...) for one-to-many assignment? Wouldn't that be very natural?

[andyferris]

Cool. I agree with @oschulz with the logic that the LHS should be == to the RHS after non-scalar setindex using = (the forth statement, below). Just briefly touching on future support for arbitrary dictionary keys (because that was alluded to above), I think we might eventually need:

A[i]          # always scalar getindex
A.[Inds]      # returns a collection with the same shape / keys as `Inds`
A[i] = x      # always scalar setindex!
A.[Inds] = B  # After this, `A.[Inds] == B`. `B` always must have the same shape / keys as `Inds`
A.[Inds] .= x # Broadcast the (possibly scalar, possibly not) RHS over the indices `Inds`

To me the end goal is to replace (so we can apply this to dictionaries) non-scalar getindex and setindex with some syntactically distinct "getindices" and "setindices!" which I've written with a .[] operator above. But in the meantime we can implement the above for arrays without that .[] operator, which AFAICT is exactly @oschulz suggestion. (For dictionaries I'm hoping we can introduce .[] in v1.x, and later consider deprecating the current non-scalar getindex/setindex in v2.0 for consistency between arrays/dicts if we want).

[oschulz]

@andyferris, I like your long term plan, that would be elegant.

I think it's important though that we do this at point where it can be done without any temporary (heap) allocation of views at all. But if we deprecate the one-to-many setindex right now, and keep the many-to-many for now, we have time, right?

@mbauman, do you think A[range] .= scalar_value will already do now for all use cases (you mentioned some technical problems), or should we extend fill! to support multiple indices for now? If we go for @andyferris long-term plan, we'd have to deprecated it again later, of course.

One question: What would happen to other functions like splice! and deleteat! that currently allow for passing index ranges (see @JeffBezanson's remarks in #26326)? We should be consistent across the board, if we can.

[mbauman]

Removing this "dual" behavior is most awkward and painful, of course, in places that depend upon it. Notably, this is mapslices and concatenation. Both fill in elements or "splat out" arrays. Naively deprecating either of these behaviors to broadcast is wrong, since it will also try to splat out tuples and other broadcastables.

I would absolutely love to propagate this change through to concatenation and deprecate mapslices in favor of map(f, julienne(…)), but unfortunately I think it's too late in the game for such huge changes… especially when we're still not satisfied on what "broadcastable" means.

For now, I'm trying to get by without the fill! method you proposed as a way of examining what our current tools provide… and trying to keep new features to a minimum.

Regarding the other array tools that allow either an integer index or a collection of indices, I don't think they're nearly as problematic. This one is just particularly horrible because A[2:3] = A[1] will sometimes work and sometimes fail, depending upon what A[1] returns.

I also don't think that we'll ever deprecate the array's A[:, :] methods to A.[:, :]. As such, we'll also never need to deprecate A[I…] .= x. For the full list of reasons why I don't think this syntax can ever be a full replacement for our current set of features, check out #22858 (comment) and my subsequent comment.

[oschulz]

Thanks for the explanations, Matt!

[mbauman]

Thanks for catching the fact that I did this backwards! As I work my way through the opposite set of deprecations, I'm more and more convinced that this is the right direction.

[oschulz]

Thanks for doing this, Matt!

[andyferris]

I don't think this syntax can ever be a full replacement for our current set of features

It certainly can't, since getindices for AbstractDict{Bool} can't support both multiple-getindex and "boolean indexing". At least for dictionaries, we'd need a different function for that...

[JeffBezanson]

Replaced by #26347