Skip to content

Commit 3b0c530

Browse files
sappelhoffnbara
andauthored
[FIX] ASR: uncentered scm block covariance + overlap/bound/jump fixes (#110)
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. --------- Co-authored-by: nbara <10333715+nbara@users.noreply.github.com>
1 parent ba7aa78 commit 3b0c530

3 files changed

Lines changed: 111 additions & 10 deletions

File tree

meegkit/utils/covariances.py

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def block_covariance(data, window=128, overlap=0.5, padding=True, estimator="cov
2424
window : int
2525
Window size.
2626
overlap : float
27-
Overlap between successive windows.
27+
Fraction of overlap between successive windows (higher = more overlap).
2828
2929
Returns
3030
-------
@@ -33,19 +33,30 @@ def block_covariance(data, window=128, overlap=0.5, padding=True, estimator="cov
3333
3434
"""
3535
assert 0 <= overlap < 1, "overlap must be < 1"
36+
window = int(window) # window may be fractional
3637
blocks = []
3738
n_chans, n_samples = data.shape
3839
if padding: # pad data with zeros
39-
pad = np.zeros((n_chans, int(window / 2)))
40+
pad = np.zeros((n_chans, window // 2))
4041
data = np.concatenate((pad, data, pad), axis=1)
42+
n_samples = data.shape[1] # bound against the padded length
4143

42-
jump = int(window * overlap)
44+
jump = max(int(round(window * (1 - overlap))), 1) # >= 1 so overlap=0 advances
4345
ix = 0
44-
while (ix + window < n_samples):
46+
while ix + window <= n_samples:
4547
blocks.append(data[:, ix:ix + window])
46-
ix = ix + jump
47-
48-
return covariances(np.array(blocks), estimator=estimator)
48+
ix += jump
49+
50+
if len(blocks) == 0:
51+
raise ValueError(
52+
"block_covariance: window is too large for the given data "
53+
"(no complete blocks).")
54+
55+
blocks = np.array(blocks)
56+
if estimator == "scm":
57+
# uncentered second moment E[x x.T] per block (not mean-subtracted)
58+
return blocks @ blocks.transpose(0, 2, 1) / window
59+
return covariances(blocks, estimator=estimator)
4960

5061

5162
def cov_lags(X, Y, shifts=None):

tests/test_asr.py

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
from meegkit.asr import ASR, asr_calibrate, asr_process, clean_windows
1212
from meegkit.utils.asr import SHAPE_RANGE, fit_eeg_distribution, yulewalk, yulewalk_filter
13+
from meegkit.utils.covariances import block_covariance
1314
from meegkit.utils.matrix import sliding_window
1415

1516
# Data files
@@ -359,7 +360,7 @@ def test_asr_calibrate_too_short():
359360
with pytest.raises(ValueError, match="shorter than one analysis window"):
360361
asr_calibrate(X_short, 250)
361362

362-
363+
363364
def test_asr_max_bad_chans_param():
364365
"""max_bad_chans is exposed on ASR and defaults to 0.3."""
365366
assert ASR().max_bad_chans == 0.3
@@ -451,6 +452,91 @@ def test_fit_eeg_distribution_default_step_sizes():
451452
assert not np.allclose(sig_old, sig_explicit, rtol=1e-9)
452453

453454

455+
def test_block_covariance_uncentered_scm():
456+
"""scm blocks use the uncentered second moment, not the mean-subtracted cov.
457+
458+
A large per-channel DC offset makes the two estimates diverge strongly.
459+
"""
460+
W = 50
461+
data = rng.standard_normal((4, 250)) + np.arange(1, 5)[:, None] * 100.0
462+
463+
cov = block_covariance(data, window=W, overlap=0.5, padding=False,
464+
estimator="scm")
465+
466+
# Reference: same block start indices the function uses.
467+
jump = max(int(round(W * (1 - 0.5))), 1)
468+
n_samples = data.shape[1]
469+
ref, centered = [], []
470+
ix = 0
471+
while ix + W <= n_samples:
472+
B = data[:, ix:ix + W]
473+
ref.append(B @ B.T / W) # uncentered second moment
474+
centered.append(np.cov(B, bias=True)) # mean-subtracted
475+
ix += jump
476+
ref = np.array(ref)
477+
centered = np.array(centered)
478+
479+
assert cov.shape == ref.shape
480+
assert np.allclose(cov, ref, rtol=1e-10)
481+
# Must NOT be the centered (mean-subtracted) version.
482+
assert not np.allclose(cov, centered)
483+
484+
485+
def test_block_covariance_overlap_semantics():
486+
"""Higher overlap yields more blocks."""
487+
W = 50
488+
data = rng.standard_normal((4, 500))
489+
n_hi = block_covariance(data, window=W, overlap=0.8).shape[0]
490+
n_lo = block_covariance(data, window=W, overlap=0.2).shape[0]
491+
assert n_hi > n_lo
492+
493+
494+
def test_block_covariance_padding_bound():
495+
"""Padding adds blocks (loop bound uses the padded length)."""
496+
W = 100
497+
data = rng.standard_normal((8, 300))
498+
n_pad = block_covariance(data, window=W, overlap=0.5, padding=True).shape[0]
499+
n_nopad = block_covariance(data, window=W, overlap=0.5,
500+
padding=False).shape[0]
501+
assert n_pad > n_nopad
502+
503+
504+
def test_block_covariance_empty_guard():
505+
"""A window too large for the data raises a clear error."""
506+
data = rng.standard_normal((4, 30))
507+
with pytest.raises(ValueError, match="too large"):
508+
block_covariance(data, window=100, padding=False)
509+
510+
511+
def test_block_covariance_exact_one_window():
512+
"""When n_samples equals window, exactly one block is returned."""
513+
data = rng.standard_normal((4, 100))
514+
cov = block_covariance(data, window=100, overlap=0.5, padding=False,
515+
estimator="scm")
516+
517+
assert cov.shape == (1, 4, 4)
518+
519+
520+
def test_block_covariance_no_hang_and_float_window():
521+
"""Zero-overlap does not hang and float windows are accepted."""
522+
raw = np.load(os.path.join(THIS_FOLDER, "data", "eeg_raw.npy"))
523+
sfreq = 250
524+
X_small = raw[:, 5 * sfreq:15 * sfreq] # 8 chans x 10 s
525+
526+
# win_overlap=0.0 previously looped forever (jump=0)
527+
asr = ASR(sfreq=sfreq, win_overlap=0.0)
528+
asr.fit(X_small)
529+
M = asr.state_["M"]
530+
T = asr.state_["T"]
531+
assert np.isfinite(M).all()
532+
assert np.isfinite(T).all()
533+
534+
# Float window must not break slicing (int-cast).
535+
cov = block_covariance(X_small, window=100.0, overlap=0.5,
536+
estimator="scm")
537+
assert np.isfinite(cov).all()
538+
539+
454540
if __name__ == "__main__":
455541
pytest.main([__file__])
456542
# test_yulewalk(250, True)

tests/test_pyriemann_imports.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,14 @@ def test_block_covariance_uses_pyriemann_covariances():
4545
from pyriemann.geometry.covariance import covariances
4646

4747
data = 0.1 + 0.01 * (1 + np.arange(24, dtype=float).reshape(3, 8))
48+
window = 4
49+
overlap = 0.5
50+
jump = max(int(round(window * (1 - overlap))), 1)
51+
n_samples = data.shape[1]
4852
expected_blocks = np.array(
49-
[data[:, start:start + 4] for start in range(0, 4, 2)]
53+
[data[:, start:start + window] for start in range(0, n_samples - window + 1, jump)]
5054
)
5155

52-
actual = block_covariance(data, window=4, overlap=0.5, padding=False)
56+
actual = block_covariance(data, window=window, overlap=overlap, padding=False)
5357

5458
np.testing.assert_allclose(actual, covariances(expected_blocks, estimator="cov"))

0 commit comments

Comments
 (0)