Repository navigation
ENH: allow dirty trees for local-only benchmarks, part 2 - #89
Merged
Merged
Conversation
ev-br
force-pushed
the
dirty-trees-code
branch
3 times, most recently
from
November 1, 2026 14:19
32cf353 to
b647d50
Compare
This was referenced Nov 2, 2026
ev-br
force-pushed
the
dirty-trees-code
branch
from
November 5, 2026 04:25
b647d50 to
c19eba3
Compare
This is a follow-up PR: - benchx-dev#83 adds the design docs - this PR is branched off benchx-dev#81, and needs a rebase once that one lands
The two asv tests that came in with setup_scope assert the exact quality warnings of a result. With dirty trees allowed in local comparisons, a result also carries the dirty-* warnings the test earns by running in a tree with uncommitted files, so compare through warnings_of(), as the collector and pyperf tests do. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
ev-br
force-pushed
the
dirty-trees-code
branch
from
November 5, 2026 15:58
c19eba3 to
3d25cc7
Compare
rok
previously approved these changes
Nov 6, 2026
rok
left a comment
Member
There was a problem hiding this comment.
This looks great, let's merge.
One thought - we might want to be able to send dirty tree results into the central store (if it exists) for convenience and also allow it in comparisons. I'm not sure if we cover this now or not, but we probably should.
Collaborator
Author
|
Opened #106 to discuss handling of dirty trees. My impression is that our strategy for comparisons is on a strict side for local usage---but then we want to balance local and tracking usages, so it might need some general design work after 0.1.0 is a thing. |
ev-br
pushed a commit
to ev-br/benchx
that referenced
this pull request
Nov 6, 2026
The rest of this commit (allow dirty trees in local comparisons) is already in main as benchx-dev#89; only these two documentation hunks were missing. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
MaanasArora
pushed a commit
that referenced
this pull request
Nov 7, 2026
* DOC: describe the numpy-revisions e2e test; update the test count The rest of this commit (allow dirty trees in local comparisons) is already in main as #89; only these two documentation hunks were missing. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * ENH: leave out/ folders out of the tree id, like __pycache__/ A script that gives each `bx bench` call its own directory inside the tree (`--out out/NAME`) changed the tree between calls: call 2 saw call 1's directory as an untracked file and recorded another subject_tree, so the comparison failed `sides-agree` (variants, environments) or `one-tree-per-side` (revisions). git_identity now excludes `**/out/**`, only when the repository tracks nothing under an `out/` (a blanket exclusion would give a clean checkout a tree id that differs from HEAD's). directory_identity skips `out`. UC-02 plan section 5.0, step 1 of its commit order. Applies to every profile and stands on its own. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * MAINT: share the run's measurands and the workload label Move two pieces out of the code that used them, so that the next change can reuse them instead of copying them: - compare: `_run_documents`, the measurands of one run in a fixed order (and the refusal of a run with none), which `compare()` built inline; - projection: `workload_label`, the `name [k=v, ...]` text `bx bench` prints for a case, which `bench.row_of` built inline. No change in behavior for any valid result: the same results are selected in the same order, and the same labels are printed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * ENH: comparator: the variants profile (UC-02) A comparison whose sides are the two values of one workload parameter, in one revision and one environment. The parameter is the profile's axis, added per comparison (`parameter=`, a token, so the document's `varying` stays one); everything else must agree, the tree and the suite included. - A unit is named by what both sides share: `_workload` drops the varied parameter, and `render` prints the rest with `workload_label` (`bench_truncate [size=1000]`). - A failed attempt (partial, censored, error) on either side makes its unit indeterminate with reason `side-failed`, so a variant that times out in the slow rounds is not judged on the fast ones (UC-02 §10). `skipped` stays an ordinary exclusion, and the other profiles keep their rule. - `alternating-slots` names the ways to get an alternating run. - `infer_parameter` gives read mode the run's only parameter. UC-02 plan section 5.1 and 5.5 (read mode), step 2 of its commit order. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * ENH: bx compare --run --profile variants [--vary NAME] [--baseline VALUE] Read mode for the variants profile: compare a stored run whose results alternate two values of one workload parameter, such as one `bx bench` call per value with --run-key, --round and --slot. - `--vary NAME` names the parameter. Without it the run's only parameter is used and said so on stderr; several are refused, naming them. Refused with the other profiles, and `--label` is refused with variants. - `--baseline` is the recorded value, quotes included ('baseline'). - The profile list comes from the comparator: `session.PROFILES`, the `--profile` choices, and the config schema's `compare.profile` enum (a test keeps the three equal). - Measure mode for variants is refused until its own step. test_uc02.py runs the loop through the CLI on a pyperf module and an asv benchmark directory: one parameter, a second parameter with --vary, a single bench call (alternating-slots), a directory per call inside an untracked-.gitignore tree, and an uncommitted edit. UC-02 plan sections 5.3 (read mode), 5.4 and 5.5, step 3 of its commit order. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * MAINT: share the unknown-tree refusal, the dirty-side notes and the filter text Move three pieces out of the code that used them, so that the next change can reuse them for a session of one target instead of copying them: - session: `_refuse_unknown_tree` and `_note_local_only`, which `compare_targets` built inline (the refusal of a side with no working-tree id, UC-03 §10, and the "will be local-only" notes); - benchx_params: `_search_text`, the `name(p0, p1)` text `expand` searches a case filter in. No change in behavior: same messages, same conditions, same order. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * ENH: bx compare TARGET --profile variants: measure mode for one target UC-02 in one command: alternate the two values of one workload parameter of the benchmark over --rounds rounds, deliver the results, and compare. bx compare . --profile variants --suite bench_suite.py --rounds 5 `session.compare_variants` lists the cases once, with the user's --filter, and finds the parameter (--vary, else the only one the selected benchmarks and the `-e` variables give; several are refused, naming who declares each). Each round runs one order per value: that value's cases only, picked by `benchx_params.search_name` through the filter the runner already understands, baseline at slot 2r and the other at 2r + 1. Benchmarks without the parameter run on neither side and are reported. Refused before anything runs: no parameter; a parameter the selected benchmarks do not all take the same two values of (so `--filter` can pick two of three); a --baseline that is not one of them, which lists the values as they must be typed; a tree with no working-tree id; Google Benchmark, which has no parameters. A dirty tree is noted before measuring and the comparison is local-only, as for revisions. `--baseline` is allowed in measure mode for variants only, and `bx compare` takes one target for variants. UC-02 plan section 5.2, the rest of 5.3 and 5.5, step 4 of its commit order. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * DOC: the variants profile (UC-02): scope, design, schema, comparator, READMEs - prototype-scope: variants is in the prototype, for one workload parameter per comparison (several at once stay deferred); the one-target form of `bx compare`. - prototype-design: the commands, the loop of `bx bench` calls with --filter or a case name, `bx compare TARGET --profile variants` (measure mode, its refusals before running), the profile row, which parameter is varied, the unit labels, and the side-failed rule. - benchmark-result-schema 5.5: the variants row loses "role by default" and gains one-benchmark-tree; a comparison has two sides, so the chosen parameters take exactly two joint assignments; the one-benchmark-tree paragraph covers revisions and variants. - comparator 3: one-benchmark-tree under revisions and variants; the strict status rule of variants (side-failed), not an invariant. - prototype/README: the benchmark, the command and the loop. The examples/matmul-params README changes (install stanzas, "Comparing numpy and torch") are not in this commit: the example moved to benchx_examples. UC-02 plan section 7, step 5 of its commit order. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * ENH: comparator: list units with numbers in natural order Units were listed in the order of their canonical JSON, a string order: the sizes of a parametrized benchmark came out as n=10, n=100, n=50. Sort by workload name, then parameter name and value, then quantity, through `projection.axis_order`, so numbers inside names and values sort as people read them (n=10, n=50, n=100; bench_2 before bench_10). This changes the order of `units` in every comparison document and in the rendered table, for every profile. Which units there are, and what they say, is unchanged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> --------- Co-authored-by: ev-br's Claude Code Agent <agent@localhost> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
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
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.
Following up to gh-83 which laid the design groundwork, this PR adds an implementation.
The code change is on top of a "stack" of up to gh-81, so here's a PR on my fork which only has the relevant changes:
rebased.ev-br#7
Will also need to remove the plan file before landing.