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

Allow QR decomposition for zero-row matrices. #1054

Closed
manuelbb-upb opened this issue Feb 23, 2024 · 2 comments · Fixed by JuliaLang/julia#53578
Closed

Allow QR decomposition for zero-row matrices. #1054

manuelbb-upb opened this issue Feb 23, 2024 · 2 comments · Fixed by JuliaLang/julia#53578

Comments

@manuelbb-upb
Copy link
Contributor

This came up in DynareJulia/FastLapackInterface.jl#39

Currently, Julia allows for QR decomposition of zero-column matrices by skipping the call to LAPACK with a simple conditional [in this line].(https://github.com/JuliaLang/julia/blob/923fe2d74d469d73f0285a9cd41c88f90edd04d1/stdlib/LinearAlgebra/src/lapack.jl#L456)

Zero-row matrices are not supported and give an argument error:

 LA.qr(zeros(0, 10))
 ** On entry to DGEQRT parameter number  3 had an illegal value
ERROR: ArgumentError: invalid argument JuliaLang/julia#3 to LAPACK call

Is there a reason for this?
The data type should support these matrices:

import LinearAlgebra as LA
A = Matrix{Float64}(undef, 0, 10)
T = Matrix{Float64}(undef, 0, 0)
qr = LA.QRCompactWY(A, T)

and qr is as I would expect it.

@dkarrasch
Copy link
Member

Would it fix the issue if we changed the condition to

n > 0 && m > 0

so that we go down the LAPACK path only in the LAPACK-valid cases ? If yes, would you be able to submit a PR, together with unit tests?

@manuelbb-upb
Copy link
Contributor Author

manuelbb-upb commented Feb 27, 2024

That would indeed be sufficient. I will prepare a PR until tomorrow evening.

Edit: Will habe to delay for one or two days, other things got in the way. This line has stood for 11 years, so it is not too urgent, I guess 😅

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 a pull request may close this issue.

2 participants