Move Blackrock nsx handling closer to the buffer API pattern#1820
Open
h-mayorquin wants to merge 2 commits intoNeuralEnsemble:masterfrom
Open
Move Blackrock nsx handling closer to the buffer API pattern#1820h-mayorquin wants to merge 2 commits intoNeuralEnsemble:masterfrom
h-mayorquin wants to merge 2 commits intoNeuralEnsemble:masterfrom
Conversation
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.
PR #1513 introduced
BaseRawWithBufferApiIO, a base class that stores lightweight buffer descriptions (file paths, offsets, dtypes) instead of persistentnp.memmapobjects, creating short-lived mmap views on demand through cached file descriptors. This PR moves BlackrockRawIO toward that same pattern. We cannot fully adoptBaseRawWithBufferApiIObecause: (a) Blackrock's standard format (v2.2/2.3/3.0) stores multiple data blocks with explicit headers in a single file, requiring block-aware offset tracking that the base class's one-buffer-per-stream model does not support; (b) the PTP variant interleaves per-sample timestamps with sample data in fixed-size packets, requiring strided mmap access (np.ndarraywith customstrides) rather than contiguous views; and (c) the segmentation logic for PTP files needs to read timestamps from the file to detect gaps, which couples parsing and segmentation in a way the base class does not allow. However, we can adopt the core idea: replace stored memmaps with on-demand creation.This has the added benefit of reducing the number of memmaps that are open at any time and we have seen that this pattern is both 1) more performant memory wise, 2) less likely to lead to leaks. Importantly, this PR does not change any behavior of the reader, it is an internal optimization only. It lays the groundwork for a follow-up PR adding gap detection on the standard format.