Repository navigation
BlackrockRawIO: align the .nev spike dtype to avoid a segfault on numpy 2.0 and 2.1 - #1962
Merged
Merged
Conversation
Contributor
Author
|
Merging this to avoid it getting on the way of other tests |
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.
I merged #1955 (lazy waveform reading) recently but there is a problem there that was not detected by the tests. The reason for this is that the CI matrix of #1955 had numpy 1.26 and 2.3.3, and numpy 2.0.2 only joined it with #1922 (drop NumPy < 2.0) the day before the merge. Right now the IO tests on master crash with a segfault in the numpy 2.0.2 job, and open PRs like #1961 (event extraction on ABF) fail the same way. This PR solves this.
The problem is that in #1955 I built the spike table with a packed dtype and its
timestampfield ends up unaligned in memory. On numpy 2.0 and 2.1 comparing that field against a negative Python int segfaults. This happens in_match_nsx_and_nev_segment_ids, and only when another Blackrock file was parsed earlier in the same process. I can't reproduce it from numpy 2.2 on and I think it is a numpy bug that got fixed there. The fix is simple: building the dtype withalign=Truemakes numpy pad each field to the alignment of its type, at the cost of one byte per spike in most files.I measured the parse memory again on the Trellis file from #1955 and the savings remain as the memory held after
parse_headeronly goes from 0.38 GB to 0.40 GB (measured withtracemalloc).