SVD via Golub-Kahan-Reinsch with Givens rotations - 100% convergence, PCA-ready - #111
Open
cyancirrus wants to merge 32 commits into
Open
SVD via Golub-Kahan-Reinsch with Givens rotations - 100% convergence, PCA-ready#111cyancirrus wants to merge 32 commits into
cyancirrus wants to merge 32 commits into
Conversation
…ing, should explore constants but unsure use-case
Francis decomposition
…es for loops n what not, also removed unused vars
…ts, correcting as to be clear that these were tuned for f32s
…2x2 which please don't call cpx with a 2x2, just do the math, but just in case it is called
…ost of the comments
pull request comments
…version with the fast version
…version with the fast version
…version with the fast version
…d the tests as those are now private, which cleans a bit of the files for symmetric/complex and added a note
…d the tests as those are now private, which cleans a bit of the files for symmetric/complex and added a note, derived givens just has transpose is just reordering the sine\/-sine which is same as passing in -sine into the original givens
…d the tests as those are now private, which cleans a bit of the files for symmetric/complex and added a note, derived givens just has transpose is just reordering the sine\/-sine which is same as passing in -sine into the original givens
…d the tests as those are now private, which cleans a bit of the files for symmetric/complex and added a note, derived givens just has transpose is just reordering the sine\/-sine which is same as passing in -sine into the original givens
…just need to center and let it rip and they have pca, they can post sort, and like do things with negative singular values, just building the core for them
Contributor
Author
Performance vs.
|
| Size | Autumn_SVD | Nalgebra_SVD | Full_Autumn_SVD | Full_Nalgebra_SVD |
|---|---|---|---|---|
| 8 | 4.77 µs | 4.89 µs | 6.69 µs | 6.48 µs |
| 16 | 18.77 µs | 15.39 µs | 34.69 µs | 22.60 µs |
| 32 | 77.76 µs | 56.96 µs | 187.72 µs | 101.37 µs |
| 64 | 364.34 µs | 247.17 µs | 1237.5 µs | 567.24 µs |
At size 8 the two implementations are essentially at parity. From 16 onward, nalgebra pulls ahead, and the gap widens with size — most noticeably in the Full_* variants (full U/Σ/V vs. values-only), where Full_Autumn_SVD is ~2.2x slower than Full_Nalgebra_SVD at size 64.
Available optimizations
(out of scope for this PR)
- Block the U/V rotations and batch them instead of applying one at a time
- Cache
U'/V'to avoid recomputing transforms - Pack the diagonal entries densely for better cache locality
Autumn SVD is this implementation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SVD (Golub–Kahan–Reinsch)
This adds a full SVD implementation using Golub–Kahan bidiagonalization followed by implicit-shift bulge chasing (the SVD analogue of Francis QR) — same family of algorithm as the eigenvalue PR, applied to the non-symmetric/rectangular case.
A few things worth calling out before you read the diff:
Xand the eigenvalues of the covariance matrixX'Xare equivalent (derivation below, under "Why this replaces the eigen-approach"). This means you can get PCA directly from this SVD without ever formingX'X— which matters, because formingX'Xsquares the condition number of your data and is a real source of numerical error, especially on ill-conditioned inputs.test_svd_reconstruct_trials, tightened to assert== 0rather than a tolerance band, so the test actually backs up the claim). Iteration budget is 20 per eigenvalue plus a 20-iteration bank, and across everything I've run it has not diverged once. I think that's a structurally sound bound, not just an empirical accident, but flagging it as something worth stress-testing further on your end too if you want extra confidence before leaning on it hard.full_*/bare split, same rotation-tracking pattern, same zero-alloc core with an allocatingauto_*wrapper on top), so hopefully the shape of this is already familiar.Why this replaces the eigen-approach for your PCA case
The eigen-solver would work here too — this isn't "that approach was wrong," it's that this one is better suited to what you're doing, for two concrete reasons:
X'Xhas condition numberκ(X)². SVD works onXdirectly, so you're never squaring your conditioning — meaningfully better numerical stability, especially as your data gets less well-conditioned.UandV) rather than a single similarity transform, it doesn't get stuck in the kind of degenerate rotational configurations that can slow or destabilize a symmetric eigensolver on nearly-degenerate inputs.The math connecting the two (so this isn't just assertion):
So
Vfrom this SVD gives you your principal components directly, andS² / (n-1)gives you the variances — no eigendecomposition ofX'Xneeded at all.What you'll need to do on your end
This gets you 95% of the way to PCA, but there are three small pieces I've deliberately left for you to wire in rather than handing you a finished
pca()— partly because they're genuinely trivial, and partly because I think it's worth actually touching this code rather than just importing a black box, especially after ~1700 lines went into the eigenvalue side of this. All three are short:UandV(both are just column permutations, not a re-derivation of anything).(u_i, v_i)and(-u_i, -v_i)are both valid. If you care about consistent sign conventions (e.g. reproducibility across runs), negate the corresponding column ofUorVto match; if you only care about magnitudes, you can ignore this entirely.All three of the above are direct consequences of what's already computed — no new math, just plumbing.
Status
This port was done a bit quickly and should be up to spec, but it's probably due for a more in-depth review pass before you fully rely on it — flagging that honestly rather than presenting it as more battle-tested than it is.
Main branch highlights
svd/full_svd_decompositionthat dispatches into whichever is correct for the input shape.cols >> rows(or vice versa) in a way that would blow out L1 cache, transposing the input first is worth doing — should be quick even at that scale, but it's a deliberate option, not something forced on you.One last thing, unrelated to the code: sorry for how the earlier back-and-forth about the eigen-solver landed — I know it probably read as pushback for its own sake at the time. This is the reasoning behind it, and hopefully it's useful now that it's concrete instead of abstract.