Repository navigation
Pack format 16, 24 and 32 samples without per-byte arrays - #607
Merged
Merged
Conversation
tompollard
reviewed
Oct 31, 2026
tompollard
left a comment
Member
There was a problem hiding this comment.
Thanks, this looks great, and the tests against a sample-by-sample reference are a nice touch!
I checked it independently:
- The written bytes are identical to
mainfor formats 80, 212, 16, 24, 32 and 516, with int16/int32/int64 input, C-ordered, Fortran-ordered and sliced arrays, odd and even lengths, and multi-frequency signals. The input array is never modified. - Removing the up-front copy is safe: the FLAC branch only shifts arrays that
astypehas already copied. - For a 1-hour, 8-channel signal at 500 Hz, peak memory went from 494 to 27 MiB for format 16, 755 to 96 MiB for format 24, and 989 to 55 MiB for format 32.
TheNetherWatcher
force-pushed
the
faster-dat-packing
branch
2 times, most recently
from
October 31, 2026 18:41
5e319be to
39fec24
Compare
wr_dat_file split every sample into separate int64 arrays for each byte, then interleaved and cast them, using about 4 bytes of memory per byte written. Convert to little-endian 16 or 32 bit integers and view the result as bytes instead (dropping the top byte for format 24), as format 80 already does. The input is no longer copied up front. Formats 80 and 212 now compute their offsets out of place, so the caller's array is still not modified. Closes #464.
TheNetherWatcher
force-pushed
the
faster-dat-packing
branch
from
October 31, 2026 18:48
39fec24 to
053a088
Compare
tompollard
added a commit
that referenced
this pull request
Oct 31, 2026
`check_sig_cohesion` finds the minimum and maximum of each channel to check that the digital samples fit the format. It used Python's built-in `min` and `max`, which go through a NumPy array one element at a time, so this check took about 85% of the time of `wrsamp` for large records. This uses `np.min` and `np.max` instead, in both the single-frequency and the expanded (multi-frequency) path. Time for a 4-hour, 8-channel record at 500 Hz (57.6 million samples), written from digital samples: | Format | check_sig_cohesion before → after | wrsamp before → after | |---|---|---| | 16 | 2.41 → 0.07 s | 2.85 → 0.45 s | | 212 | 2.41 → 0.08 s | 3.07 → 0.78 s | The written files are unchanged. There were no tests for this check, so I added `TestSigCohesion`: a value just outside the range in the last sample is rejected (in both paths), and the smallest and largest values of each format are allowed. These pass on `main` as well, and fail if the check skips the last sample. Related: #607 speeds up the byte packing in the same `wrsamp` call. The two don't touch the same lines, apart from both adding a changelog entry and a test class at the end of the file. `pytest -m "not network"` passes (101 tests), and the docs build passes.
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.
Closes #464.
wr_dat_filepacked formats 16, 24 and 32 by masking every sample into a separate int64 array for each byte, interleaving those arrays, and then casting touint8. That uses about 4 bytes of memory for every byte written, plus an up-front copy of the whole signal.As @bemoody suggested in #464, this now converts the signal to little-endian 16 or 32 bit integers and views the result as bytes (dropping the top byte of each sample for format 24), like format 80 already does. The explicit
<keeps the output little-endian on any machine.The up-front copy is gone too. It was only there because formats 80 and 212 changed the array in place, so those two now compute their offsets out of place instead. The caller's array is still never modified.
Peak memory and time of
wr_dat_filefor 4 hours × 8 channels at 500 Hz (460 MB of int64 samples), best of 3:The written files are unchanged. The new
TestWrDatFiletests compare the bytes for every writable format against a sample-by-sample reference (int.to_bytesand the format 212 bit layout) for odd and even lengths, the format's minimum and maximum values, Fortran-ordered or sliced input, and multi-frequency (expanded) signals. They also check that the input array isn't modified. They pass onmainas well, and fail if the byte order or the dropped byte for format 24 is changed.pytest -m "not network"passes (121 tests), and the docs build passes.