Skip to content

fix: use relative cutoffs for singular values and residuals#213

Open
gaoflow wants to merge 1 commit into
mljs:mainfrom
gaoflow:relative-singular-value-cutoff
Open

fix: use relative cutoffs for singular values and residuals#213
gaoflow wants to merge 1 commit into
mljs:mainfrom
gaoflow:relative-singular-value-cutoff

Conversation

@gaoflow

@gaoflow gaoflow commented Jul 26, 2026

Copy link
Copy Markdown

pseudoInverse compares singular values against an absolute constant (Number.EPSILON) and linearDependencies compares its solve residual against an absolute 10e-10. Both quantities scale with the matrix, so both functions answer according to the magnitude of the input rather than its rank — in both directions:

pseudoInverse(new Matrix([[1, 2], [2, 4], [3, 6], [4, 8]]));
// [[ 7.3e14, -2.5e13, -1.6e14, -4.9e13],    numpy: [[0.00667, 0.01333, 0.02, 0.02667],
//  [-3.7e14,  1.2e13,  8.1e13,  2.5e13]]            [0.01333, 0.02667, 0.04, 0.05333]]

pseudoInverse(new Matrix([[2, 4], [7, 1]]).mul(1e-17));
// [[0, 0], [0, 0]]  -- full rank and well conditioned, but every singular value
//                      is below Number.EPSILON, so all of them are discarded

linearDependencies(new Matrix([[1, 2, 3], [4, 5, 6], [2, 4, 6]]).mul(1e6));
// [[0,0,0],[0,0,0],[0,0,0]]  -- row 3 is exactly 2 * row 1, at every scale

linearDependencies(new Matrix([[1, 2, 3], [4, 5, 6], [7, 8, 10]]).mul(1e-9));
// [[0, 1.88, -0.88], [0.18, 0, 0.55], [-0.28, 1.78, 0]]  -- full rank, yet every
//                                                           row is "dependent"

The library already contains the correct rule: SVD.threshold and SVD.rank are relative to s[0], and SVD.solve / SVD.inverse use it — so svd.inverse() returns the right pseudo-inverse from the very same decomposition that pseudoInverse gets wrong. numpy.linalg.pinv uses the same relative rule (rcond * max(s)).

Fix. The pseudoInverse cutoff becomes threshold * max(rows, columns) * s[0], which is exactly SVD.rank's tolerance, so the two can no longer disagree about which singular values are zero. linearDependencies divides the residual by the magnitude of the row it explains. thresholdValue stays absolute — it filters the coefficients, which are already scale-free. This changes threshold and thresholdError from absolute to relative parameters; matrix.d.ts is updated.

Tests. Five new tests, all red before the change: numpy values for two rank-deficient matrices at ordinary magnitude, pinv(k*A) === pinv(A)/k for k from 1e-17 to 1e17 over four shapes, the four Moore-Penrose conditions at three magnitudes, and scale invariance of linearDependencies in both directions. The existing pinned tests are unchanged and still pass (261 -> 266) — the old pseudoinverse test only used matrices at the one magnitude where an absolute cutoff happens to work. I also ran a property sweep over ~12.5k random cases (shapes 1..6, magnitudes 1e-18..1e18, 40% forced rank-deficient): 2042 violations before, 0 after.

One note from auditing the other tolerance constants in src/: the eps * tst1 / eps * norm tests in svd.js and evd.js are already relative and correct, but nipals.js has the same shape of problem — terminationCriteria = 1e-10 is compared against sum((t - tOld)^2), which scales with the data. On the iris fixture the tests use, scaling x by 1e-6 moves the first loading vector from [0.5211, -0.2693, 0.5804, 0.5649] to [0.5490, -0.2041, 0.5802, 0.5660]. I left it out of this PR because making it relative also means re-picking the default constant; happy to send it separately if you want it.

pseudoInverse compared singular values against an absolute constant
(Number.EPSILON) and linearDependencies compared the solve residual against an
absolute 10e-10. Both quantities scale with the matrix, so the answers depended
on the magnitude of the input rather than on its rank: a rank-deficient matrix
gets a pseudo-inverse made of rounding noise, and a small enough matrix gets
truncated to zero or reported as fully dependent.

pseudoInverse now uses threshold * max(rows, columns) * s[0], the tolerance
SVD.rank already applies, which is also the rcond * max(s) rule of LAPACK
pseudo-inverses. linearDependencies divides the residual by the magnitude of
the row it explains; thresholdValue stays absolute because the coefficients it
filters are scale-free.
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.60%. Comparing base (ee13f4b) to head (0d5049b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #213      +/-   ##
==========================================
+ Coverage   68.58%   68.60%   +0.02%     
==========================================
  Files          49       49              
  Lines        5847     5852       +5     
  Branches     1042     1043       +1     
==========================================
+ Hits         4010     4015       +5     
  Misses       1824     1824              
  Partials       13       13              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stropitek
stropitek self-requested a review July 27, 2026 07:47
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 this pull request may close these issues.

1 participant