Skip to content

[FIX] ASR: use a real PSD square root in asr_calibrate#114

Merged
nbara merged 1 commit into
nbara:masterfrom
sappelhoff:fix/asr-calibrate-psd-sqrt
Jul 20, 2026
Merged

[FIX] ASR: use a real PSD square root in asr_calibrate#114
nbara merged 1 commit into
nbara:masterfrom
sappelhoff:fix/asr-calibrate-psd-sqrt

Conversation

@sappelhoff

Copy link
Copy Markdown
Collaborator

asr_calibrate computed M = linalg.sqrtm(np.real(Uavg)), which only strips a pre-existing imaginary part but does not make Uavg positive-semidefinite. If the robust average covariance Uavg has even a tiny negative eigenvalue (realistic for average-referenced / rank-deficient EEG covariance means), scipy.linalg.sqrtm of the resulting indefinite symmetric matrix returns a complex M, which then silently propagates through eigh(M) into V and the threshold matrix T. asr_process already guards against this by taking np.real() of its eigen-quantities, but asr_calibrate did not.

Add _psd_sqrtm(), a real symmetric PSD square root that symmetrizes its input and clamps numerically-negative eigenvalues to zero before taking the root, and use it in asr_calibrate. For a genuinely PSD Uavg (the normal case) this is the same principal square root sqrtm would return, so calibration is numerically unchanged.

Testing

Added a regression test showing _psd_sqrtm stays real on a slightly indefinite matrix where sqrtm returns complex. Full tests/test_asr.py passes; ruff clean.

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.25%. Comparing base (f71758d) to head (3183ac1).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
+ Coverage   83.22%   83.25%   +0.03%     
==========================================
  Files          25       25              
  Lines        2843     2849       +6     
==========================================
+ Hits         2366     2372       +6     
  Misses        477      477              

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

@sappelhoff

Copy link
Copy Markdown
Collaborator Author

Note: includes the nonlinear_eigenspace reproducibility fix from #118. test_asr_class[*-riemann] compares incremental-vs-bulk output with a loose tolerance and is flaky on CI because that path used an unseeded RNG; this can trip on any PR regardless of what it changes. #118 is the root fix — once it merges first, this commit drops out on rebase.

@sappelhoff sappelhoff mentioned this pull request Jul 14, 2026
@sappelhoff
sappelhoff force-pushed the fix/asr-calibrate-psd-sqrt branch from 5b08383 to ded6cee Compare July 19, 2026 09:11
asr_calibrate computed M = linalg.sqrtm(np.real(Uavg)), which only strips a
pre-existing imaginary part but does not make Uavg positive-semidefinite. If
the robust average covariance Uavg has even a tiny negative eigenvalue
(realistic for average-referenced / rank-deficient EEG covariance means),
scipy.linalg.sqrtm of the resulting indefinite symmetric matrix returns a
complex M, which then silently propagates through eigh(M) into V and the
threshold matrix T.

asr_process already guards against this by taking np.real() of its
eigen-quantities after eigh, but asr_calibrate lacked the equivalent guard.
Add _psd_sqrtm(), a real symmetric PSD square root that symmetrizes its input
and clamps numerically-negative eigenvalues to zero before taking the square
root, and use it in asr_calibrate. For a genuinely PSD Uavg (the normal case)
this is the same principal square root sqrtm would return, so calibration is
numerically unchanged; when Uavg is slightly indefinite it now yields a real
M, and hence real V/T.
@nbara
nbara force-pushed the fix/asr-calibrate-psd-sqrt branch from ded6cee to 3183ac1 Compare July 20, 2026 14:06
@nbara
nbara merged commit cbc8943 into nbara:master Jul 20, 2026
5 checks passed
@sappelhoff
sappelhoff deleted the fix/asr-calibrate-psd-sqrt branch July 20, 2026 14:24
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.

2 participants