Support Bisection for ROCSOLVER - #281
Conversation
|
|
||
| function gesvdx!(::LAPACK, A, S, U, Vᴴ; kwargs...) | ||
| YALAPACK.gesvdx!(A, S, U, Vᴴ; kwargs...) | ||
| _complete_svd_basis!(U, Vᴴ, length(S)) |
There was a problem hiding this comment.
This might have to go inside of the YALAPACK implementation, I think that one checks that the rows/columns of V,U are not larger than minmn:
MatrixAlgebraKit.jl/src/yalapack.jl
Lines 2251 to 2252 in 9cd1f86
There was a problem hiding this comment.
I'm quite confused actually why the tests are passing, it seems like that shouldn't be happening 😕
There was a problem hiding this comment.
Wait, I'm confused about what's confusing
There was a problem hiding this comment.
Me too, the checks on the number of column of U and the number of rows of Vᴴ are inequality checks, so more columns/rows then necessary are allowed:
MatrixAlgebraKit.jl/src/yalapack.jl
Line 2238 in 9cd1f86
MatrixAlgebraKit.jl/src/yalapack.jl
Line 2247 in 9cd1f86
There was a problem hiding this comment.
Ah yes this is again for svd_full where we want size(U, 2) to be m, but I should explicitly check it's not > m also.
There was a problem hiding this comment.
Does this mean we are now allowing cases like size(A) = (m, n) with m < n, size(U, 2) = k with m <= k <= n as a valid input?
There was a problem hiding this comment.
Oh yea I see, sorry, it should indeed not be >=. The range == 'I' case throws this stuff off, it's annoying
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
3 participants
Moving this out of #275 since it's not really batching.
Wrapped the
Bisection()algo for ROCm, and added some helper functions insrc/implementations/svd.jlso that it can also be used forsvd_full!. I put the helpers there since NVIDIA might add a similar algorithm someday, or we could also allow LAPACK users to combine the two. Can remove that if we don't want it. Also added tests.