🐛 fix: avoid float32 overflow in the SOM's PCA-fallback axis

[nstarman]

What's wrong

init_prototypes' PCA fallback (used when standalone SOMOrderer gets no prior init) finds the first principal axis via eigh(centered.T @ centered). The Gram matrix squares the position magnitudes, which overflows float32's ~3.4e38 ceiling well within ordinary data — 1 kpc in SI units is ~3.086e19 m, and squaring that alone exceeds the ceiling.

The overflow raises nothing: eigh of an inf-valued matrix returns NaN eigenvectors, and argsort ranks NaN-dot-products without complaint. The resulting ordering is silently wrong — on a 120-point half-circle, |ρ| against the true curve parameter drops from 1.0 (galactic units) to 0.75 (SI), while n_visited still reports every point visited, so nothing downstream can tell the ordering degraded.

This is pre-existing on main, not introduced by #56 — found during an independent audit of that PR and #80, filed as #86 to avoid widening either.

The fix

jnp.linalg.svd on the centred matrix directly gives the same leading axis (its first right-singular vector) without ever forming the Gram, so nothing gets squared and the overflow has nothing to act on. This is option 2 from the issue, the numerically standard way to get a principal axis.

Testing

Added test_pca_fallback_survives_large_position_magnitude to TestNumericalHazards in tests/unit/test_som.py, parametrized over position-magnitude scale including 1e18 and 3.086e19 (1 kpc in m). Confirmed it fails on main at those scales (non-monotone, NaN-driven prototypes) and passes after the fix. Full test suite: 1009 passed, 1 skipped.

Closes #86.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

The new regression test should explicitly force float32 inputs; otherwise environments with jax_enable_x64 enabled may not exercise the overflow scenario it is intended to guard.

Review effort: Lite
Findings: 1

Open (1)

• Test relies on default dtype instead of forcing float32 · New

What changed in this PR

This PR addresses a numerical stability bug in the SOM standalone initialization path (init_prototypes) where computing the PCA axis via a float32 Gram matrix (centered.T @ centered) can overflow for large-magnitude position units (e.g., SI meters for kpc-scale data), silently producing a corrupted ordering. The fix switches the PCA fallback to use SVD on the centered data matrix, avoiding explicit squaring of magnitudes, and adds a regression test for large scales.

Changes:

• Replace the PCA fallback axis computation in init_prototypes from eigh(centered.T @ centered) to svd(centered)[2][0] to avoid float32 overflow.
• Add a regression test covering large position-magnitude scales (including ~1 kpc in meters) to ensure the PCA fallback remains finite and monotone.

File	Description
src/​phasecurvefit/​_src/​som.py	Switch PCA-fallback axis extraction to an SVD-based approach to avoid float32 Gram-matrix overflow.
tests/​unit/​test_som.py	Add a numerical-hazards regression test for large-magnitude position scales in SOM prototype initialization.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@2385f16). Learn more about missing BASE report.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #136   +/-   ##
=======================================
  Coverage        ?   91.97%           
=======================================
  Files           ?       34           
  Lines           ?     1982           
  Branches        ?      109           
=======================================
  Hits            ?     1823           
  Misses          ?      129           
  Partials        ?       30           

☔ 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.

[Copilot]

Copilot review overview

🟢 Approval recommended

The SVD-based axis extraction directly addresses the overflow root cause and is covered by a focused regression test that forces the float32 execution path.

Review effort: Lite
Findings: 1

Open (1)

• Test only checks one prototype component for finiteness · New

Resolved since last review (1)

• Test relies on default dtype instead of forcing float32

[Copilot]

Copilot review overview

🟢 Approval recommended

The change directly removes the overflow mechanism in the PCA fallback and is covered by a targeted float32 regression test over the reported hazardous scales.

Review effort: Lite
Findings: None

Resolved since last review (1)

• Test only checks one prototype component for finiteness