[FIX] ASR: uncentered scm block covariance + overlap/bound/jump fixes#110
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
Collaborator
Author
|
Note: includes the |
Open
sappelhoff
force-pushed
the
fix/asr-block-covariance
branch
from
July 19, 2026 09:15
a7a23e7 to
7860a64
Compare
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
force-pushed
the
fix/asr-block-covariance
branch
from
July 20, 2026 12:56
7860a64 to
e73290f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a coordinated cluster of issues in
block_covariance:estimator="scm"compute the uncentered per-block second momentE[x x.T] = B @ B.T / windowdirectly, matchingASR.transform(cov = 1/N * X @ X.T) and the ASR spec (the calibration covariance is not mean-subtracted). pyriemann'sscmmean-centers each block, which diverges the calibration matrixMfrom the correct value on real data and makes calibrate inconsistent with transform. Other estimators still route through pyriemanncovariances.n_sampleswas 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 clearValueError.jumpnow usesround(window * (1 - overlap))sooverlapis the fraction of overlap (higher = more overlap), matchingclean_windowsandasr_calibrate. Previously a larger overlap produced a larger jump (less actual overlap).jumpis clamped to>= 1so zero-overlap no longer loops forever, andwindowis cast tointso fractional windows don't break slicing.Testing
Adds one regression test per fix. Full
tests/test_asr.pypasses;ruffclean.