Skip to content

Make the ISVD test robust against ill-conditioning and sign ambiguity - #28

Merged
timholy merged 1 commit into
masterfrom
fix-flaky-test
May 19, 2026
Merged

Make the ISVD test robust against ill-conditioning and sign ambiguity#28
timholy merged 1 commit into
masterfrom
fix-flaky-test

Conversation

@timholy

@timholy timholy commented May 19, 2026

Copy link
Copy Markdown
Member

Problem

The @testset "ISVD" fails intermittently — it failed on the Julia min CI job of #26, at test/runtests.jl's U2 ≈ U comparison.

The test built its target as M = A*B with A = rand(20, 5), B = rand(5, 30). Two independent issues made it flaky:

  1. No headroom in the target. rand entries are all positive, so M is dominated by a rank-one "mean" component and has a lopsided spectrum — σ₅/σ₁ frequently lands at 0.02–0.06. The trailing singular vector is then only weakly determined, and the two incremental SVD paths the test compares (update! from a nothing start vs. isvd from a zero-filled start) disagree on it by more than the default isapprox tolerance. Measured rate: ~0.7% of random draws.

  2. Sign ambiguity. Left singular vectors are determined only up to sign, and the two paths occasionally pick opposite signs for a column. @test U2 ≈ U ignores this — even with a well-conditioned target, ~0.03% of draws fail on a clean sign flip. The test already handles this correctly for its unequal-block comparisons (col1 ≈ col2 || col1 ≈ -col2) but not here.

Fix

  • Build M from orthonormal factors with an explicit, well-separated set of singular values (5, 4, 3, 2, 1), so every singular vector is pinned down to full precision. No seeded RNG — the stream is not stable across Julia versions; the headroom is structural instead.
  • Compare U columnwise allowing a sign flip (matching the idiom already used elsewhere in the file). The sign-coupled rows of Vt are compared by magnitude.

Verification

  • Core comparisons (pA ≈ A, sign-aware U2 ≈ U, s2 ≈ s): 0 failures over 80,000 random trials on both Julia 1.10 and 1.12 — vs. ~206/30,000 with the old construction.
  • Full edited testset: 0 failures over 500 runs.
  • Pkg.test() passes (37/37).

🤖 Generated with Claude Code

The `@testset "ISVD"` built its target as `M = A*B` with `A = rand(20, 5)`
and `B = rand(5, 30)`. Because `rand` entries are all positive, M had a
dominant rank-one component and a lopsided spectrum: the smallest singular
value was often a small fraction of the largest. The trailing singular
vector is then only weakly determined, and the two incremental SVD code
paths compared by the test (`update!` from a `nothing` start vs. `isvd`
from a zero-filled start) disagreed on it by more than the default
`isapprox` tolerance. This failed intermittently, e.g. on the `min` Julia
CI job, depending on the random draw.

Build M instead from orthonormal factors and an explicit, well-separated
set of singular values (5, 4, 3, 2, 1), so every singular vector is pinned
down to full precision. A seeded RNG was avoided deliberately, since the
stream is not stable across Julia versions.

Even with a well-conditioned target, left singular vectors are determined
only up to sign, and the two code paths occasionally choose opposite signs
for a column. Compare `U` columnwise allowing a flip -- the same idiom the
test already uses for the unequal-block comparisons -- and compare the
sign-coupled rows of `Vt` by magnitude.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@timholy
timholy merged commit aa09c06 into master May 19, 2026
2 checks passed
@timholy
timholy deleted the fix-flaky-test branch May 19, 2026 12:04
@codecov

codecov Bot commented May 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.71%. Comparing base (48603f5) to head (e048e41).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #28   +/-   ##
=======================================
  Coverage   98.71%   98.71%           
=======================================
  Files           1        1           
  Lines          78       78           
=======================================
  Hits           77       77           
  Misses          1        1           

☔ View full report in Codecov by Sentry.
📢 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.

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