Avoid dense waveform allocation when exporting PC features - #4789
Open
JESUSROYETH wants to merge 1 commit into
Open
JESUSROYETH wants to merge 1 commit into
JESUSROYETH wants to merge 1 commit into
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
1 participant
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.
export_to_phy()computes PCA features by default, but currently gathers a dense(spikes, samples, recording_channels)waveform array before processing one channel at a time. This change uses a zero-copy sliding window and gathers only the waveforms needed by each PCA channel. In the benchmark below, this removes a 276 MiB temporary per worker for the busiest 1 s chunk.The dense gather was introduced in #4488. This keeps its per-channel batched transform and only changes how waveforms are gathered.
I measured a 384-channel recording with 59,636 spikes and default 1 s chunks on a fresh 8-vCPU Linux VM. Results are medians of five alternating runs after warm-up:
n_jobsOn the public 384-channel SpikeGLX fixture, the default cold route including PCA fitting reduced peak RSS from 669.7 to 558.0 MiB; its timing was noisy, so no speed claim is made. Dense analyzers also benefit: with 38,323 spikes and
n_jobs=1, median export time went from 11.802 to 9.371 s.All paired exports produced the same complete
pc_features.npySHA-256. An independent per-spike oracle had maximum absolute error3.58e-7; the principal-component and Phy-export tests pass with 17 tests, and Black is clean.