Decode Persyst and Nihon Kohden data in cache-sized blocks - #14251
Merged
larsoner merged 2 commits intoAug 30, 2026
Conversation
Both readers materialize the entire requested range before calibrating it, so
every intermediate is as large as the request and none of it stays in cache.
Persyst had a hand-rolled copy of _read_segments_file plus one extra full-size
temporary: np.reshape(record, (n_chs, -1), order='F').astype(np.float32).
_mult_cal_one already casts, so that .astype was a whole-array copy for nothing.
The method is now a call to the shared helper -- a net deletion.
Nihon Kohden reads the whole request and then builds four full-size temporaries
over it (+ 0x8000, .astype(int16), * cal to float64, += / *=). Same work, now a
block at a time.
raw.get_data(), interleaved cross-process medians:
Persyst 107 MB 88.7 -> 39.3 ms 2.26x
Nihon 106 MB 267.6 -> 147.7 ms 1.81x
Persyst shipped 1.05x
Nihon shipped 0.995x (1600 reps; fits one block)
Persyst's gain splits cleanly: 1.32x from dropping the redundant .astype alone,
the rest from the smaller block.
The two constants differ (16 MiB Persyst, 1 MiB Nihon) because the optimum
tracks time points per block, not bytes -- Nihon has 25 channels and builds four
temporaries per block, and its sweep improves down to ~256 KiB before falling
off a cliff at 64 KiB, so 1 MiB is chosen for margin.
One behaviour note: dropping Persyst's .astype(np.float32) changes results for a
32-bit .dat whose raw counts exceed 2**24, where float32 was silently rounding
them. That is a precision fix rather than a regression; 16-bit files and the
shipped 32-bit fixture are unaffected.
Output is bit-identical across every readable Persyst and Nihon fixture,
including the synthesised large ones.
bruAristimunha
requested review from
agramfort,
drammock and
larsoner
as code owners
August 29, 2026 19:59
Member
|
A speedup plus fewer (executed) lines, love it, thanks @bruAristimunha ! |
larsoner
added a commit
to larsoner/mne-python
that referenced
this pull request
Sep 2, 2026
* upstream/main: (22 commits) Avoid copying all epochs data in GetEpochsMixin._getitem (mne-tools#14262) Add Report.save(only_if_changed=True) (mne-tools#14261) Add jamica to related software [ci skip] (mne-tools#14260) Remove debugging cruft (mne-tools#14259) ENH: add Raw annotation span conversion (mne-tools#14240) Interactive dipole fitting: add STC mesh controls (mne-tools#14256) Document code principles in AGENTS.md (mne-tools#14239) MAINT: Update dependency specifiers (mne-tools#14257) [dependabot]: Bump the actions group with 2 updates (mne-tools#14258) ENH: Add JAMICA as an ICA method (mne-tools#14247) Reuse the MEF session across reads (mne-tools#14254) Read KIT data in cache-sized blocks (mne-tools#14255) Read EGI simple-binary event channels in blocks (mne-tools#14250) Decode Persyst and Nihon Kohden data in cache-sized blocks (mne-tools#14251) Normalize byte order before calibrating strided integer buffers (mne-tools#14252) Speed up EDF and BDF reading (mne-tools#14237) ENH: Add Forward-based projection reconstruction (mne-tools#14235) Read Artemis123, Curry, EEGLAB, Eximia, FIL and NSx in cache-sized blocks (mne-tools#14246) Remove rotating dipoles capability from interactive dipole fitting GUI (mne-tools#14243) Read BrainVision data in cache-sized blocks [ci skip] (mne-tools#14241) ...
larsoner
added a commit
to natinew77-creator/mne-python
that referenced
this pull request
Sep 3, 2026
* upstream/main: (37 commits) Speed up evoked time plotting (mne-tools#14249) Persist the numba JIT cache, sysmon coverage, refleak freeze (mne-tools#14265) Fix interpolate_to spline target positions (mne-tools#14266) CI reduce package builds on pull requests (mne-tools#14264) Avoid copying all epochs data in GetEpochsMixin._getitem (mne-tools#14262) Add Report.save(only_if_changed=True) (mne-tools#14261) Add jamica to related software [ci skip] (mne-tools#14260) Remove debugging cruft (mne-tools#14259) ENH: add Raw annotation span conversion (mne-tools#14240) Interactive dipole fitting: add STC mesh controls (mne-tools#14256) Document code principles in AGENTS.md (mne-tools#14239) MAINT: Update dependency specifiers (mne-tools#14257) [dependabot]: Bump the actions group with 2 updates (mne-tools#14258) ENH: Add JAMICA as an ICA method (mne-tools#14247) Reuse the MEF session across reads (mne-tools#14254) Read KIT data in cache-sized blocks (mne-tools#14255) Read EGI simple-binary event channels in blocks (mne-tools#14250) Decode Persyst and Nihon Kohden data in cache-sized blocks (mne-tools#14251) Normalize byte order before calibrating strided integer buffers (mne-tools#14252) Speed up EDF and BDF reading (mne-tools#14237) ...
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.
What does this implement/fix?
Two readers materialise the entire requested range before calibrating it, so
every intermediate is as large as the request and none of it stays in cache.
Persyst had a hand-rolled copy of
_read_segments_file()plus one extrafull-size temporary:
np.reshape(record, (n_chs, -1), order="F").astype(np.float32)._mult_cal_one()already casts, so that.astypewas a whole-array copy fornothing. The method is now a call to the shared helper — a net deletion.
Nihon Kohden reads the whole request and then builds four full-size
temporaries over it (
+ 0x8000,.astype(int16),* calto float64,+=/*=).Same work, now a block at a time.
Numbers
Median of 5 interleaved process pairs against a pristine
mainworktree:Persyst's gain splits cleanly: 1.32x from dropping the redundant
.astypealone (shared helper at its 100 MB default), the rest from the smaller block.
The Nihon shipped-fixture figure is a median of 8 runs x 200 reps — at 0.36 ms a
single-shot comparison is pure noise (an early 7-rep run read 0.86x). That file
is 5800 samples and fits in one 1 MiB block, so it keeps its current code path
exactly.
Why the two constants differ
The optimum tracks time points per block, not bytes. Nihon has 25 channels and
builds four temporaries per block; its sweep improves monotonically down to
~256 KiB before falling off a cliff at 64 KiB, so 1 MiB is chosen for margin.
Persyst sits at 16 MiB. This is the same concern I raised on #14246 — a
max_block_samples=parameter would express it better than a per-reader bytebudget, and I am happy to go that way instead.
One behaviour change worth flagging
Dropping Persyst's
.astype(np.float32)changes results for a 32-bit.datwhose raw counts exceed
2**24, where float32 was silently rounding them. Thatis a precision fix rather than a regression, and 16-bit files (and the shipped
32-bit fixture, whose counts are well inside the range) are unaffected.
Correctness
mainacross every readable Persyst and Nihon Kohdenfixture, including the synthesised large ones (
np.array_equal, data andannotations).
pytest mne/io/persyst mne/io/nihon mne/_fiff: 164 passed.Large files were synthesised by tiling — Persyst derives its sample count from
the
.datfile size, and Nihon's single data block was tiled withrecord_durationpatched. No shipped fixture is big enough to show the effect,so CI cannot demonstrate it.
Additional information
AI disclosure: I directed the work and reviewed and tested every change; Claude
Code (Claude Opus 5) profiled the readers, ran the A/B measurements and made the
edits under my direction.