Skip to content
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

Inconsistency of dimensions in QR decomposition #719

Open
alois-bissuel opened this issue Jul 13, 2018 · 4 comments
Open

Inconsistency of dimensions in QR decomposition #719

alois-bissuel opened this issue Jul 13, 2018 · 4 comments

Comments

@alois-bissuel
Copy link
Contributor

Doing a reduced QR decomposition may give different dimensions when calling qr.reduced.justR(A) instead of qr.reduced(A).r

Explanation:
If M is an m x n matrix, and m<n, then M=QR where Q is m x m and R is m x n.
In doQr, is skipQ is true, then upperTriangular(A(0 until mn, ::) is called to output R, which implicitly outputs a square n x n matrix instead of a rectangular m x n matrix. If skipQ is set to false, then the code correctly erases the lower triangular part of the matrix A (which is the array on which Lapack is called).

@alois-bissuel
Copy link
Contributor Author

I have created a small commit to fix this in my forked version of Breeze : see pull request here alois-bissuel#1
Sorry if it is not the right way to do it, it is my first tries on GitHub !

@dlwh
Copy link
Member

dlwh commented Nov 30, 2018

hi alois, sorry for being so slow. The best way to do this is to open a PR against scalanlp/breeze, not your own repository.

thanks

@alois-bissuel
Copy link
Contributor Author

Sure, will do!

@alois-bissuel
Copy link
Contributor Author

Done, see PR 736.

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

No branches or pull requests

2 participants