Taking transpose deprecation messages seriously

[mbauman]

Here's a shot at simplifying the transpose deprecation message. cc @Sacha0.

[Sacha0]

Thanks Matt! :) Though I agree this deprecation message should be tightened up, about this revision I share the misgivings expressed in #25463 (comment). Specifically, this revision does not provide enough information to facilitate functionally equivalent rewrites in most cases. Perhaps something similar to the iteration below, which provides that information while remaining comparably brief, would do the trick? Best!

The syntax x.' for transposition is deprecated, use transpose(x) or copy(transpose(x)) instead. The transpose function now always returns a lazy view into its argument, avoiding intermediate allocations. This means that modifying the transpose will also modify the original array. For vector v in v.', transpose(v) is always the functionally equivalent rewrite. For matrix A in A.', the functionally equivalent rewrite depends upon context: In *, \, and / expressions such as A.'*B, A.'\B, and B/A.', transpose(A) is the functionally equivalent rewrite. In almost all other cases, copy(transpose(A)) is the functionally equivalent rewrite (though over time transpose(A) should become superior in most such cases as well).

[mbauman]

I think we have different philosophies. I don't think we need to worry ourselves about giving folks the exact transformation to choose which replacement to use. I'd rather just give them precise information about how to choose.

The longer this message gets, the less likely it all gets read. My proposal in this PR is already verging on too long for my tastes.

[Sacha0]

I think we have different philosophies. I don't think we need to worry ourselves about giving folks the exact transformation to choose which replacement to use. I'd rather just give them precise information about how to choose.

I would say that I agree with the last sentence, but it seems we are nonetheless not on the same page, so let's iterate :).

As a user upgrading my code to a new release, I want each deprecation warning to tell me clearly and unambiguously what to write to make my code work precisely as it did before. I do not want to guess at or have to experiment with rewrites, nor do I want the rewrite to change the meaning of my code insofar as possible. I want to upgrade with minimal time, effort, and frustration, and move on with whatever I am trying to accomplish.

From that perspective, the proposed message leaves something to be desired. Specifically,

Use transpose(x) in cases where the result is an intermediate computation (like as a replacement within x.' * A), or add a call to copy to ensure the result is independent from its argument.

would often lead me astray where the result is an intermediate computation but does not appear directly in a *, /, or \ expression. For example, rewriting A + A.' as A + transpose(A) leads to type and dispatch changes, such as A + A.' for A::SparseMatrixCSC yielding Array rather than SparseMatrixCSC as in #25331. Similar gotchas would exist for a host of operations with the proposed message's rewrite.

Hence my expansion of the quoted sentence above to

For vector v in v.', transpose(v) is always the functionally equivalent rewrite. For matrix A in A.', the functionally equivalent rewrite depends upon context: In *, \, and / expressions such as A.'*B, A.'\B, and B/A.', transpose(A) is the functionally equivalent rewrite. In almost all other cases, copy(transpose(A)) is the functionally equivalent rewrite.

to achieve

I'd rather just give them precise information about how to choose.

The suggestion I gave above could be pared down a bit further, e.g. to

For vector v in v.', use transpose(v). For matrix A in A.', the appropriate rewrite depends upon context: In *, \, and / expressions, use transpose(A). In almost all other cases, use copy(transpose(A)).

though beyond that I see little room for further reduction without leading users astray / into frustration. Thoughts? Best!

[mbauman]

Ah, yes, I see your point. And I imagine that folks will generally identify the more general lazy/copy issue themselves according to their own comfort and expertise. How's this for a final message?

The postfix .' syntax is deprecated. For vector v in v.', use transpose(v) instead. For matrix A in A.', use copy(transpose(A)) instead, unless A.' appears as an argument of *, / or \. In those cases, use transpose(A) instead.

[Sacha0]

How's this for a final message?

Looks great! Much thanks for your efforts Matt! :)

@StefanKarpinski,

and

[KristofferC]

I really think we should shorten it. Just a couple of words and a link to NEWS.md or something, where there can be a long description.

[StefanKarpinski]

💯 this deprecation message is ridiculous. I would never read it. Sorry, @Sacha0!

[Sacha0]

Sorry, @Sacha0!

No worries! :) As above I agree, and happily it seems we've converged on a much improved depwarn. This pull request should be mergeworthy after updating its text to #25491 (comment) I think, @mbauman? Best!

[mbauman]

I just pushed the version @Sacha0 and I converged on. Still a bit long, but a big improvement.

julia> [1].'
┌ Warning: The postfix .' syntax is deprecated. For vector v in v.', use transpose(v) instead. 
For matrix A in A.', use copy(transpose(A)) instead, unless A.' appears as an argument of *, / 
or \. In those cases, use transpose(A) instead.
└                                                                             @ nothing none:0

[ararslan]

Would it be possible to add some line breaks so that it prints more like

┌ Warning: The postfix .' syntax is deprecated. For vector v in v.',
│ use transpose(v) instead. For matrix A in A.', use copy(transpose(A))
│ instead, unless A.' appears as an argument of *, / or \. In those
│ cases, use transpose(A) instead.
└ @ Main REPL[1]:1

?

[Sacha0]

Looks great! :)

[stevengj]

It seems much clearer to me to write:

The postfix x.' syntax is deprecated. Use either transpose(x), which makes a "lazy" wrapper that acts like a transposed array without copying the data, or copy(transpose(x)) if you want to make a new transposed copy.

The differentiation between the vector and matrix cases in the deprecation message, while I understand the motivation, is just confusing to me.

[mbauman]

Yes, that's similar to my initial version, too. I think Sacha has a point, though: I think the message that details the exact transformation is better for novice users.

[KristofferC]

Would it be possible to add some line breaks so that it prints more like

I think the logging should do that automatically at some point.

[stevengj]

It’s precisely novice users who will think that the rules as stated, with all sorts of special cases, are insanely complicated.

Most users don’t need the exact transformation. They don’t need their code to be exactly equivalent to 0.6 in terms of making copies. They just want to know how to transpose something.

[mbauman]

They don’t need their code to be exactly equivalent to 0.6 in terms of making copies.

Absolutely true. But that's not the only consideration. We're not nearly close to the point where the transpose wrappers perform similarly to the original structures. Very trivial example:

julia> A = sprand(100000,100000,.000001)
100000×100000 SparseArrays.SparseMatrixCSC{Float64,Int64} with 9966 stored entries:
  ⋮

julia> @time sqrt.(copy(transpose(A)))
  0.003343 seconds (38 allocations: 1.832 MiB)
100000×100000 SparseArrays.SparseMatrixCSC{Float64,Int64} with 9966 stored entries:
  ⋮

julia> @time sqrt.(transpose(A))
105.450658 seconds (13 allocations: 74.506 GiB, 0.03% gc time)
100000×100000 Array{Float64,2}:
  ⋮