Skip to content

Speed up small reads on the Raw.get_data hot path - #14212

Merged
larsoner merged 3 commits into
mne-tools:mainfrom
bruAristimunha:pr/1-hot-path
Aug 25, 2026
Merged

Speed up small reads on the Raw.get_data hot path#14212
larsoner merged 3 commits into
mne-tools:mainfrom
bruAristimunha:pr/1-hot-path

Conversation

@bruAristimunha

@bruAristimunha bruAristimunha commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Reference issue (if any)

None. Performance work; overview of the planned series in #14209.

What does this implement/fix?

Speeds up small reads on the Raw.get_data hot path:

  • get_data resolves picks=None to arange(nchan) directly instead of going
    through string-based channel-name machinery on every call
  • _picks_to_idx returns early for integer arrays that are unique and in range
    (duplicates keep the validating path, per review)
  • _mult_cal_one fuses gather, type-cast and calibration into one pass

Output is bit-identical; all suites pass.

Random 2 s windows via public API, best of 4 interleaved runs vs main
(128 ch × 1800 s @ 512 Hz fixtures): EDF 1543 → 774 µs, BDF 2255 → 1498 µs,
BrainVision 973 → 245 µs, FIF 1087 → 305 µs per read.

Additional information

AI disclosure: I directed the work and reviewed/tested everything; Claude Code
(Opus 4.1) wrote most of the code edits and the benchmark harness under my
direction (scripts in benchmarks/io_dl/ on the branch).

This is part 1 of a stacked series (#14213#14216 depend on it).

Comment thread mne/_fiff/pick.py
Comment thread mne/io/base.py Outdated

return sel, start, stop

def _get_windows(self, starts, width, *, out=None, sel=None):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't appear to be used anywhere or tested, so probably shouldn't be in this PR

@larsoner larsoner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than two comments, seems reasonable to me. Can you give speedup numbers for this PR?

@larsoner

Copy link
Copy Markdown
Member

Oh never mind the speedups are above 🤦

@bruAristimunha

bruAristimunha commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both comments:

  • pick fast path uses np.unique as you suggested
  • removed _get_windows

This PR alone, random 2 s windows, public API, best of 4 interleaved runs vs
main (128 ch x 1800 s @ 512 Hz fixtures): EDF 1543 -> 774 µs, BDF 2255 ->
1498 µs, BV 973 -> 245 µs, FIF 1087 -> 305 µs. Benchmark scripts are on the
branch under benchmarks/io_dl/.

@larsoner larsoner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also take out of draft mode if ready to go from your end!

Comment thread doc/changes/dev/14212.bugfix.rst Outdated
Sped up raw data access for workloads making many small reads:
``Raw.get_data`` no longer materializes the full time axis when ``tmin`` and
``tmax`` are unset, resolves ``picks=None`` without channel-name machinery,
and ``_mult_cal_one`` applies gather/type-cast/calibration in a single pass.

@larsoner larsoner Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs name at the end, and should be simpler. This is also not a bug but an enhancement so should be 14212.newfeature.rst. Something like

Sped up reading by optimizing small read paths, by `<name>`_

Please also modify AGENTS.md in a way that would have made this stuff clear

get_data resolves picks=None to arange directly instead of going through
string-based channel-name machinery on every call; _picks_to_idx gets an
early return for integer arrays already unique and in range (duplicate
picks keep taking the validating path); _mult_cal_one fuses gather,
type-cast, and calibration into a single elementwise pass.
@bruAristimunha

Copy link
Copy Markdown
Contributor Author

All set:

  • Taken out of draft — ready for review from your end.
  • Fragment rewritten per your suggestion as 14212.newfeature.rst:
    Sped up reading by optimizing small read paths, by `Bruno Aristimunha`_
    ``` (name anchor already existed in `doc/changes/names.inc`).
    
  • AGENTS.md now documents the fragment conventions (type-by-intent,
    name-link ending, names.inc requirement) so this doesn't recur.

@larsoner

larsoner commented Aug 25, 2026

Copy link
Copy Markdown
Member

I don't see the AGENTS.md change?

Screenshot 2026-08-25 at 11 29 36 AM

@bruAristimunha

Copy link
Copy Markdown
Contributor Author

sorry, it was my rule configuration of not pushing agents' things, i am pushing at the moment.

Adds the performance-changelog entry and captures in AGENTS.md the two
conventions this effort leaned on: how to write changelog fragments
(type by intent — performance work is newfeature; one-sentence entries
ending with a contributor name link anchored in names.inc) and the
interleaved-A/B benchmarking discipline used to validate these speedups.
@bruAristimunha

bruAristimunha commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

AGENTS.md is updated on the branch with the fragment rules and the A/B benchmarking discipline.

@larsoner larsoner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking for merge-when-greene, thanks in advance @bruAristimunha !

@larsoner larsoner moved this from Ideas to In Progress in 2026 MNE-Python Maintainers Sprint Aug 25, 2026
@larsoner
larsoner merged commit 783e725 into mne-tools:main Aug 25, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants