-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove (c)transpose definitions from operators.jl. Add them to string.jl #9170
Conversation
To clarify, special matrix types like |
I remember a discussion whether (c)transpose of a matrix needs to call (c)transpose on its elements, e.g to represent a block matrix as a matrix of matrices. Currently, transpose of a matrix does indeed call transpose on the elements, so if transpose of Any is not defined, transposing a matrix with custom types will not work. |
af083ef
to
96ba103
Compare
@Jutho I'm wondering how often you transpose a matrix of non-number or non-string elements. In any case, I think it would be okay to require that the behavior of |
Doing |
Would it cause too much inconvenience to require that users define the behavior of If it does, we could consider to restrict the recursive behavior of |
@Jutho and @StefanKarpinski, can I go ahead and merge this or are your comments blocking objections. |
I'm not sure what spurred this change. I recall convincing at least @JeffBezanson that it was ok for transpose and conj to default to identity on non-numeric types. |
It can be hard to remember old comments. Even when is's your own, #7244 (comment). I wrote the lines that I want to remove now, but at least I saw the potential issue. The issues #7244 and #6395 seem to be the relevant references here. I really don't think that it is a good idea to have methods defined for |
Perhaps |
Perhaps I'm being dense but I'm not seeing what the problem with the current definitions is. |
@mbauman The problem is that not all matrix-like types will be subtypes of AbstractMatrix @StefanKarpinski Could you elaborate why returning a wrong result is not a problem, i.e. returning the identity when transposing a matrix? Wouldn't a few no method errors be better than wrong results? |
The choices are
Now we prefer (2), since the type in question should just implement |
I was too probably too brief when I opened this issue because I didn't really think that two dimensional arrays of non-number and strings were transposed that often. The special However, this is not the main problem here. As mentioned before, I really don't like general fallbacks for
|
|
Maybe it's time to reconsider introducing the
In #987 we did not resolve what "cheap" means. Consider:
@andreasnoack can correct me here, but I think "things you shouldn't explicitly transpose" could be another useful criterion for defining a |
The most useful definition for a |
Maybe I am looking at this in the wrong perspective and any further explanation would be appreciated. I don't understand why it is okay to assume structure for julia> A = sub(sparse([1 0;1 1]), :, :)
2x2 SubArray{Int64,2,Base.SparseMatrix.SparseMatrixCSC{Int64,Int64},(Colon,Colon),0}:
1 0
1 1
julia> transpose(A) == A
true You generally seem to think of |
I have no problem with seeing the recursive |
I wonder if we can improve this by separating the |
@Jutho For linear algebra, I think that recursive transposing is the right thing to do and that using @StefanKarpinski Could you elaborate? I don't understand your comment. |
I'm not sure there was anything to my comment. I don't think what I was thinking of really helps. |
The definitions give silent errors for matrix types that don't define
transpose
whenever the*(A,B,C)
method is called. See the example below.