Decode Persyst and Nihon Kohden data in cache-sized blocks - #14251
Open
bruAristimunha wants to merge 2 commits into
Open
Decode Persyst and Nihon Kohden data in cache-sized blocks#14251bruAristimunha wants to merge 2 commits into
bruAristimunha wants to merge 2 commits into
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
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.