Migrate the algebra benchmark suite to criterion by JoeyBF · Pull Request #269 · SpectralSequences/sseq

JoeyBF · 2026-07-15T00:45:58Z

Splits the benchmark infrastructure out of the Nassau GPU work (#264) so it can land on its own.

What

Replaces the bencher harness with criterion (plus pprof for flamegraph output).
Adds a general benchmark suite for the algebra crate:
- benches/common/mod.rs — shared helpers for algebra-multiplication and module-action benchmarks, plus Steenrod-module loading.
- benches/milnor.rs — the Milnor multiply, ported to criterion.
- benches/multiplication.rs — element multiplication across Adem/Milnor.
- benches/module_action.rs — module actions.
- benches/nassau_milnor.rs — the Nassau-regime (p = 2) Milnor multiply.

Notes

This is pure benchmark infrastructure — no source-code changes, no dependency on the GPU work. It builds against the current master.

Test plan

cargo check -p algebra
cargo bench --no-run -p algebra — every bench target compiles (milnor, multiplication, module_action, nassau_milnor)

🤖 Generated with Claude Code

Generated by Claude Code

Summary by CodeRabbit

Performance
- Added expanded benchmarking for algebra multiplication, module actions, and Nassau–Milnor multiplication.
- Added coverage across supported primes, algebra representations, and predefined Steenrod modules.
- Added flamegraph profiling and HTML benchmark reports for easier performance analysis.
- Migrated existing Milnor benchmarks to the new benchmarking framework.

Replace the `bencher` harness with `criterion` (plus `pprof` flamegraph output) and add a general benchmark suite for the algebra crate: - `benches/common/mod.rs`: shared helpers for algebra-multiplication and module-action benchmarks, plus Steenrod-module loading. - `benches/milnor.rs`: the Milnor multiply, ported to criterion. - `benches/multiplication.rs`, `benches/module_action.rs`, `benches/nassau_milnor.rs`: new coverage of element multiplication, module actions, and the Nassau-regime Milnor multiply. This is pure benchmark infrastructure with no source-code dependencies, split out of the Nassau GPU work so it can land on its own. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UPYvsLdEfitgCbAiPxxx3U

coderabbitai · 2026-07-15T00:46:12Z

Warning

Review limit reached

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

Next review available in: 45 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 214df96c-8bb8-4550-a750-b1fb9a29d7cc

📥 Commits

Reviewing files that changed from the base of the PR and between 6bab1e2 and d98f323.

📒 Files selected for processing (3)

ext/crates/algebra/benches/common/mod.rs
ext/crates/algebra/benches/milnor.rs
ext/crates/algebra/benches/module_action.rs

📝 Walkthrough

Walkthrough

Changes

The algebra crate migrated benchmarks from Bencher to Criterion, added pprof flamegraph profiling, and introduced shared utilities plus new workloads for multiplication, module actions, and Nassau Milnor multiplication.

Algebra benchmark suite

Layer / File(s)	Summary
Criterion harness migration `ext/crates/algebra/Cargo.toml`, `ext/crates/algebra/benches/milnor.rs`	Benchmark dependencies and targets now use Criterion with profiling, and the Milnor partial-product benchmark uses Criterion harness macros and groups.
Shared benchmark utilities `ext/crates/algebra/benches/common/mod.rs`	Shared helpers measure algebra multiplication and module actions and load compatible Steenrod modules from JSON specifications.
Multiplication workloads `ext/crates/algebra/benches/multiplication.rs`, `ext/crates/algebra/benches/nassau_milnor.rs`	New benchmarks cover selected Adem and Milnor primes plus sampled dense-element multiplication workloads for Nassau’s Milnor kernel.
Module-action workload `ext/crates/algebra/benches/module_action.rs`	Predefined compatible modules are benchmarked for basis and dense actions, while unsupported or unloadable modules are skipped.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Poem

A rabbit hops through benchmarks bright,
With Criterion timing every flight.
Dense vectors thump, Milnor sings,
Flamegraphs bloom on profiling wings.
New algebra paths take delight!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: migrating the algebra benchmark suite to Criterion.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

coderabbitai

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ext/crates/algebra/benches/common/mod.rs`:
- Around line 231-245: Update the loop nesting in the act benchmark so the
inputs/mod_degree loop is outermost and the op_idx loop is inner, matching
act_on_basis. Keep scratch sizing, zeroing, module.act, and black_box within the
per-mod_degree iteration so each degree slice incurs that harness overhead only
once.

In `@ext/crates/algebra/benches/milnor.rs`:
- Around line 18-37: Update the benchmark around
PPartMultiplier::<MOD4>::new_from_allocation to use iter_batched, constructing
or hoisting PPartAllocation::default() in the setup phase so allocation is
excluded from timing. Replace the per-item correctness assertions with
std::hint::black_box on the iteration results, keeping the benchmark focused on
multiplier execution and retaining both MOD4 and non-MOD4 paths as needed.

In `@ext/crates/algebra/benches/module_action.rs`:
- Around line 31-47: Move the BenchmarkGroup creation and finalization out of
the basis loop, keeping one group per spec.name while iterating over supported
AlgebraType values. Preserve the existing module loading, skip behavior, and
bench_module_action calls so Milnor and Adem benchmarks appear side by side in
the same Criterion report.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: aaa095c1-fa60-41dc-bda4-5ddda4fde418

📥 Commits

Reviewing files that changed from the base of the PR and between a3a63cb and 6bab1e2.

📒 Files selected for processing (6)

ext/crates/algebra/Cargo.toml
ext/crates/algebra/benches/common/mod.rs
ext/crates/algebra/benches/milnor.rs
ext/crates/algebra/benches/module_action.rs
ext/crates/algebra/benches/multiplication.rs
ext/crates/algebra/benches/nassau_milnor.rs

- module_action: create one Criterion group per module (outside the base loop) so the Milnor and Adem bases share a report instead of landing in separate same-named runs. - milnor: hoist the `PPartAllocation` into `iter_batched` setup so allocation is excluded from the timed window, and `black_box` the yielded coefficients instead of asserting inside the hot loop. - common: match the `act` benchmark's loop nesting to `act_on_basis` (size/zero scratch and `black_box` once per degree slice) so the two are comparable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UPYvsLdEfitgCbAiPxxx3U

JoeyBF · 2026-07-15T04:58:28Z

I'll just go ahead and merge this since it's self-contained and not user-facing.

* Migrate the algebra benchmark suite to criterion Replace the `bencher` harness with `criterion` (plus `pprof` flamegraph output) and add a general benchmark suite for the algebra crate: - `benches/common/mod.rs`: shared helpers for algebra-multiplication and module-action benchmarks, plus Steenrod-module loading. - `benches/milnor.rs`: the Milnor multiply, ported to criterion. - `benches/multiplication.rs`, `benches/module_action.rs`, `benches/nassau_milnor.rs`: new coverage of element multiplication, module actions, and the Nassau-regime Milnor multiply. This is pure benchmark infrastructure with no source-code dependencies, split out of the Nassau GPU work so it can land on its own. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UPYvsLdEfitgCbAiPxxx3U * Address review: tighten benchmark measurement - module_action: create one Criterion group per module (outside the base loop) so the Milnor and Adem bases share a report instead of landing in separate same-named runs. - milnor: hoist the `PPartAllocation` into `iter_batched` setup so allocation is excluded from the timed window, and `black_box` the yielded coefficients instead of asserting inside the hot loop. - common: match the `act` benchmark's loop nesting to `act_on_basis` (size/zero scratch and `black_box` once per degree slice) so the two are comparable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UPYvsLdEfitgCbAiPxxx3U --------- Co-authored-by: Claude <noreply@anthropic.com>

coderabbitai Bot requested changes Jul 15, 2026

View reviewed changes

Comment thread ext/crates/algebra/benches/common/mod.rs Outdated

Comment thread ext/crates/algebra/benches/milnor.rs

Comment thread ext/crates/algebra/benches/module_action.rs

coderabbitai Bot approved these changes Jul 15, 2026

View reviewed changes

JoeyBF merged commit 4867b30 into SpectralSequences:master Jul 15, 2026
20 checks passed

JoeyBF deleted the claude/algebra-criterion-benches branch July 15, 2026 04:58

This was referenced Jul 15, 2026

Add hash-free seqno index tables to the Milnor algebra #270

Open

GPU offload for Nassau's Milnor multiply (p = 2) #271

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Migrate the algebra benchmark suite to criterion#269

Migrate the algebra benchmark suite to criterion#269
JoeyBF merged 2 commits into
SpectralSequences:masterfrom
JoeyBF:claude/algebra-criterion-benches

JoeyBF commented Jul 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 15, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoeyBF commented Jul 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

JoeyBF commented Jul 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Notes

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoeyBF commented Jul 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JoeyBF commented Jul 15, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 15, 2026 •

edited

Loading