Skip to content

Conversation

@shamazmazum
Copy link
Contributor

These functions returned incorrect values and could even crash lisp when working with row-major matrices.

Fixes #217

These functions returned incorrect values and could even crash lisp
when working with row-major matrices.
@shamazmazum
Copy link
Contributor Author

I am sorry that transpose! is not inplace now, but it's correct which is more important

@shamazmazum
Copy link
Contributor Author

I was thinking about matrix transposition. It seems it's not needed at all.

E.g. for QR decomposition you can do A^T = R^T Q^T and obtain A^T, R^T and Q^T by simply swapping dimensions and changing :row-major to :column-major. Matrix-matrix multiplication also has :transa and :transb flags to avoid transposition.

I guess, maybe you can merge this PR as a quick fix and then I can eliminate transposition in the next PR.

P.S. Inplace transposition of MxN matrices with M/=N is not trivial. I won't do this :)

@shamazmazum
Copy link
Contributor Author

I've tried to eliminate transposition in SVD decomposition. This works as follows:

  • If m is in column-major format, proceed as usual
  • If m is in row-major format:
    • fast-transpose it (create a new matrix with the same storage, layout changed to :column-major and dimensions reversed):
      m' = fast-transpose m
    • Obtain vt' s u' = lapack-svd m'
    • Calculate vt = fast-transpose vt' and u = fast-transpose u'
    • Return u, s, vt.

The outcome of this is that svd returns different (but correct) results for the same matrices in row-major and in column-major formats (the difference is in signs of elements in u and vt matrices).

All said is true for other non-unique decompositions (e.g. for eigenvalue decomposition: if there are many eigenvalues with the same absolute value, they may appear in different order).

Can this be allowed in magicl? If yes, there is no need to transpose anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

QR decomposition is broken

1 participant