Skip to content

Implement SafeSVD algorithm - #145

Closed
VictorVanthilt wants to merge 3 commits into
mainfrom
vv-safeSVD
Closed

Implement SafeSVD algorithm#145
VictorVanthilt wants to merge 3 commits into
mainfrom
vv-safeSVD

Conversation

@VictorVanthilt

Copy link
Copy Markdown
Member

When performing a singular value decomposition it can be beneficial to try to use LAPACK_DivideAndConquer for its speed but if it fails, use the more stable but slower LAPACK_QRIteration algorithm.

Thereto I implemented a very minimal SafeSVD algorithm that makes an extra copy of the input data, tries LAPACK_DivideAndConquer on the copy, and if it fails performs LAPACK_QRIteration on the original data.

Feel free to comment on what other magic functions I need to add to make things like Mooncake happy.
I haven't added any tests yet, does anyone know a pathological matrix that will fail with YALAPACK.gesdd! but succeed with YALAPACK.gesvd!?

@kshyatt

Copy link
Copy Markdown
Member

You shouldn't need to add anything for mooncake btw, the algorithm is marked as a non differentiable type

@kshyatt

Copy link
Copy Markdown
Member

BTW can this also support the AMD algorithms ROCSOLVER_DivideAndConquer and ROCSOLVER_QRIteration?

@VictorVanthilt

VictorVanthilt commented Jan 6, 2026

Copy link
Copy Markdown
MemberAuthor

any reason why DivideAndConquer and Bisection are left out of:
const ROCSOLVER_SVDAlgorithm = Union{ROCSOLVER_QRIteration, ROCSOLVER_Jacobi}?

@kshyatt

kshyatt commented Jan 6, 2026

Copy link
Copy Markdown
Member

Oh I got it mixed up, neither gpu library has gesdd yet. So no worries!

It might nevertheless be nice to do this in a way that's easily extensible to non LAPACK backends for the future, in case either does add gesdd

@codecov

codecovBot commented Jan 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 46.66667% with 8 lines in your changes missing coverage. Please review.

Files with missing linesPatch %Lines
src/implementations/svd.jl46.66%8 Missing ⚠️
Files with missing linesCoverage Δ
src/MatrixAlgebraKit.jl100.00% <ø> (ø)
src/interface/decompositions.jl73.68% <ø> (ø)
src/implementations/svd.jl74.39% <46.66%> (-18.28%)⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lkdvoslkdvos left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the remark about the GPU implementations, it might really make sense to just spend the time and make it a bit more generic:

struct FallbackAlgorithm{A <:AbstractAlgorithm, B <:AbstractAlgorithm} <:AbstractAlgorithm
alg1::A
alg2::Bendfor f in (:svd_compact, :svd_full)
f! =Symbol(f, :!)
# hijack non-mutating function to avoid double-copy@evalfunction$f(A, alg::FallbackAlgorithm)
Ac =copy_input($f, A)
out =initialize_output($f!, A, alg.alg1)
returntry$f!(Ac, out, alg.alg1)
catchcopy!(Ac, A)
$f!(Ac, out, alg.alg2)
endend# implement mutating function to make copy@evalfunction$f!(A, out, alg::FallbackAlgorithm)
Ac =copy_input($f, A)
returntry$f!(Ac, out, alg.alg1)
catch$f!(A, out, alg.alg2)
endendend

I think our implementations of the other functions might even be generic enough to make this "just work".

This also needs some additional tests, and possibly some select_algorithm overloads or additional constructors to make this a bit more convenient

@lkdvos

Copy link
Copy Markdown
Member

Possibly might find some failure cases here: Reference-LAPACK/lapack#672 (comment)

@Jutho

Jutho commented Jan 6, 2026

Copy link
Copy Markdown
Member

While I am typically among the first to consider possible generalisations, in this case I am not sure if this is really the right approach. In fact, I would go further and say that I think this should probably be called something like LAPACK_SafeSVD (but could become the default for BLAS matrices), and would like to write a specialised YALAPACK call for this, that doesn't really throw the error but simply does the LAPACK style info checking, and possibly reuse some of the work space between the gesvd! and gesdd! calls.

@kshyatt

Copy link
Copy Markdown
Member

LAPACK_SafeSVD also works from my PoV. I'm just against giving something a generic name that's really LAPACK only.

@JuthoJutho mentioned this pull request Mar 12, 2026
@lkdvoslkdvos closed this Mar 15, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@VictorVanthilt@kshyatt@lkdvos@Jutho