Skip to content

Detrend in place in NoisyChannels.__init__ to avoid full-array copies#196

Merged
sappelhoff merged 3 commits into
mainfrom
perf/in-place-detrend
Jun 15, 2026
Merged

Detrend in place in NoisyChannels.__init__ to avoid full-array copies#196
sappelhoff merged 3 commits into
mainfrom
perf/in-place-detrend

Conversation

@sappelhoff

Copy link
Copy Markdown
Owner

Summary

Detrend the EEG signal in place during NoisyChannels.__init__, avoiding two transient full-size copies of the recording during construction.

  • removeTrend gains a copy=True keyword argument that is passed through to mne.filter.filter_data. The default (copy=True) preserves the exact current behavior for every existing caller.
  • NoisyChannels.__init__ now calls removeTrend(self.raw_mne._data, ..., copy=False) instead of removeTrend(self.raw_mne.get_data(), ...).

Why

The previous code allocated two avoidable full-array copies while detrending:

  1. get_data() always returns a copy (verified: it does not share memory with _data).
  2. mne.filter.filter_data(copy=True) allocates a separate output buffer.

Filtering in place removes both. On an 18.5-min, 99-channel @ 500 Hz recording (~439 MB), filter_data peaks at +887 MB → +443 MB (one full-array transient removed), and __init__ drops from ~1.94 s to ~1.77 s.

The matlab_strict path already filters in place, so copy only affects the default (MNE high-pass) branch.

Correctness

  • mne.filter.filter_data(copy=False) is bit-identical to copy=True (verified np.array_equal); copy only controls whether MNE allocates a new array.
  • Full test suite passes (47 passed), including tests/test_matprep_compare.py.
  • get_bads(as_dict=True) and all _extra_info arrays unchanged on the real recording (reject "omit"/None) and the eegbci fixture (matlab_strict False/True).

@codecov

codecov Bot commented Jun 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.85%. Comparing base (24530cb) to head (ca8708d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #196   +/-   ##
=======================================
  Coverage   97.85%   97.85%           
=======================================
  Files           7        7           
  Lines         839      839           
=======================================
  Hits          821      821           
  Misses         18       18           

☔ 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 sappelhoff enabled auto-merge (squash) June 15, 2026 09:22
@sappelhoff sappelhoff merged commit 8d2c7b6 into main Jun 15, 2026
12 of 14 checks passed
@sappelhoff sappelhoff deleted the perf/in-place-detrend branch June 15, 2026 09:30
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.

1 participant