Prevent overlapping ERP epoch windows in chronological within-session CV

[lindicaphxag-tech]

Summary

Addresses the direct shared-raw-sample failure described in #280: an opt-in chronological within-session CV splitter that removes overlapping epoch extraction intervals from train/test folds. It does not guarantee post-filter signal independence.

Problem

Random trial-wise CV can put epochs that share samples from the same continuous recording into train and test. With a 500-sample epoch and 100-sample SOA, trial-disjoint folds are not signal-disjoint.

Approach

• add PurgedEpochKFold, a sklearn-compatible group-aware splitter;
• sort epochs by true processed event sample within each run;
• form contiguous test blocks per run, choosing near-equal boundaries to improve class balance without shuffling epochs;
• purge candidate training epochs whose half-open sample interval overlaps any test interval from the same run;
• never purge across recording boundaries, using the available (subject, session, run) identity so reused run names cannot cause false cross-session/subject purges;
• fail closed if true event timing is absent or a fold lacks class support;
• expose event_sample and epoch_n_samples only on demand, preserving the default paradigm metadata schema;
• auto-route timing metadata when WithinSessionEvaluation(..., cv_class=PurgedEpochKFold) is selected;
• report n_purged_overlap and purge_fraction as per-fold protocol diagnostics.

Exact large-integer interval correctness (current head)

The original sample parser passed integer event/sample-window coordinates through float64; above 2**53 that can collapse distinct integer sample indices and invalidate exact half-open overlap decisions. fb067c4 preserves true int64 sample identity, refuses ambiguous large floating inputs, and prevents overflow when constructing interval stops. d0100fc adds exact translation-invariance, boundary and fail-closed regressions; pre-commit formatting 45a35dc is the current source head.

Independent exact-source native CI: Python 3.11/3.13 focused PurgedEpochKFold tests both SUCCESS. That workflow checks out the immutable 45a35dc SHA and runs the actual moabb/tests/test_splits.py -k purged_epoch tests, rather than duplicating the parser in a toy implementation. The official full test matrix on the current head runs separately; focused green tests do not replace full upstream acceptance.

Reused run-label correctness

Run labels like "0" are routinely reused across subjects or sessions. The original splitter grouped solely by run, which could place independent recordings on the same sample axis and incorrectly remove training epochs that merely have overlapping numeric sample indices from different raw files. Source fix dfa052a encodes the available (subject, session, run) tuple without changing the three-column groups shape or default metadata. Regression cfca859 verifies independent sessions, subjects and refusal of missing recording identity. Current code is bot-formatted Head 3c76a2c. Independent immutable-SHA validation passed both Python 3.11 and 3.13 on the real moabb/tests/test_splits.py -k purged_epoch regression subset. The upstream full cross-platform matrix and maintainer Review are separate gates.

Minimal usage

from moabb.evaluations import PurgedEpochKFold, WithinSessionEvaluation

evaluation = WithinSessionEvaluation(
    paradigm=paradigm,
    datasets=[dataset],
    cv_class=PurgedEpochKFold,
    n_splits=5,
)

No manual chronology labels are required; the evaluation requests event timing from the processed epoch stream.

Regression design

The synthetic witness uses 100-sample SOA and 500-sample epochs:

• shuffled StratifiedKFold has train/test interval overlap;
• PurgedEpochKFold has zero same-run overlap;
• every epoch appears in a test fold exactly once;
• test folds remain contiguous in event time;
• when feasible, boundary selection improves class balance while preserving class support;
• independent runs never purge one another;
• missing/invalid timing fails explicitly;
• BaseParadigm.get_data() keeps its existing subject/session/run metadata by default and only adds timing when requested.

Scope / limitation

Important preprocessing caveat discovered in review: BaseParadigm.make_process_pipelines() applies the standard continuous-Raw IIR band-pass before cutting epochs. A zero-phase IIR filter has nonlocal temporal response, so disjoint extraction intervals do not prove that the resulting filtered signals contain no cross-boundary contribution. This PR prevents direct reuse of overlapping raw sample-index windows; it does not claim strict signal independence or eliminate all possible temporal leakage. A stricter protocol requires fold-aware preprocessing or independently justified transformation-specific support. A new synthetic regression (moabb/tests/test_purged_preprocessing_boundary.py) demonstrates this counterexample explicitly and preserves the narrower contract.

The v1 contract assumes a fixed event-locked epoch window within an evaluated partition. The interval is represented on the processed epoch sample grid. It intentionally does not infer chronology from DataFrame row order and does not claim to model arbitrary time-warping post-processing. If a chronological partition cannot provide class support in every fold, the splitter fails closed rather than interleaving time blocks.

The review suggestion to use time-split CV with stratification is addressed by selecting near-equal contiguous boundaries with dynamic programming; temporal order remains intact and overlap is purged exactly. The previous full upstream matrix was green; the newly formatted exact-integer head has its own current checks and independent Python 3.11/3.13 source-pinned regression proof. Completion of the latest full matrix and maintainer review are still required. The four runtime files carry timing metadata only for this opt-in evaluation path; default metadata remains unchanged. This PR is ready for review.

[byneuro]

I am not super supportive of this design, like, I feel that we can have the same thing just using the native split or implement some time split CV with stratification. The PR is also touch many extra sensitive areas of moabb.

[lindicaphxag-tech]

Thanks for the design feedback. I checked the native alternatives against issue #280: GroupKFold by run prevents mixing separate recordings, but it cannot split the single-run case in the issue; ordinary StratifiedKFold shuffles epochs and reintroduces sample overlap. This draft instead makes contiguous time blocks within each run, purges every half-open interval overlap, and fails closed when a fold lacks class support. The timing columns are opt-in, so the default paradigm metadata remains unchanged; the evaluation path is activated only for this splitter.

I agree the cross-cutting integration needs a clear scope boundary. Would you prefer this issue to cover only run-held-out evaluation (which leaves single-run recordings unresolved), or is the opt-in single-run time/purge protocol in scope? I am keeping the PR as draft until that design question is resolved.

I also fixed the two failures from the first full CI run: MNE offsets event samples when concatenating runs, so the regression now compares timing within each run; and the cross-run purge test now uses valid non-overlapping epochs with an explicit cross-run overlap candidate. I moved metadata assertions back into their original test. On the new head 61fe1c453262782da079880d9e0b26049f5c4450, all three affected test modules passed locally under Python 3.11, and pre-commit passed for the changed files.

[lindicaphxag-tech]

Update on the new head: I merged current develop and resolved the changelog conflict while preserving both entries. GitHub now reports the branch mergeable; pre-commit, link-check, and changelog checks pass. The full cross-platform test matrix has started and is pending. Focused full-file validation remains green after the base sync: test_paradigms.py, test_splits.py, and test_evaluations.py all passed under Python 3.11.

[lindicaphxag-tech]

CI update for head 4c5b03592f16a210d25218714887e2ce6d3ae9ae: the main test matrix has now passed on Ubuntu Python 3.11, 3.12, 3.13, and 3.14, plus macOS and Windows on Python 3.12; pre-commit, link-check, changelog, and Codecov are also green. The docs build and braindecode-dev job are still running. I am leaving the PR in draft until the design scope in my earlier reply is resolved.

[lindicaphxag-tech]

I followed up on the stratified-time-split suggestion in commit 0a38c741c6d3151a0cff88dfc530cc1e2fb6f033. The splitter now uses dynamic programming to shift near-equal fold boundaries for better class balance while keeping each test fold contiguous in event time; it still purges exact half-open sample overlap and fails closed when class support cannot be maintained. A regression witness compares the class-proportion error against equal-size temporal chunks and checks contiguity, full test coverage, class support, and zero overlap. Local validation on this head: all of test_splits.py and test_evaluations.py passed; Ruff check/format passed. New upstream CI is running. Does this interpretation of stratified time CV address the design concern? I am keeping the PR draft until we get scope guidance.

[lindicaphxag-tech]

CI has advanced on current head 0a38c741c6d3151a0cff88dfc530cc1e2fb6f033: Ubuntu Python 3.11 and 3.14 are green, as are Codecov, pre-commit, link-check, and changelog validation. Ubuntu 3.12/3.13, macOS, Windows, docs, and braindecode-dev remain in progress. The PR remains draft pending the design-scope response requested above.

[lindicaphxag-tech]

I’ve made the draft ready for review. The stratified-time suggestion is implemented with dynamic-programming boundary selection while keeping each test fold contiguous and purging exact sample overlap. The complete upstream matrix is green now: Ubuntu Python 3.11–3.14, macOS/Windows Python 3.12, docs, and the braindecode-dev job; pre-commit, link, changelog, and Codecov checks also pass. Event timing is opt-in, so the default paradigm metadata schema is unchanged. Please review the updated scope and let me know if you want a narrower integration.

[byneuro]

Can you do some visual, ala scikit learn, to represent exactly what this split is? I have the feeling that we are making a mistake here, but I may be wrong.

[lindicaphxag-tech]

Follow-up to the requested visualization: the review prompted a material correction to the scientific scope, which I have now made in the PR itself rather than hiding it in a comment.

The 5×30 matrix correctly describes PurgedEpochKFold's guarantee of zero overlap between declared raw epoch-extraction intervals. However, I traced the upstream data pipeline and found an additional temporal dependence that the diagram alone could conceal: BaseParadigm.make_process_pipelines() applies get_filter_pipeline() to the continuous Raw recording before epoching, using an IIR band-pass. A change inside a held-out raw interval can therefore affect a nonoverlapping neighbor after filtering (especially the zero-phase path). Simply purging direct raw-window intersections is not a proof of independent post-filter signals.

To avoid overclaiming, I updated the PR title/summary, API figure explanation, and PurgedEpochKFold docstring to specify the raw-index overlap contract. I also added a focused synthetic IIR impulse counterexample under moabb/tests/test_purged_preprocessing_boundary.py. It demonstrates the distinction without falsely implying the current splitter can fix filter tails; a fully leakage-isolated pipeline would require fold-aware preprocessing or a separately justified transformation-support constraint.

Current new head: c338c4da62eb4013b1be9c3736f144ccd5512f94, mergeable; upstream CI rerunning. The original exact per-cell visualization test remains unchanged. This should help distinguish whether the desired fix for #280 is (a) eliminate literal overlapping epoch sample windows (implemented here) or (b) establish stronger post-preprocessing signal independence (not implemented or proven). I welcome a narrower-scope decision rather than broadening the code unreviewed.

[lindicaphxag-tech]

I took your concern about touching sensitive evaluation/paradigm code seriously and prepared an actual narrow alternative, rather than adding another general CI/status message.

Two-file, internal fork draft (NOT a second upstream PR): on top of current develop, it only appends the opt-in epoch_interval_groups / PurgedEpochKFold core to moabb/evaluations/splitters.py, plus a separate purpose-built test module. It deliberately does not change paradigm preprocessing, dataset metadata, WithinSessionEvaluation, defaults, or any API registry. Thus it is reviewable without routing the feature through the sensitive areas you flagged in #1235.

I also found a scalability issue in the earlier N_train × N_test overlapping-interval broadcast. The narrow variant now uses a sorted start/prefix-max-end interval lookup to preserve the exact half-open raw-epoch overlap rule in O((N+M) log M) time and O(N+M) memory; the new tests compare it against a separate dense overlap oracle on 100 randomized unsorted/nested interval cohorts and exercise 30,000 train/test intervals. Final validation is now green: source-exact GitHub Actions 37834320764 passed all 9/9 behavior/adversarial cases on both Python 3.11 and 3.12. I then removed the temporary validation workflow from the PR to retain exactly two changed files; both file Git blobs are byte-identical to the successful CI head (splitters.py 9de7ac3b, test_purged_epoch_core_only.py 564c5756). The 30,000-interval scalability test was part of those passing checks.

Important unchanged scientific limit: independently purged raw epoch windows can still share information after continuous IIR filtering. This alternative makes no claim to fix fold-unaware preprocessing or the original issue end-to-end without later integration. It is intentionally a small, opt-in raw-index primitive.

Scope decision for maintainers: Would you prefer (A) such a two-file standalone sklearn-compatible splitter as a first minimal upstream change (with evaluation integration separate), or (B) no new splitter until a full preprocessing-aware protocol exists? I will NOT submit a competing second upstream PR unless the direction is useful; #1235 stays the single active upstream discussion.

[lindicaphxag-tech]

A concrete narrower implementation is available for your scope review: fork-only Draft PR #6 (not a competing upstream submission). Since the earlier version, I corrected a scientifically meaningful recording-identity issue: identical run labels from distinct subjects/sessions no longer share a sample coordinate system or accidentally purge each other's epochs. I also added an independent 30k-interval sweep-line oracle and dedicated Python 3.11/3.13 targeted CI; both focused jobs pass, with 13/14 current full-fork checks successful and docs still running. This alternative changes only splitter core, tests, changelog, and a focused workflow; no paradigm/evaluator/preprocessing/default behavior. The only guarantee is direct raw epoch extraction-window non-overlap, not post-IIR signal independence. Would you prefer this opt-in isolated primitive as a smaller first upstream change, or an end-to-end fold-aware preprocessing design for #280? I will avoid opening a duplicate upstream PR until you advise.