Skip to content

chore: add benchmark suite (wrapper micro-benchmarks + e2e sentinels) with PR head-vs-base reporting - #109

Merged
FBumann merged 8 commits into
mainfrom
feat/benchmark-restructure
Jul 23, 2026
Merged

chore: add benchmark suite (wrapper micro-benchmarks + e2e sentinels) with PR head-vs-base reporting#109
FBumann merged 8 commits into
mainfrom
feat/benchmark-restructure

Conversation

@FBumann

@FBumann FBumann commented Jul 23, 2026

Copy link
Copy Markdown
Owner

Supersedes #108 (standalone — includes its commits).

Summary

  • benchmarks/test_bench_aggregate.py — pytest-benchmem suite in three layers (15 cases, deterministic IDs and per-call seeded data):
    • test_wrapper_* — micro-benchmarks of the wrapper's own conversion stages (_to_dataframe, _result_from_tsam, _concat_results) at wide/multi-slice inputs with 10 rounds. Profiling showed ~90% of an end-to-end run is inside tsam, which this repo cannot regress — its PRs can only regress these stages, so they get the tightest statistics and regressions are attributed to a stage.
    • test_config_* — representation and extremes cost ratios at 90 days (config effects are multiplicative, so ratios match 365 days at a quarter of the cost).
    • test_e2e_* — five full-pipeline sentinels at production size (default, full config = hierarchical + Distribution(scope="global") + extremes replace, the 128-column medoid/dist_global interaction, 8 slices) for integration surprises like tsam version bumps.
  • .github/workflows/benchmarks.yaml — informational reporting adapted from fluxopt's benchmark-hint pattern: benchmarks PR head and base in the same job (same runner, pinned benchmem), keeps one sticky PR comment with a single severity-sorted benchmem compare --diff table (min walltime + memray peak Δ%), mirrors it to the step summary, and uploads an interactive benchmem plot artifact. continue-on-error — never blocks a merge. Fork PRs run too (read-only token gets summary + artifact); only the sticky comment is repo-guarded. persist-credentials: false since benchmarks execute PR code.
  • benchmarks/README.md — local run/baseline/compare/plot/flamegraph usage.
  • .benchmarks/ gitignored (baselines are machine-specific).

Findings that shaped the case selection

  • Distribution(scope="global") is the only expensive config option: 2–2.6× over medoid at 8 columns, 3× at 128 (tsam's durationRepresentation global branch is an unvectorized per-attribute loop — upstream issue candidate).
  • extremes replace, dist_cluster, mean, and preserve_column_means are free within noise; slices scale exactly linearly (sequential loop).
  • Memray peak columns are deterministic (exact 0.0% deltas on unchanged code), making them the most reliable regression signal on noisy runners.

🤖 Generated with Claude Code

FBumann and others added 5 commits July 23, 2026 20:05
…ting

Benchmark aggregate() under the production config focus (hierarchical,
Distribution scope=global, include_period_sums=False, extremes replace):
a deterministic representation x extremes grid, the representation x
columns interaction, preserve_column_means, and column/slice/day scaling.

The Benchmarks workflow adapts fluxopt's sticky-comment reporting: it
runs the suite on the PR head and base in the same job, posts a benchmem
delta table (walltime + memray peak) as one sticky PR comment, and
uploads an interactive benchmem plot artifact. Informational only, never
blocks a merge. Local baselines land in .benchmarks/ (gitignored,
machine-specific).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ruff-action tracks the latest ruff, which now formats Python code
blocks inside markdown and rejects aligned trailing comments.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Seed the RNG per make_data call so each case's data is independent of
test selection and execution order. Add a 365-day case so the day axis
scales within its own test. Report one severity-sorted table (group by
module, sort by change). Run fork PRs too — only the sticky comment
needs a writable token, so only that step is repo-guarded — and stop
persisting the checkout token since benchmarks execute PR code. Use
standalone wording when there is no base run to compare against.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…entinels

Profiling showed ~90% of an end-to-end run is inside tsam, which this
repo cannot regress. Benchmark the wrapper's own conversion stages
directly (_to_dataframe, _result_from_tsam, _concat_results) with ten
rounds and wide inputs, measure config cost ratios at 90 days where the
effects are multiplicative anyway, and keep five full-pipeline sentinels
at production size. 15 cases instead of 36, roughly half the runtime,
tighter statistics where this repo's PRs actually land.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@FBumann, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 22 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 97e2f3d7-b156-4bd4-828a-a0e696833d90

📥 Commits

Reviewing files that changed from the base of the PR and between 6cd9f5d and b613380.

📒 Files selected for processing (6)
  • .github/workflows/benchmarks.yaml
  • .gitignore
  • README.md
  • benchmarks/README.md
  • benchmarks/test_bench_aggregate.py
  • docs/index.md
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/benchmark-restructure

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@FBumann
FBumann changed the base branch from feat/benchmarks to main July 23, 2026 19:47
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@FBumann FBumann changed the title chore: restructure benchmarks into wrapper micro-benchmarks and e2e sentinels chore: add benchmark suite (wrapper micro-benchmarks + e2e sentinels) with PR head-vs-base reporting Jul 23, 2026
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

Benchmarks

Δ% vs base, one table sorted by biggest change first
(min walltime and memray peak). Peak memory is
deterministic — any delta there is real; walltime on GitHub
runners is noisy, so only large time deltas are meaningful.
The interactive plot is attached as the benchmark-plot artifact.

Full table

benchmarks/test_bench_aggregate.py

name metric base head
test_user_disaggregate time 0.01189s +4.1%
peak 26.7 MiB 0.0%
test_e2e_slices time 1.185s +3.6%
peak 14.3 MiB 0.0%
test_config_representation[dist_global] time 0.123s +3.1%
peak 1.78 MiB 0.0%
test_config_representation[dist_cluster] time 0.04893s +2.9%
peak 1.78 MiB 0.0%
test_config_representation[mean] time 0.04734s +1.6%
peak 1.78 MiB 0.0%
test_e2e_multidim time 0.224s +1.6%
peak 26.7 MiB 0.0%
test_user_apply time 0.3959s +1.4%
peak 104 MiB 0.0%
test_config_representation[medoid] time 0.05028s +1.3%
peak 1.9 MiB 0.0%
test_e2e_full time 0.15s +1.0%
peak 6.6 MiB 0.0%
test_e2e_default time 0.08032s +1.0%
peak 7.11 MiB 0.0%
test_wrapper_concat_results time 0.01969s +1.0%
peak 8.72 MiB 0.0%
test_config_representation[dist_global_minmax] time 0.1259s +0.9%
peak 1.78 MiB 0.0%
test_config_extremes[append] time 0.05169s +0.1%
peak 1.9 MiB 0.0%
test_e2e_wide[medoid] time 0.514s -0.4%
peak 113 MiB 0.0%
test_e2e_wide[dist_global] time 1.849s -0.8%
peak 104 MiB 0.0%
test_wrapper_to_dataframe time 0.0003702s -2.6%
peak 8.55 MiB 0.0%
test_config_extremes[replace] time 0.05198s -2.8%
peak 1.9 MiB 0.0%
test_wrapper_result_conversion time 0.03803s -9.0%
peak 60.3 MiB 0.0%

FBumann and others added 2 commits July 23, 2026 21:54
Sub-millisecond micro-benchmarks now time 10 iterations per round so the
min converges instead of sampling single-call jitter; all cases get a
warmup round so import and allocator effects stay out of the min; the
90-day config cases get five rounds; GC is disabled during CI timing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cover the remaining user paths with real cost: the multi-cluster_dim
MultiIndex stack path, reusing a stored clustering on new data via
clustering.apply, and expanding cluster-level data back to the full
time axis via disaggregate. Accuracy metrics and reconstructed need no
extra cases — tsam computes them eagerly inside aggregate(), so every
e2e case and the result-conversion micro already include them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@FBumann
FBumann merged commit 3fccdfc into main Jul 23, 2026
10 checks passed
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.

1 participant