search

scalanlp / breeze

Breeze is/was a numerical processing library for Scala.
https://www.scalanlp.org
Apache License 2.0
3.45k stars 692 forks source link

Inconsistency of dimensions in QR decomposition #719

Open alois-bissuel opened 6 years ago

alois-bissuel commented 6 years ago

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 commented 6 years ago

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

dlwh commented 5 years ago

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 commented 5 years ago

Sure, will do!

alois-bissuel commented 5 years ago

Done, see PR 736.