-
-
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
WIP:Add Transpose immutable #6837
Conversation
I like the general idea of lazy transpose. The tricky part is how do you propose to handle setindex, does writing into a transpose object change the original matrix? Guess you just document really clearly if transpose returns a view, and you use Properly supporting sparse here basically means introducing a SparseMatrixCSR type, transpose outputs the same data as the opposite type and the current sparse transpose implementation becomes the conversion operation between the two types. |
What does transpose mean for N>2? |
Since we want a transposed vector to be a row vector (as opposed to a row matrix) I guess thst the only reasonable definition of transposing higher dimensional arrays would be to swap the first two dimensions. But I agree that starting out with N <= 2 makes a lot of sense. Also, thanks for trying this out! |
I don't see a unique definition for |
Python Pandas defines a version of transpose for N=3 that allows arbitrary swapping of dimensions, which seems (to me) like a reasonable definition. Do LAPACK routines allow for arbitrary strides along each dimension, or only the major stride? |
Numpy's stock transpose ( |
+1 for reversing the dimensions. This basically gives us row major arrays. |
That's really my point, that for |
Reversing the dimensions is ok as long as Transpose{Vector} is treated like a row matrix the same way that Vector is treated like a column matrix. Making all of that consistent is going to take some effort. |
Reversing the dimensions would mean that the transpose of a column vector But I agree that transpose in more than two dimensions d On Wed, May 14, 2014 at 7:58 PM, Stefan Karpinski
|
@tkelman I/We better document this well, but the general shift to views will probably give some surprises in terms of breaking code, but hopefully also in faster code. @kmsquire LAPACK requires the storage to be contiguous along the major stride. BLAS allows arbitrary strides for vector arguments i.e. BLAS 1 & 2. @jiahao I'm not familiar with transposing arrays with I think the permutation vector approach can be combined with the covector behaviour. Although the permutation of But again, right now we only support transposing when Finally, I would be happy if someone with Scheme skills could have a look at the lines I have commented out in |
I think this is an excellent idea (I particularly like that I think it is worth considering making conjugate-transpose a distinct type, say |
I agree about conjugate transpose. If we're going to have |
There is one other issue with the Some possible solutions:
|
To follow up, I threw together a package for providing a convenient syntax for in-place operations, e.g.
will call |
While I like the general attitude of switching to views where possible, and also see the advantage of having a Transpose type for the matrix multiplication zoo, one might want to consider how to deal with the corresponding CTranspose type? Since this involves an additional complex conjugation, it is not simply a view. If the CTranspose type is used outside the context of matrix multiplication, when will the conjugation be applied? Every time the user accesses a specific entry of the CTranspose object. If this happens many times, it could become much more inefficient then just computing the complex conjugated matrix or Hermitian conjugated matrix once. |
The CTranspose type isn't entirely necessary. ctranspose of real arrays could apply the lazy Transpose construction, while ctranspose of complex arrays could do the actual eager ctranspose. Of course, that introduces a problem generic programming in that for some array types setindex!{M<:Matrix}(t::CTranspose{M}, v, i, j) = (t.array[j,i] = v') |
I would be inclined to think that negating the imaginary part of a complex |
Yes, I suspect that's true. |
That's probably true indeed. I just started wondering about this since I noticed that the current code for |
I was playing around with this idea over the weekend, and while it makes things drastically simpler in general ( Whereas As a result, we would probably need to split
|
To follow up, here's my attempt: 1919d59 (branched from @andreasnoackjensen's code). It defines separate types It almost halves the number of lines in |
@simonbyrne – that seems like a fairly sane, targeted approach. I guess it feels a bit limited not to make the transpose construction more general but since we a) can't agree on what higher order transpose should mean and b) it causes all kinds of generic programming problems as you outlined, maybe the targeted approach for just vectors and matrices is best. It also occurs to me that we could use Array{T,-n} as the actual transposed types rather than using a wrapper. Arrays with negative dimensionality would count their dimensions downward instead of upward. Then Array{T,-1} could be covector, Array{T,-2} a "comatrix", and so on. A comatrix would basically be a row-major matrix. |
@StefanKarpinski That's perhaps worth considering, though it would mean that every |
We should be careful about the idea of The |
I'm not a fan of |
I generally agree that the Transpose and Covector parametric types are probably a safer way to go.
|
One more brainteaser: what should be the (conjugate)transpose of a matrix of matrices? Should we also transpose the individual entries? |
ctransposeblock!(B,A,m,n-newn,offseti,offsetj+newn) | ||
end | ||
return B | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boy, there's a lot of near-duplicated code here. I'd really prefer a generic implementation in which you passed transpose
or ctranspose
as an argument.
Nice! How did the benchmarks work out? Naturally, I'm only concerned about performance for element-by-element access patterns. Does LLVM hoist the |
Thanks for the comments. I'll look into them and there is still work to do here and more benchmarks are needed to detect if the new methods are efficient. Right now, I wanted to direct the attention to the compile time issue which is quite visible in https://travis-ci.org/JuliaLang/julia/jobs/126553560#L1648 My feeling is that unless something can be done with the compile time this change is simply not feasible. |
Yeah, I noticed that. Quite a bummer. |
Bump. Needs to happen soon to go into 0.6. |
Is this still blocked on a compiler performance problem? Is there an issue for it? |
I don't think the compile time is a blocker anymore. The huge number of method compilations could be avoided by making sure that elements in |
That sounds promising. It would be great if this could get done in time for the 0.6 feature freeze at end of year. |
I said it in the line comments, but I would prefer that we punt entirely on conjugation here. I'm hoping that between my I feel that having the conjugation more modular should be a conceptual advantage (and probably better for developers). For instance, you can have |
I believe we're aiming for RowVector in 0.6, and TransposedMatrix later, so changing milestone. |
@andreasnoack Are we likely to be able to make this by feature freeze? |
A much more up-to-date version here is in #23424. |
Yes. This is not relevant anymore. The current efforts are in #23424. |
Here is a sketch of how a
Transpose
type could work. Before continuing with redefining all the methods to the new type, I would like to hear what people think about it. Basically, I am just introducing the immutableand although there is an
N
there, this proposal deals with arrays of dimension one and two. That is what is possible to transpose right now and I think it is beyond the scope of this proposal to handle higher dimensions.This is basically a kind of lazy transpose which we can use for dispatch to optimised methods. If the idea is accepted, we can remove all the
Ax_mul_Bx!
methods and just havemul!
and similarly forldiv
/rdiv
, see #5332.With this proposal
transpose(Vector{T})
is aVectorTranspose{T,Vector{T},false}
, which is still a one dimensional thing, but with this we can havewhich was discussed in #4774. Another useful consequence would be that we can evaluate something like
A*B*C'
more efficiently. Right nowB*C'
callsA_mul_Bc
without actually formingC'
, butA*B*C'
calls*(A,B,ctranspose(C))
so for more complicated matrix expression there is more copying than necessary.cc: @jiahao, @toivoh