perf(processors): moving_windows buffer ping-pong, direct scipy.fft convolution, upsampler slice fill by ggmarshall · Pull Request #217 · legend-exp/dspeed

ggmarshall · 2026-08-10T22:27:27Z

Note

Now performance changes only, top of the stack: #219, #220 (merged) → #221 (correctness fixes) → this. Lower PRs' commits appear in this diff until they merge; I'll rebase after each.

A/B-benchmarked on real L200 p18 waveform blocks through the production build_dsp path; outputs bit-identical.

change	speedup
`moving_windows`: ping-pong one scratch buffer instead of `1+num_mw` copies per call; keep the running average in a register	1.3–1.4×
`fft_convolve_wf`: call `scipy.fft` directly (same `next_fast_len` length), skipping `fftconvolve`'s per-call python overhead	~1.15×
`upsampler`: clipped slice fill instead of per-sample bounds check	1.3–1.7×

Whole stack (all four PRs): ICPC 11.2 → 6.9 s, COAX 13.7 → 8.6 s per channel-file; verified bit-identical end-to-end against a stock-dspeed control run (report).

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR targets performance and correctness in the LEGEND-200 germanium DSP chain by removing hot-loop allocations, preventing unintended input mutation, and hardening centroid/extrema logic—backed by added/updated regression tests.

Changes:

Fix correctness issues in centroid/extrema handling (e.g., get_wf_centroid undefined-crossing behavior; get_multi_local_extrema minima coincidence bug) and adjust wf_alignment to treat NaN centroid as a fallback case.
Introduce utils.contains_nan() and replace many np.isnan(...).any() guards in numba kernels to avoid boolean-array allocations and enable early-exit scanning.
Speed up key processors (moving windows ping-pong buffering, scalar indexing in linear_slope_*, FFT convolution path changes) and improve LH5 load error chaining.

Reviewed changes

Copilot reviewed 38 out of 38 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/processors/test_wf_alignment.py	Updates centroid/NaN alignment expectations and adds a NaN-centroid regression test.
tests/processors/test_get_wf_centroid.py	Adds no-zero-crossing regression coverage for centroid calculation.
tests/processors/test_get_multi_local_extrema.py	Adds regression test covering conservative-search minima correctness.
tests/processors/test_convolutions.py	Adds initial mode/NaN/mutation coverage for convolution processors.
tests/conftest.py	Makes session teardown robust when tmptestdir wasn’t used.
src/dspeed/utils.py	Adds `contains_nan()` numba helper for allocation-free NaN detection.
src/dspeed/processors/windower.py	Switches NaN guard to `contains_nan()`.
src/dspeed/processors/wf_correction.py	Switches NaN guards to `contains_nan()`.
src/dspeed/processors/wf_alignment.py	Uses `contains_nan()` and treats NaN centroid as a non-fatal fallback window.
src/dspeed/processors/upsampler.py	Uses `contains_nan()` and optimizes fill logic via clipped slice assignment.
src/dspeed/processors/trap_filters.py	Switches NaN guards to `contains_nan()`.
src/dspeed/processors/time_point_thresh.py	Switches NaN guards to `contains_nan()` (including array thresholds).
src/dspeed/processors/time_over_threshold.py	Switches NaN guards to `contains_nan()`.
src/dspeed/processors/sort.py	Switches NaN guard to `contains_nan()`.
src/dspeed/processors/soft_pileup_corr.py	Switches NaN guards to `contains_nan()`.
src/dspeed/processors/saturation.py	Switches NaN guard to `contains_nan()`.
src/dspeed/processors/recursive_filter.py	Switches NaN guards to `contains_nan()` for input/coeff arrays.
src/dspeed/processors/rc_cr2.py	Switches NaN guards/checks to `contains_nan()`.
src/dspeed/processors/pulse_injector.py	Switches NaN guards to `contains_nan()`.
src/dspeed/processors/presum.py	Switches NaN guard to `contains_nan()`.
src/dspeed/processors/poly_fit.py	Switches NaN guards to `contains_nan()`.
src/dspeed/processors/pole_zero.py	Switches NaN guards/checks to `contains_nan()`.
src/dspeed/processors/pmt_pulse_injector.py	Switches NaN guards to `contains_nan()`.
src/dspeed/processors/pad.py	Switches NaN guard to `contains_nan()` for the padded input region.
src/dspeed/processors/moving_windows.py	Uses `contains_nan()` and reduces copies via scratch-buffer ping-pong; carries running accumulator in registers.
src/dspeed/processors/min_max.py	Switches NaN guard to `contains_nan()`.
src/dspeed/processors/log_check.py	Switches NaN guard to `contains_nan()`.
src/dspeed/processors/linear_slope_fit.py	Uses `contains_nan()` and fixes hot-loop temporaries by indexing scalar outputs.
src/dspeed/processors/inl_correction.py	Switches NaN guard to `contains_nan()` for INL array.
src/dspeed/processors/histogram.py	Switches NaN guard to `contains_nan()`.
src/dspeed/processors/histogram_stats.py	Switches NaN guards to `contains_nan()` for weights.
src/dspeed/processors/get_wf_centroid.py	Fixes undefined-crossing handling (avoid OOB read) and uses `contains_nan()`.
src/dspeed/processors/get_multi_local_extrema.py	Fixes conservative minima coincidence list source; uses `contains_nan()`.
src/dspeed/processors/fixed_time_pickoff.py	Switches NaN guard to `contains_nan()`.
src/dspeed/processors/convolutions.py	Prevents NaN zeroing from mutating shared inputs; switches FFT implementation to `scipy.fft`.
src/dspeed/processors/bl_subtract.py	Switches NaN guard to `contains_nan()`.
src/dspeed/processors/arithmetic.py	Switches NaN guard to `contains_nan()`.
src/dspeed/processing_chain.py	Fixes LH5 decode exception handling and chains the original exception.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

codecov · 2026-08-10T22:34:11Z

Codecov Report

❌ Patch coverage is 21.31148% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.80%. Comparing base (676b89d) to head (2c56fc1).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
src/dspeed/processors/moving_windows.py	0.00%	41 Missing ⚠️
src/dspeed/processors/upsampler.py	0.00%	6 Missing ⚠️
src/dspeed/processors/convolutions.py	92.85%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #217      +/-   ##
==========================================
- Coverage   69.10%   68.80%   -0.30%     
==========================================
  Files          62       62              
  Lines        4191     4222      +31     
==========================================
+ Hits         2896     2905       +9     
- Misses       1295     1317      +22

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ggmarshall · 2026-08-10T23:15:33Z

see full report here: https://claude.ai/code/artifact/6d1c437c-c519-4797-a510-993d6aa80e98

ggmarshall · 2026-08-31T21:13:39Z

@iguinn can we merge this? gives good speedups, all outputs have ben tested, most of this is just cleaning up the nan check

iguinn · 2026-08-31T22:05:23Z

Can you break this up? Almost all of the gains here are going to come from linear_slope_fit which is called many times, and which I don't have problems with...I'm a little worried about the AI generated tests (which are known to be something they need human input to do well with), and I'd prefer to tackle the other things with this now rather than later.

ggmarshall · 2026-09-01T01:51:34Z

Broken up as requested: #219 was the linear_slope_fit change (thanks for merging), #220 is just the contains_nan guard swap (one commit on main now), and this PR keeps the rest — the correctness fixes (get_wf_centroid OOB read, fft_convolve_wf input mutation, get_multi_local_extrema minima branch, loadlh5), the remaining perf work (moving_windows, fft_convolve_wf via scipy.fft, upsampler), and all of the new tests, so they can be scrutinized here. This branch is stacked on #220; once that merges I'll rebase and the diff here shrinks to just its own changes. The union of the three PRs is byte-identical to the tree that was validated end-to-end against production p18, and the full suite passes on each branch.

moving_window_multi copied the full waveform 1+num_mw times per call; ping-pong between w_out and a single scratch buffer instead, choosing the start buffer so the last pass lands in w_out. Carry the running average in a register (cast back to the element dtype each step) in all three processors: numba cannot prove input/output don't alias, so indexing w_out[i-1] reloaded the accumulator from memory every sample. Bit-identical outputs; 1.3-1.4x on the germanium chain cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Replicates scipy.signal.fftconvolve with the same next_fast_len FFT length, so outputs are bit-identical, while skipping its per-call python plumbing (shape/axis normalization, swap checks, _centered), which is significant at the 16-waveform block size the processing chain uses. ~1.15x on germanium-chain shapes; also drops one intermediate allocation via the in-place spectrum multiply. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…ds check Bit-identical outputs (the NaN prefill still covers gap samples for fractional upsample factors); 1.3-1.7x on germanium-chain shapes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

ggmarshall · 2026-09-01T03:32:24Z

Split further: correctness fixes (with their regression tests) are now #221; this PR keeps only the perf changes, stacked on #221. Both branches rebased onto main now that #220 is merged. The union of the stack is byte-identical to the previously validated tree, and the full suite passes on each branch.

Copilot AI lite review requested due to automatic review settings August 10, 2026 22:27

Copilot started reviewing on behalf of ggmarshall August 10, 2026 22:27

View session

Copilot AI reviewed Aug 10, 2026

View reviewed changes

Comment thread src/dspeed/processors/convolutions.py Resolved

ggmarshall force-pushed the perf-ge-chain branch from a7f0a4a to e3e21a6 Compare August 10, 2026 22:40

iguinn self-requested a review August 12, 2026 16:59

This was referenced Aug 31, 2026

perf(linear_slope_fit): remove per-sample heap allocations (10x processor speedup) #219

Merged

perf(processors): allocation-free early-exit contains_nan guard in all nopython kernels #220

Merged

ggmarshall force-pushed the perf-ge-chain branch from e4c3948 to 3f66538 Compare September 1, 2026 01:51

ggmarshall force-pushed the perf-ge-chain branch from 3f66538 to 3a942f7 Compare September 1, 2026 02:56

ggmarshall mentioned this pull request Sep 1, 2026

fix(processors): get_wf_centroid OOB read, fft_convolve_wf input mutation, get_multi_local_extrema minima branch, loadlh5 error path #221

Merged

ggmarshall and others added 3 commits September 1, 2026 05:32

perf(upsampler): clipped slice fill instead of per-output-sample boun…

628f0a8

…ds check Bit-identical outputs (the NaN prefill still covers gap samples for fractional upsample factors); 1.3-1.7x on germanium-chain shapes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

ggmarshall force-pushed the perf-ge-chain branch from 3a942f7 to 628f0a8 Compare September 1, 2026 03:32

ggmarshall changed the title ~~Germanium-chain correctness fixes and perf: get_wf_centroid OOB read, fft_convolve_wf input mutation, moving_windows/convolutions/upsampler speedups~~ perf(processors): moving_windows buffer ping-pong, direct scipy.fft convolution, upsampler slice fill Sep 1, 2026

Merge branch 'main' into perf-ge-chain

2c56fc1

Navigation Menu

Repository navigation

perf(processors): moving_windows buffer ping-pong, direct scipy.fft convolution, upsampler slice fill - #217

perf(processors): moving_windows buffer ping-pong, direct scipy.fft convolution, upsampler slice fill#217
ggmarshall wants to merge 4 commits into
legend-exp:mainfrom
ggmarshall:perf-ge-chain

ggmarshall commented Aug 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

codecov Bot commented Aug 10, 2026 •

edited

Loading

Uh oh!

ggmarshall commented Aug 10, 2026

Uh oh!

ggmarshall commented Aug 31, 2026

Uh oh!

iguinn commented Aug 31, 2026

Uh oh!

ggmarshall commented Sep 1, 2026

Uh oh!

ggmarshall commented Sep 1, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ggmarshall commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codecov Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ggmarshall commented Aug 10, 2026

Uh oh!

ggmarshall commented Aug 31, 2026

Uh oh!

iguinn commented Aug 31, 2026

Uh oh!

ggmarshall commented Sep 1, 2026

Uh oh!

ggmarshall commented Sep 1, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ggmarshall commented Aug 10, 2026 •

edited

Loading

codecov Bot commented Aug 10, 2026 •

edited

Loading