add gla_householder_qr_null #132
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
kshyatt
force-pushed
the
jh/genericqrnull
branch
from
292087e to
8989b7b
Compare
|
Can we turn on the tests for this by getting rid of the |
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
|
So it seems there are still issues with the eigenvalue AD. I'll investigate this elsewhere. |
| (blocksize == 1) || throw(ArgumentError("Only blocksize = 1 implemented for GLA_HouseholderQR.")) | ||
| m, n = size(A) | ||
| minmn = min(m, n) | ||
| fill!(N, zero(eltype(N))) |
There was a problem hiding this comment.
| fill!(N, zero(eltype(N))) | |
| MatrixAlgebraKit.zero!(N) |
Alternatively we can also just import this, but this is probably slightly more consistent
lkdvos
left a comment
lkdvos
left a comment
There was a problem hiding this comment.
Looks good to me!
The only thing I was wondering is whether or not it would be easy/straightforward to share some of the code with the regular qr_null implementations, but I had a quick look and that doesn't really seem to be the case, so this is definitely good to go for me.
Left a small style comment
As requested by @kshyatt in #129 .