You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The functions eigh, svd, qr and lstsq are not well-defined as written.
Uniqueness lstsq
linalg.lstsq docs currently read:
“Returns the least-squares solution to a linear matrix equation”
but the solution needn't be unique whenever A has non-trivial kernel (e.g. A = 0).
Proposed solution:
Rewrite it as "Returns a least-square solution [...]"
Perhaps we want to add a note on how to handle these cases (e.g. the solution with smallest norm is chosen) so that this function is well-defined for every input matrix. Note that this would not allow for the implementation of this function via faster but less robust algorithms such as LAPACK's gels, which assumes that A is full-rank and performs a QR decomposition.
Uniqueness eigh and svd:
Eigenvectors (resp. singular vectors) of a generic matrix are only defined up to sign in the real case and multiplication by a scalar of norm 1 in the complex case
When the matrix has repeated eigenvalues (resp. singular values) the associated eigenvectors are just defined up to multiplication by a rotation (or multiplication by a unitary matrix in the complex case).
Proposed solution:
Change the wording from "Returns the eigenvalues and eigenvectors of a symmetric matrix" to "Returns an eigendecomposition of a symmetric matrix".
Change the wording of linalg.svd as well.
QR decomposition cannot be defined
The qr decomposition is just well-defined whenever the matrix has full column rank. In the singular case pivoting is necessary.
Proposed solution: The behaviour of this function should be marked as unspecified whenever the input matrix does not have full column rank.
Thanks for the detailed review @lezcano. Your proposed solutions for eigh, svd and qr all sound good to me, as does the first bullet for lstsq.
Perhaps we want to add a note on how to handle these cases (e.g. the solution with smallest norm is chosen) so that this function is well-defined for every input matrix. Note that this would not allow for the implementation of this function via faster but less robust algorithms such as LAPACK's gels, which assumes that A is full-rank and performs a QR decomposition.
I don't think we should add such a note. One principle we'd tried to adhere to in the API design is "don't make decisions that limit implementers in how they optimize for performance". So taking a decision on performance-robustness tradeoffs for lstsq doesn't sound optimal.
Pointing out the issue is good though - but I'd do it as a note like "if the solution is not unique, implementations may return different results".
For what is worth, I do not know of any implementation of lstsq that does not return the minimum norm solution. For example, all the LAPACK implementations do so. Note that even gels falls in this category as well, albeit under the assumption that the matrix is full-rank.
On a similar decision to this one, see the OP of the PR for linalg.matrix_rank. The OP says that, even though some implementations provide an hermitian boolean flag, this is not included in the specification because it denotes an optional optimisation. In the same way, the behaviour of gels could also be provided by the libraries via an optional flag full_rank=False or similar.
The functions
eigh
,svd
,qr
andlstsq
are not well-defined as written.Uniqueness
lstsq
linalg.lstsq
docs currently read:“Returns the least-squares solution to a linear matrix equation”
but the solution needn't be unique whenever
A
has non-trivial kernel (e.g.A = 0
).Proposed solution:
gels
, which assumes thatA
is full-rank and performs a QR decomposition.Uniqueness
eigh
andsvd
:Proposed solution:
linalg.svd
as well.QR decomposition cannot be defined
The
qr
decomposition is just well-defined whenever the matrix has full column rank. In the singular case pivoting is necessary.Proposed solution: The behaviour of this function should be marked as unspecified whenever the input matrix does not have full column rank.
cc @rgommers @mruberry
The text was updated successfully, but these errors were encountered: