WIP/RFH: Deprecate generalized linear indexing #20040
Conversation
ararslan
added
deprecation
Sacha0
force-pushed
the
mb/deprecate-17440
branch
from
32b7b4a
to
fa31b38
Compare
|
(Apologies, accidentally pushed directly to this branch. Should be corrected now. Best!) |
|
Re: @andreasnoack in #20043:
Yes, this deprecates both behaviors. You now need to specifically opt in to using vectors as matrices. You can still share the same kernel. See, for example, how I changed triangular.jl. I'm still not 100% on this change; it doesn't really simplify implementations. But it does tighten up our behaviors and make vectors more different from matrices, and it does allow catching a few more mistakes on the user side. Of course, trailing singleton dimensions aren't nearly as error-prone, since they're bounds checked and must be |
| ### Multiplication with triangle to the left and hence rhs cannot be transposed. | ||
| for (f, g) in ((:*, :A_mul_B!), (:Ac_mul_B, :Ac_mul_B!), (:At_mul_B, :At_mul_B!)) | ||
| @eval begin | ||
| function ($f)(A::AbstractTriangular, B::$mat) | ||
| ($f)(A::AbstractTriangular, B::AbstractVector) = ($f)(A, reshape(B, Val{2})) |
There was a problem hiding this comment.
If I read this correctly, I believe this changes the behavior such that the rhs is now promoted to a matrix, right?
There was a problem hiding this comment.
Echoing @andreasnoack, these changes result in a matrix being returned rather than a vector, e.g.
julia> UpperTriangular(rand(3, 3)) * rand(3)
3×1 Array{Float64,2}:
0.978125
0.687285
0.568977Best!
|
@mbauman Thanks for explaining. I didn't read your original post carefully enough and I didn't anticipate how much this behavior was actually used in Base. Often you'll have to implement the change to realize that. |
|
Ah, you know what? Let's do this as bounds-checking instead of changing the implementation. I'll need some help on the C side, but it'll mean that we can separate these behaviors and make this a simpler fix. |
Sacha0
force-pushed
the
mb/deprecate-17440
branch
from
81f5645
to
fa31b38
Compare
Do you mean (1) sequentially deprecate partial linear indexing and indexing with trailing singleton dimensions (as opposed to simultaneous deprecation) or (2) only deprecate partial linear indexing at this time? (Trying to determine how best to help.) Thanks! |
|
Yeah, I was thinking about just doing partial linear indexing first, and then seeing if there's still a stomach for trailing singletons. I'll need some help with the codegen modifications for Array. |
|
Closed in favor of the smaller, simpler, and more incremental process in #20079. |
This deprecates generalized linear indexing, meaning that after this patch, the only two ways to index into an
AbstractArray{T,N}is either with one index or withNindices. This removes support for what I've called "partial linear indexing" (e.g.,rand(2,3,4)[1, 7]) as well as trailing1indices (e.g.,(1:10)[2,1,1]).Note that until #18457 is merged, these deprecated definitions need to occur before the real definitions in order to get the method sorting right.
There are tons of deprecation warnings, and I need help in chasing them all down. Please feel free to push to this branch or open PRs targeted at it. My computer is slowly chugging through the test suite and saving the warnings, but here's an initial list:
And then there are a bunch of places within the standard library that need refactored.
I'll post the full depwarn log once I finally get to the end of the test suite, but here's an initial list. Here is the full log from make test. I modified depwarn to try and print where it thought the deprecation was coming from in an easy-to-grep-for way; that's how I've populated the list below. So if something there doesn't make sense, you can refer to the full log and find the full stack trace for it. While there are thousands of warnings, it's not as bad as I thought it might be… It seems like many come from the same areas: