Skip to content

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

Merged
iguinn merged 1 commit into
legend-exp:mainfrom
ggmarshall:perf-linear-slope-fit
Aug 31, 2026
Merged

iguinn merged 1 commit into
legend-exp:mainfrom
ggmarshall:perf-linear-slope-fit

Conversation

@ggmarshall

@ggmarshall ggmarshall commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Split out of #217 as requested

linear_slope_fit / linear_slope_diff: the Welford update loops did full-array arithmetic on the length-1 output views (mean[:] += ...), which allocates a heap temporary per sample per output. Indexing the outputs as scalars (mean[0]) removes the allocations: 10× per processor call (173 → 17 µs/waveform) on real L200 p18 waveform blocks, benchmarked through the production build_dsp path. Since these processors are called many times per germanium chain, this is the bulk of the whole-chain speedup measured on #217 (ICPC 11.2 s → 6.9 s per channel-file for the full stack).

Numerical note: linear_slope_diff outputs, and linear_slope_fit's mean/std on near-zero float32 data, shift within the last float32 ulps relative to the old array-expression path (≤1.5e-3 ADC absolute on L200 QC quantities, which are near-zero). Verified end-to-end by rebuilding a full 199-channel L200 raw file and comparing column-by-column against a stock-dspeed control run; all other outputs bit-identical. Full report: https://claude.ai/code/artifact/6d1c437c-c519-4797-a510-993d6aa80e98

🤖 Generated with Claude Code

…le loop

The Welford loops did full-array arithmetic on the length-1 output
views, which numba compiles to a heap-allocated temporary array per
sample. Indexing them explicitly keeps the loop in registers: 10x
faster on real waveform blocks (173 -> 17 us/wf for 781-sample f32).

linear_slope_fit outputs are bit-identical. linear_slope_diff shifts by
<= 2e-5 relative (scalar promotion differs from the old array-expression
path in the last ulps of the f32 accumulation).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 31, 2026 22:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is localized and consistent with existing gufunc scalar-output conventions (using [0] indexing) while preserving the prior computation flow.

Pull request overview

This PR removes per-sample heap allocations in the hot Welford update loops inside linear_slope_fit and linear_slope_diff by indexing the scalar (length-1) gufunc outputs as scalars (out[0]) instead of performing full-array arithmetic on them. This aligns with the PR’s stated goal of eliminating temporary arrays in the tight loop to achieve the reported processor-level speedup.

Changes:

  • Update Welford mean/variance updates in linear_slope_fit to operate on mean[0]/stdev[0] scalars rather than array expressions.
  • Update linear_slope_diff to compute residuals using slope[0]/intercept[0] and accumulate into mean[0]/rms[0] scalars.
  • Replace in-place np.sqrt(out, out) usage with scalar out[0] = np.sqrt(out[0]) to avoid array-expression temporaries.
File summaries
File Description
src/dspeed/processors/linear_slope_fit.py Removes per-sample temporary allocations in Welford update loops by switching to scalar indexing on length-1 gufunc outputs/inputs.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

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

@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.74%. Comparing base (7dc457e) to head (a5d6c59).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/dspeed/processors/linear_slope_fit.py 0.00% 10 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #219   +/-   ##
=======================================
  Coverage   66.74%   66.74%           
=======================================
  Files          61       61           
  Lines        4066     4066           
=======================================
  Hits         2714     2714           
  Misses       1352     1352           

☔ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants