Skip to content

[FIX] ASR: uncentered scm block covariance + overlap/bound/jump fixes#110

Merged
nbara merged 3 commits into
nbara:masterfrom
sappelhoff:fix/asr-block-covariance
Jul 20, 2026
Merged

[FIX] ASR: uncentered scm block covariance + overlap/bound/jump fixes#110
nbara merged 3 commits into
nbara:masterfrom
sappelhoff:fix/asr-block-covariance

Conversation

@sappelhoff

Copy link
Copy Markdown
Collaborator

Fixes a coordinated cluster of issues in block_covariance:

  • Uncentered scm covariance: for estimator="scm" compute the uncentered per-block second moment E[x x.T] = B @ B.T / window directly, matching ASR.transform (cov = 1/N * X @ X.T) and the ASR spec (the calibration covariance is not mean-subtracted). pyriemann's scm mean-centers each block, which diverges the calibration matrix M from the correct value on real data and makes calibrate inconsistent with transform. Other estimators still route through pyriemann covariances.
  • Loop bound: the loop bound is now computed against the padded length. Previously n_samples was captured before padding, so padding added zero iterations and trailing samples were dropped. A window too large to form any complete block now raises a clear ValueError.
  • Overlap convention: jump now uses round(window * (1 - overlap)) so overlap is the fraction of overlap (higher = more overlap), matching clean_windows and asr_calibrate. Previously a larger overlap produced a larger jump (less actual overlap).
  • Robustness: jump is clamped to >= 1 so zero-overlap no longer loops forever, and window is cast to int so fractional windows don't break slicing.

Testing

Adds one regression test per fix. 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.15%. Comparing base (ba7aa78) to head (a3bd391).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
+ Coverage   83.11%   83.15%   +0.04%     
==========================================
  Files          25       25              
  Lines        2825     2832       +7     
==========================================
+ Hits         2348     2355       +7     
  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-block-covariance branch from a7a23e7 to 7860a64 Compare July 19, 2026 09:15
Fixes a cluster of issues in block_covariance:

- Overlap convention: jump now uses round(window * (1 - overlap)) so
  `overlap` is the fraction of overlap (higher = more overlap), matching
  clean_windows and asr_calibrate. Previously a larger overlap produced a
  larger jump (less actual overlap), colliding with asr_calibrate's
  win_overlap convention.
- Loop bound: it is now computed against the padded length. Previously
  n_samples was captured before padding, so padding added zero iterations
  and trailing samples were dropped. A window too large to form any
  complete block now raises a clear ValueError.
- jump is clamped to >= 1 so zero-overlap no longer loops forever, and
  window is cast to int so fractional windows don't break slicing.
- For estimator="scm", compute the uncentered per-block second moment
  E[x x.T] = B @ B.T / window directly, matching ASR.transform
  (cov = 1/N * X @ X.T) and the ASR spec (covariance is not
  mean-subtracted). pyriemann's scm mean-centers each block, which
  diverges the calibration matrix M from the correct value on real data
  and makes calibrate inconsistent with transform. Other estimators still
  route through pyriemann covariances.

Adds regression tests for each fix.
@nbara
nbara force-pushed the fix/asr-block-covariance branch from 7860a64 to e73290f Compare July 20, 2026 12:56
@nbara
nbara merged commit 3b0c530 into nbara:master Jul 20, 2026
7 checks passed
@sappelhoff
sappelhoff deleted the fix/asr-block-covariance branch July 20, 2026 13:33
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