Stop sorting a whole column to size error-bar caps - #264
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesPlotting heuristics
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Merging this PR will improve performance by 25.53%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_first_payload_errorbar_large |
769.7 ms | 613.1 ms | +25.53% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing alek/errorbar-payload-cost (2e4b8ad) with main (119a1ce)
Footnotes
-
2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Alek99
force-pushed
the
alek/errorbar-payload-cost
branch
from
July 24, 2026 22:13
8045a4e to
c28a404
Compare
`test_first_payload_errorbar_large` is the most expensive benchmark in the suite (~557 ms on CodSpeed). Two thirds of it is work neither answer needs. `_auto_cap_size` wants the median adjacent gap between distinct positions, and reached for `np.unique` — which sorts unconditionally, an O(N log N) pass over the full column. Error-bar positions are usually an ordered independent variable, so one O(N) diff both proves the column is already non-decreasing and yields those gaps directly; only an out-of-order column still pays for the sort. Same distinct values in the same order, so the median is identical. `_zero_baseline_anchor` compacted `base`/`value` down to their finite rows before asking three all-or-nothing questions about them, allocating and copying two full columns per axis per build. Each question is really "does any row violate this?", which the finite mask answers in place. NaN rows are excluded by the mask exactly as the compaction excluded them. Measured on the benchmark's shape (1M points, yerr=1.0, decimated tier), min-of-3: first payload 48.07 ms -> 14.09 ms -70.7% _auto_cap_size 27.48 ms -> 2.92 ms -89.4% (1M sorted positions) _zero_baseline 4.44 ms -> 1.99 ms -55.2% (3M rows) Both sit on real build paths, not just the benchmark: cap sizing runs for every auto-cap error bar, and the zero-baseline probe runs for every bar and histogram build. `_auto_cap_size` returns identically over sorted, unsorted, descending, duplicate-heavy, all-equal, single, empty, NaN-bearing and -0.0-bearing columns; `_zero_baseline_anchor` over all-positive, all-negative, mixed, nonzero-base, NaN-bearing, all-NaN and all-zero columns.
Alek99
force-pushed
the
alek/errorbar-payload-cost
branch
from
July 24, 2026 22:37
c28a404 to
2e4b8ad
Compare
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.
What
test_first_payload_errorbar_largeis the most expensive benchmark in the suite (~557 ms on CodSpeed; nothing else on main is close). Two thirds of it is work neither answer needs._auto_cap_sizewants the median adjacent gap between distinct positions, and reached fornp.unique— which sorts unconditionally, an O(N log N) pass over the full column. Error-bar positions are usually an ordered independent variable, so one O(N) diff both proves the column is already non-decreasing and yields those gaps directly; only an out-of-order column still pays for the sort. Same distinct values in the same order, so the median is identical._zero_baseline_anchorcompactedbase/valuedown to their finite rows before asking three all-or-nothing questions about them, allocating and copying two full columns per axis per build. Each question is really "does any row violate this?", which the finite mask answers in place. NaN rows are excluded by the mask exactly as the compaction excluded them.Numbers
Benchmark shape (1M points,
yerr=1.0, decimated tier), min-of-3:_auto_cap_size(1M sorted positions)_zero_baseline_anchor(3M rows)Neither of these is benchmark-only work. Cap sizing runs for every auto-cap error bar, and the zero-baseline probe runs for every bar and histogram build, so the win follows real charts.
Correctness
Both functions were checked against their previous implementations, not just spot-checked:
_auto_cap_sizereturns identically over sorted, unsorted, descending, duplicate-heavy, all-equal, single-element, empty, NaN-bearing and-0.0-bearing columns._zero_baseline_anchorreturns identically over all-positive, all-negative, mixed-sign, nonzero-base, NaN-bearing, all-NaN and all-zero columns.Full Python suite: 2222 passed, 4 skipped.
ruff checkandruff format --checkclean.Summary by CodeRabbit