From 4f63cfda300652f107e2c3a92fdd43ef0957b565 Mon Sep 17 00:00:00 2001 From: ev-br's Claude Code Agent Date: Wed, 28 Oct 2026 18:29:49 +0000 Subject: [PATCH 1/9] 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 --- docs/benchx.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/benchx.md b/docs/benchx.md index 45c62c2..4f21ecf 100644 --- a/docs/benchx.md +++ b/docs/benchx.md @@ -224,9 +224,18 @@ $ pip install benchx ```console $ uv venv && uv pip install -e '.[test]' # editable; a plain `uv pip install .` works too, the schemas ship in the package -$ .venv/bin/python -m pytest tests # 344 tests, fake Google Benchmark binary, no compiler +$ .venv/bin/python -m pytest tests # 470 tests, fake Google Benchmark binary, no compiler ``` +`tests/test_e2e_numpy_revisions.py` is an end-to-end test that builds NumPy with +`spin` and compares two tags in one `bx compare`. It is skipped unless +`BENCHX_E2E_NUMPY` (a numpy git checkout) and `BENCHX_E2E_EXAMPLE` (the +`numpy-revisions` example directory) are set and `spin` is on the PATH; it also +needs the project-file entries of PR #65 and PR #71. `BENCHX_E2E_WORKTREES` +keeps the builds between runs, `BENCHX_E2E_REVISIONS` picks the two tags, +`BENCHX_E2E_SPIN_EXAMPLE` (the `numpy-spin` example directory) adds a single-run +scenario, and the test's docstring has the rest. + A regular (non-editable) install ships the test suite too, so an installed copy can test itself: `pip install 'benchx[test]'` followed by `python -m pytest --pyargs benchx`. From 2eb180758dbbe1bcfdbe484af2e8d19f4f809ac6 Mon Sep 17 00:00:00 2001 From: ev-br's Claude Code Agent Date: Mon, 2 Nov 2026 17:29:37 +0000 Subject: [PATCH 2/9] 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 --- src/benchx/snapshot.py | 19 ++++++++++++++--- tests/test_prototype.py | 45 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/benchx/snapshot.py b/src/benchx/snapshot.py index 76e14c8..e78f714 100644 --- a/src/benchx/snapshot.py +++ b/src/benchx/snapshot.py @@ -110,6 +110,19 @@ def merge(base: dict, facts: dict) -> dict: # Bytecode caches are written by running the suite, so they must not change the tree id. _PYCACHE_EXCLUDE = ":(exclude,glob)**/__pycache__/**" +# So is `out/`, where a script gives each `bx bench` call its own results directory: +# call 2 would otherwise see call 1's directory as an untracked file and record another +# tree id. Left out only when the repository tracks nothing under an `out/`; with a +# tracked `out/x.txt` a blanket exclusion would give a clean checkout a tree id that +# differs from HEAD's, and an edit to that file would leave the tree `clean`. +_OUT_GLOB = ":(glob)**/out/**" +_OUT_EXCLUDE = ":(exclude,glob)**/out/**" + + +def _out_is_untracked(top) -> bool: + """Whether nothing under an `out/` directory is tracked (False when git cannot say).""" + return _run(["git", "ls-files", "--", _OUT_GLOB], cwd=top) == "" + _IGNORED: tuple[Path, ...] = () @@ -142,8 +155,8 @@ def git_identity(path) -> dict | None: head = top and _run(["git", "rev-parse", "HEAD"], cwd=top) if not head: return None - excludes = [_PYCACHE_EXCLUDE, *(f":(top,exclude){rel}" for rel in _ignored_in(top) - if not _git_ignores(top, rel))] + excludes = [_PYCACHE_EXCLUDE, *([_OUT_EXCLUDE] if _out_is_untracked(top) else []), + *(f":(top,exclude){rel}" for rel in _ignored_in(top) if not _git_ignores(top, rel))] status = _run(["git", "status", "--porcelain", "--untracked-files=all", "--", ".", *excludes], cwd=top) # A scratch index that does not exist yet, so the repository's own index # is untouched (schema §4.1). @@ -208,7 +221,7 @@ def benchmark_identity(suite_file, subject_source) -> dict | None: return {"uri": suite.parent.as_uri(), "type": "directory", "tree": tree, "dirty": "dirty"} -_SKIPPED_DIRS = {".git", ".benchx", "__pycache__"} +_SKIPPED_DIRS = {".git", ".benchx", "__pycache__", "out"} def directory_identity(path) -> dict | None: diff --git a/tests/test_prototype.py b/tests/test_prototype.py index 8138cb3..204d77d 100644 --- a/tests/test_prototype.py +++ b/tests/test_prototype.py @@ -667,3 +667,48 @@ def test_git_identity_ignores_pycache_folders(tmp_path): (folder / "mod.cpython-312.pyc").write_bytes(b"\0") assert runner.snapshot.git_identity(src) == before assert before["dirty"] == "clean" + + +def test_git_identity_ignores_out_folders(tmp_path): + """A script that gives each `bx bench` call its own `--out out/NAME` must not change the tree.""" + src = tmp_path / "src" + src.mkdir() + git(src, "init", "-q") + (src / "mod.py").write_text("x = 1\n") + git(src, "add", "-A") + git(src, "commit", "-qm", "one") + before = runner.snapshot.git_identity(src) + for folder in (src / "out" / "call-1", src / "pkg" / "out"): + folder.mkdir(parents=True) + (folder / "result.json").write_text("{}\n") + assert runner.snapshot.git_identity(src) == before + assert before["dirty"] == "clean" + + +def test_directory_identity_ignores_out_folders(tmp_path): + plain = tmp_path / "plain" + plain.mkdir() + (plain / "mod.py").write_text("x = 1\n") + before = runner.snapshot.directory_identity(plain) + for folder in (plain / "out" / "call-1", plain / "pkg" / "out"): + folder.mkdir(parents=True) + (folder / "result.json").write_text("{}\n") + assert runner.snapshot.directory_identity(plain) == before + + +def test_git_identity_keeps_a_tracked_out_folder(tmp_path): + """With a file under `out/` tracked, the folder is not excluded: editing it makes the tree dirty.""" + src = tmp_path / "src" + (src / "out").mkdir(parents=True) + git(src, "init", "-q") + (src / "mod.py").write_text("x = 1\n") + (src / "out" / "x.txt").write_text("one\n") + git(src, "add", "-A") + git(src, "commit", "-qm", "one") + clean = runner.snapshot.git_identity(src) + assert clean["dirty"] == "clean" + assert clean["tree"] == git(src, "rev-parse", "HEAD^{tree}") + (src / "out" / "x.txt").write_text("two\n") + edited = runner.snapshot.git_identity(src) + assert edited["dirty"] == "dirty" + assert edited["tree"] != clean["tree"] From b5917e6ac894739fab5d91d9ec68e0836ed6153a Mon Sep 17 00:00:00 2001 From: ev-br's Claude Code Agent Date: Mon, 2 Nov 2026 18:27:40 +0000 Subject: [PATCH 3/9] 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 --- src/benchx/bench.py | 5 ++--- src/benchx/compare.py | 14 ++++++++++---- src/benchx/projection.py | 6 ++++++ tests/test_projection.py | 8 +++++++- 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/benchx/bench.py b/src/benchx/bench.py index 725e1e0..c54c4fd 100644 --- a/src/benchx/bench.py +++ b/src/benchx/bench.py @@ -14,6 +14,7 @@ from . import core, snapshot from .errors import SessionError +from .projection import workload_label from .store import warning_codes @@ -75,10 +76,8 @@ def row_of(document: dict) -> dict: measurement = document["measurement"] # An observation is a number, or an object carrying one with its metadata (pyperf's). values = [o["value"] if isinstance(o, dict) else o for o in measurement.get("observations", [])] - workload = document["coordinates"]["workload"] - parameters = ", ".join(f"{k}={v}" for k, v in workload.get("parameters", {}).items()) return { - "case": workload["name"] + (f" [{parameters}]" if parameters else ""), + "case": workload_label(document["coordinates"]["workload"]), "status": measurement["status"] + (f" ({measurement['reason']})" if measurement.get("reason") else ""), "n": len(values), "median": statistics.median(values) if values else None, diff --git a/src/benchx/compare.py b/src/benchx/compare.py index 1cab73c..612143e 100644 --- a/src/benchx/compare.py +++ b/src/benchx/compare.py @@ -91,12 +91,18 @@ def _verdict(effect, noise, k, direction): return "changed" +def _run_documents(documents, run_key) -> list[dict]: + """The measurands of one run, in a fixed order, so the same results give the same document.""" + found = sorted((d for d in documents if d["provenance"].get("run_key") == run_key and _measurand(d)), + key=lambda d: (d["producer"]["name"], d["ingest_key"])) + if not found: + raise CompareError(f"no results for run {run_key!r}") + return found + + def compare(documents, *, run_key, profile, baseline, label=None, k=3.0, min_rounds=3) -> dict: projection = profile_projection(profile, label) - documents = sorted((d for d in documents if d["provenance"].get("run_key") == run_key and _measurand(d)), - key=lambda d: (d["producer"]["name"], d["ingest_key"])) - if not documents: - raise CompareError(f"no results for run {run_key!r}") + documents = _run_documents(documents, run_key) naming = projection.axis[0] varying = _dotted(naming) doc = {"schema_version": catalog.COMPARISON_DOCUMENT, "mode": "run", "run_key": run_key, "profile": profile, diff --git a/src/benchx/projection.py b/src/benchx/projection.py index 28ad347..5995d1c 100644 --- a/src/benchx/projection.py +++ b/src/benchx/projection.py @@ -252,6 +252,12 @@ def axis_order(value) -> tuple: return (3, canonical(value).decode()) +def workload_label(workload: dict) -> str: + """A workload as people read it: its name, then its parameters, `name [k=v, ...]`.""" + parameters = ", ".join(f"{k}={v}" for k, v in (workload.get("parameters") or {}).items()) + return workload["name"] + (f" [{parameters}]" if parameters else "") + + def reported_coordinates(doc: dict) -> dict: """Everything reported about what was measured, minus the axis revision and the quantity (§4.3).""" return {key: doc[key] for key in REPORTED if key in doc} diff --git a/tests/test_projection.py b/tests/test_projection.py index 6cf0f2e..41d458d 100644 --- a/tests/test_projection.py +++ b/tests/test_projection.py @@ -8,7 +8,7 @@ from benchx import compare, core, identity from benchx.core import catalog -from benchx.projection import Projection, ProjectionError +from benchx.projection import Projection, ProjectionError, workload_label POLICIES = catalog.SCHEMAS / "identity-policy" / "0.1.0" / "examples" DEFAULT = json.loads((POLICIES / "default.json").read_text()) @@ -136,6 +136,12 @@ def test_a_profile_is_named_after_itself_and_its_policy(): b.series_fingerprint(b.apply(ARROW), "wall-time", identity.MEDIAN) +def test_a_workload_is_labelled_with_its_parameters(): + assert workload_label({"name": "bench"}) == "bench" + assert workload_label({"name": "bench", "parameters": {}}) == "bench" + assert workload_label({"name": "bench", "parameters": {"size": "1000", "n": "2"}}) == "bench [size=1000, n=2]" + + def test_the_quantity_is_never_a_coordinate(): """Quantity is in the measurement: no role can ignore it, so it always splits series.""" other = copy.deepcopy(ARROW) From 16b4d698abc6a70c71331d2595bc4524efc6a6ac Mon Sep 17 00:00:00 2001 From: ev-br's Claude Code Agent Date: Mon, 2 Nov 2026 17:34:25 +0000 Subject: [PATCH 4/9] ENH: comparator: the variants profile (UC-02) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/benchx/compare.py | 91 ++++++++++++---- tests/test_comparator.py | 225 +++++++++++++++++++++++++++++++++++++++ tests/test_projection.py | 22 ++++ 3 files changed, 319 insertions(+), 19 deletions(-) diff --git a/src/benchx/compare.py b/src/benchx/compare.py index 612143e..4b18f32 100644 --- a/src/benchx/compare.py +++ b/src/benchx/compare.py @@ -11,11 +11,12 @@ import json import math +import re import statistics from . import identity from .core import canonical, catalog -from .projection import Projection, get +from .projection import Projection, get, workload_label from .snapshot import DIRTY_WARNINGS from .store import store_project @@ -36,8 +37,22 @@ paths={"/revision/key": "match", "/provenance/subject_tree": "match"}, invariants=["one-run", "two-sides", "sides-differ", "equal-attempts", "alternating-slots"]), + # UC-02: one revision, one environment, two values of one workload parameter. Everything else + # must agree, so the axis (the parameter) is added per comparison: see `profile_projection`. + "variants": Projection(identity.POLICY_DATA, [], + profile="benchx/profile/variants/v1", + paths={"/revision/key": "match", "/provenance/subject_tree": "match"}, + invariants=["one-run", "two-sides", "equal-attempts", "alternating-slots", + "one-benchmark-tree"]), } +# Profiles for which a failed attempt on either side fails its unit (UC-02 §10): a variant that +# times out in the slow rounds must not be judged on the fast ones. The other profiles drop the +# attempt's pair and keep the verdict from the rest. +STRICT_STATUS = {"variants"} +FAILED_STATUSES = ("partial", "censored", "error") # `skipped` is not a failure WORKLOAD = "/coordinates/workload" +PARAMETERS = WORKLOAD + "/parameters/" +_PARAMETER_NAME = re.compile(r"[A-Za-z0-9_-]+") # The noise is the standard error of the per-round differences, so a verdict needs # at least two pairs, whatever `min_rounds` a caller asks for. MIN_PAIRS = 2 @@ -51,9 +66,10 @@ def _c(value) -> str: return canonical(value).decode() -def profile_projection(profile: str, label: str | None = None) -> Projection: +def profile_projection(profile: str, label: str | None = None, parameter: str | None = None) -> Projection: """The profile's projection; for environments, with the label that names - the sides at the front of its axis.""" + the sides at the front of its axis, and for variants, with the workload + parameter whose two values are the sides.""" if profile not in PROFILES: raise CompareError(f"profile {profile!r} is not in the implementation ({', '.join(PROFILES)})") projection = PROFILES[profile] @@ -61,6 +77,12 @@ def profile_projection(profile: str, label: str | None = None) -> Projection: if not label: raise CompareError("the environments profile needs the label that names the sides") projection = projection.with_axis_first(f"/provenance/labels/{label}") + if profile == "variants": + if not parameter: + raise CompareError("the variants profile needs the workload parameter whose values are the sides") + if not _PARAMETER_NAME.fullmatch(parameter): # keeps the document's `varying` a token + raise CompareError(f"workload parameter {parameter!r}: use letters, digits, '_' and '-' only") + projection = projection.with_axis_first(PARAMETERS + parameter) return projection @@ -100,8 +122,24 @@ def _run_documents(documents, run_key) -> list[dict]: return found -def compare(documents, *, run_key, profile, baseline, label=None, k=3.0, min_rounds=3) -> dict: - projection = profile_projection(profile, label) +def infer_parameter(documents, run_key) -> str: + """The workload parameter a variants comparison of this run varies, when the caller names none: + the only parameter the run's results carry. An `-e` variable counts, since the runner records + it as a workload parameter. Read mode only; measure mode asks before running (session).""" + names = sorted({name for d in _run_documents(documents, run_key) + for name in (d["coordinates"]["workload"].get("parameters") or {})}) + if not names: + raise CompareError("nothing to vary: no workload parameter in the results " + "(declare one in `params`/`param_names`)") + if len(names) > 1: + raise CompareError(f"several workload parameters ({', '.join(names)}): give --vary NAME") + return names[0] + + +def compare(documents, *, run_key, profile, baseline, label=None, parameter=None, k=3.0, min_rounds=3) -> dict: + """The run-mode comparison document. `label` names the sides of `environments`, and `parameter` + those of `variants`.""" + projection = profile_projection(profile, label, parameter) documents = _run_documents(documents, run_key) naming = projection.axis[0] varying = _dotted(naming) @@ -121,6 +159,9 @@ def compare(documents, *, run_key, profile, baseline, label=None, k=3.0, min_rou detail = (f"a result has no {varying}: its tree's dirty state is unknown, so the side cannot be told " "apart from the other or checked for a change during the run. Make every file in the " "tree readable, or git-ignore it") + elif profile == "variants": + detail = (f"a result has no workload parameter {parameter}: declare it in the benchmark's " + "`params`/`param_names`") else: detail = f"a result has no {varying}: give every bx bench call --label NAME=VALUE, and name that label here" failed.append({"invariant": "sides", "detail": detail}) @@ -165,12 +206,15 @@ def compare(documents, *, run_key, profile, baseline, label=None, k=3.0, min_rou if "one-benchmark-tree" in check: failed.extend(_benchmark_file(documents, names, sides)) if "alternating-slots" in check: + # Variants are usually measured by one command, so say how to get an alternating run. + how = ("; make one bx bench call per value, each with --run-key, --round and --slot, " + "or run bx compare TARGET --profile variants" if profile == "variants" else "") slots = {} for d, s in zip(documents, sides): slot = d.get("procedure", {}).get("slot") if slot is None: failed.append({"invariant": "alternating-slots", - "detail": "a result has no slot: give every bx bench call --slot"}) + "detail": "a result has no slot: give every bx bench call --slot" + how}) break slots.setdefault(slot, set()).add(s) else: @@ -178,7 +222,7 @@ def compare(documents, *, run_key, profile, baseline, label=None, k=3.0, min_rou if any(len(o) != 1 for o in order) or any(a == b for a, b in zip(order, order[1:])): failed.append({"invariant": "alternating-slots", "detail": "sides do not alternate by procedure.slot: run the sides in the same " - "order every round (a, b, a, b, ...)"}) + "order every round (a, b, a, b, ...)" + how}) # Units: one workload variant and quantity each. Everything else the # projection matches must agree across both sides (schema §5.5). @@ -192,14 +236,15 @@ def compare(documents, *, run_key, profile, baseline, label=None, k=3.0, min_rou for (_, quantity), unit in units.items(): differing = _differing(unit["identities"]) if differing: - failed.append({"invariant": "sides-agree", "workload": _workload(unit["docs"]), "quantity": quantity, - "detail": f"{', '.join(differing)} differ"}) + failed.append({"invariant": "sides-agree", "workload": _workload(unit["docs"], projection.axis), + "quantity": quantity, "detail": f"{', '.join(differing)} differ"}) if failed: return doc - for (_, quantity), unit in sorted(units.items(), key=lambda item: (_c(_workload(item[1]["docs"])), item[0][1])): + for (_, quantity), unit in sorted( + units.items(), key=lambda item: (_c(_workload(item[1]["docs"], projection.axis)), item[0][1])): by_side = unit["docs"] - where = {"workload": _workload(by_side), "quantity": quantity} + where = {"workload": _workload(by_side, projection.axis), "quantity": quantity} absent = [role for role, s in (("baseline", base), ("contender", contender)) if not by_side[s]] if absent: doc["missing"].extend({**where, "side": role} for role in absent) @@ -209,18 +254,19 @@ def compare(documents, *, run_key, profile, baseline, label=None, k=3.0, min_rou "detail": f"{len(by_side[base])} against {len(by_side[contender])}: run the same " "number of rounds on both sides"}) continue - rounds = {} + rounds, side_failed = {}, False for s in (base, contender): for d in by_side[s]: if d["measurement"]["status"] != "success": status, reason = d["measurement"]["status"], d["measurement"].get("reason") result = {"producer": d["producer"]["name"], "ingest_key": d["ingest_key"]} doc["exclusions"].append({"result": result, "reason": f"{status}: {reason}" if reason else status}) + side_failed = side_failed or (profile in STRICT_STATUS and status in FAILED_STATUSES) continue rounds.setdefault(d.get("procedure", {}).get("round"), {})[s] = d pairs = [(r[base], r[contender]) for _, r in sorted(rounds.items(), key=lambda x: (x[0] is None, x[0])) if base in r and contender in r] - unit_doc = _unit(where, pairs, by_side, base, contender, k, min_rounds) + unit_doc = _unit(where, pairs, by_side, base, contender, k, min_rounds, side_failed) unit_doc["unmatched_differences"] = _differences(projection, by_side[base], by_side[contender]) if caveats := _caveats(by_side, base, contender): unit_doc["caveats"] = caveats @@ -229,7 +275,7 @@ def compare(documents, *, run_key, profile, baseline, label=None, k=3.0, min_rou def _benchmark_file(documents, names, sides) -> list[dict]: - """The `one-benchmark-tree` invariant (schema §5.5, UC-03 §10). A benchmark recorded apart from + """The `one-benchmark-tree` invariant (schema §5.5, UC-03 §10, UC-02 §5). A benchmark recorded apart from the subject has its own `benchmark_tree`, which differs from `subject_tree`; it must be one file on both sides and in every round. A benchmark in the subject's checkout has `benchmark_tree` equal to `subject_tree` and differs between the sides as the subject does, so it is not @@ -264,10 +310,13 @@ def _benchmark_file(documents, names, sides) -> list[dict]: return failed -def _workload(by_side) -> dict: - """The unit's workload variant, as the comparison document names it.""" +def _workload(by_side, axis=()) -> dict: + """The unit's workload variant, as the comparison document names it: what both sides share, + so the parameters on the profile's axis (the ones that tell the sides apart) are left out.""" workload = next(d for docs in by_side.values() for d in docs)["coordinates"]["workload"] - return {"name": workload["name"], **({"parameters": workload["parameters"]} if workload.get("parameters") else {})} + varied = {path.removeprefix(PARAMETERS) for path in axis if path.startswith(PARAMETERS)} + parameters = {k: v for k, v in (workload.get("parameters") or {}).items() if k not in varied} + return {"name": workload["name"], **({"parameters": parameters} if parameters else {})} _ABSENT = object() @@ -315,12 +364,16 @@ def _caveats(by_side, base, contender) -> list[str]: return sorted(found) -def _unit(where, pairs, by_side, base, contender, k, min_rounds): +def _unit(where, pairs, by_side, base, contender, k, min_rounds, side_failed=False): first = {s: by_side[s][0] for s in (base, contender)} quantity = first[base]["measurement"]["quantity"] unit = {**where, "series": {name: identity.series_fingerprint(first[s], store_project(first[s]), identity.MEDIAN) for name, s in (("baseline", base), ("contender", contender))}, "rounds": len(pairs), "noise_source": "paired-rounds", "verdict": "indeterminate"} + if side_failed: + # UC-02 §10: if either side fails to run, the benchmark has failed; no effect is reported. + unit["reason"] = "side-failed" + return unit estimates = [(_estimate(b), _estimate(c)) for b, c in pairs] estimates = [(b, c) for b, c in estimates if b is not None and c is not None] if estimates: @@ -394,7 +447,7 @@ def render(doc: dict) -> str: effect = f"{u['effect']:+8.2%}" if "effect" in u else " ?" noise = f"{u['noise']:7.2%}" if "noise" in u else " ?" reason = f" ({u['reason']})" if "reason" in u else "" - lines.append(f" {u['workload']['name']:<32} {u['quantity']:<10} effect {effect} noise {noise}" + lines.append(f" {workload_label(u['workload']):<32} {u['quantity']:<10} effect {effect} noise {noise}" f" rounds={u['rounds']}{reason}") for key, title in (("missing", "missing"), ("failed_invariants", "failed invariants"), ("exclusions", "excluded")): diff --git a/tests/test_comparator.py b/tests/test_comparator.py index 9bafbba..9ec6b78 100644 --- a/tests/test_comparator.py +++ b/tests/test_comparator.py @@ -357,3 +357,228 @@ def test_a_result_without_a_benchmark_tree_is_outside_the_rule(): docs = run() assert all("benchmark_tree" not in d["provenance"] for d in docs) assert invariants(evaluate(docs)) == [] + + +# --- variants (UC-02, schema §5.5): the sides are two values of one workload parameter --------- + +VALUES = ("'baseline'", "'variant'") + + +def variants_run(rounds=3, sizes=(None,), ratio=1.1, base_values=(1.0, 2.0, 3.0), name="role", values=VALUES): + """Alternating attempts of the two values of the parameter `name`, one slot each; the second + value is `ratio` times the first in each round. With `sizes`, a slot holds one result per + size, and `size` is a second workload parameter. Every result is one revision and one tree.""" + docs, slot = [], 0 + for r in range(rounds): + for value, scale in zip(values, (1, ratio)): + for size in sizes: + parameters = {name: value, **({"size": size} if size is not None else {})} + docs.append(result(HEAD, r, slot, base_values[r] * scale, ingest_key=f"run:{slot}:{size}", + coordinates__workload={"name": "bench_truncate", "parameters": parameters})) + slot += 1 + return docs + + +def vevaluate(docs, parameter="role", baseline="'baseline'", **kwargs): + doc = compare.compare(docs, run_key="run", profile="variants", baseline=baseline, parameter=parameter, **kwargs) + assert core.check(doc, "comparison-document") == [] + return doc + + +def test_variants_a_clean_run_has_one_unit_per_benchmark(): + doc = vevaluate(variants_run()) + assert invariants(doc) == [] and doc["profile"] == "variants" + assert doc["varying"] == "coordinates.workload.parameters.role" + assert (doc["baseline"], doc["contender"]) == VALUES + [unit] = doc["units"] + assert unit["workload"] == {"name": "bench_truncate"} # the varied parameter is not part of the unit + assert unit["verdict"] == "regressed" and unit["effect"] == pytest.approx(0.1) + assert doc["local_only"] is False + + +def test_variants_the_baseline_can_be_either_value(): + doc = vevaluate(variants_run(), baseline="'variant'") + assert invariants(doc) == [] and (doc["baseline"], doc["contender"]) == VALUES[::-1] + assert doc["units"][0]["verdict"] == "improved" + + +def test_variants_another_parameter_makes_one_unit_per_value_of_it(): + doc = vevaluate(variants_run(sizes=("1000", "100000"))) + assert invariants(doc) == [] + assert [u["workload"] for u in doc["units"]] == [ + {"name": "bench_truncate", "parameters": {"size": "1000"}}, + {"name": "bench_truncate", "parameters": {"size": "100000"}}] # no `role` in either + shown = compare.render(doc) + assert "bench_truncate [size=1000]" in shown and "bench_truncate [size=100000]" in shown + assert "role" not in shown.split("\n", 1)[1] + + +def test_variants_no_parameter_name_is_special(): + doc = vevaluate(variants_run(name="impl"), parameter="impl") + assert invariants(doc) == [] and doc["varying"] == "coordinates.workload.parameters.impl" + + +def test_variants_a_third_value_is_a_third_side(): + docs = variants_run() + docs.append(result(HEAD, 3, 6, 1.0, ingest_key="run:6", coordinates__workload={ + "name": "bench_truncate", "parameters": {"role": "'other'"}})) + doc = vevaluate(docs) + assert invariants(doc) == ["two-sides"] and doc["units"] == [] + + +@pytest.mark.parametrize("slotless", [False, True]) +def test_variants_a_run_that_does_not_alternate_names_the_ways_to_get_one(slotless): + """One `bx bench` call runs both values in one order, with no round and no slot.""" + docs = variants_run() + for d in docs: + if slotless: + del d["procedure"] + else: + d["procedure"]["slot"] = d["procedure"]["round"] # both values in one slot + doc = vevaluate(docs) + assert invariants(doc) == ["alternating-slots"] + detail = doc["failed_invariants"][0]["detail"] + assert "--round and --slot" in detail and "bx compare TARGET --profile variants" in detail + + +def test_the_other_profiles_do_not_name_variants_in_the_alternation_detail(): + docs = run() + for d in docs: + del d["procedure"] + assert "variants" not in evaluate(docs)["failed_invariants"][0]["detail"] + + +def test_variants_unequal_attempts(): + docs = variants_run() + docs.append(result(HEAD, 3, 6, 4.0, ingest_key="run:6", coordinates__workload={ + "name": "bench_truncate", "parameters": {"role": VALUES[0]}})) + doc = vevaluate(docs) + assert invariants(doc) == ["equal-attempts"] and doc["units"] == [] + + +def fail(doc, status): + doc["measurement"] = {"quantity": doc["measurement"]["quantity"], "status": status, + **({"reason": "timeout"} if status != "partial" else {"reason": "timeout", "observations": [1.0]})} + return doc + + +@pytest.mark.parametrize("status", ["error", "censored", "partial"]) +@pytest.mark.parametrize("which", [0, 1]) +def test_variants_a_failed_attempt_on_either_side_fails_the_unit(status, which): + """UC-02 §10: a value that times out in the slow rounds is not judged on the fast ones.""" + docs = variants_run(rounds=4, base_values=(1.0, 2.0, 3.0, 4.0)) + failing = docs[2 + which] # round 1: the baseline's result, then the variant's + fail(failing, status) + doc = vevaluate(docs) + [unit] = doc["units"] + assert invariants(doc) == [] and unit["verdict"] == "indeterminate" and unit["reason"] == "side-failed" + assert "effect" not in unit and "noise" not in unit and unit["rounds"] == 3 + [excluded] = doc["exclusions"] + assert excluded["result"]["ingest_key"] == failing["ingest_key"] and excluded["reason"].startswith(status) + assert "(side-failed)" in compare.render(doc) + + +def test_variants_only_the_unit_with_the_failed_attempt_is_indeterminate(): + docs = variants_run(rounds=4, base_values=(1.0, 2.0, 3.0, 4.0), sizes=("1000", "100000")) + fail(docs[4], "error") # round 1, baseline, size 1000 + verdicts = {u["workload"]["parameters"]["size"]: u["verdict"] for u in vevaluate(docs)["units"]} + assert verdicts == {"1000": "indeterminate", "100000": "regressed"} + + +def test_variants_a_skipped_attempt_is_an_ordinary_exclusion(): + docs = variants_run(rounds=4, base_values=(1.0, 2.0, 3.0, 4.0)) + docs[2]["measurement"] = {"quantity": docs[2]["measurement"]["quantity"], "status": "skipped", "reason": "n/a"} + doc = vevaluate(docs) + [unit] = doc["units"] + assert unit["verdict"] == "regressed" and unit["rounds"] == 3 and "reason" not in unit + assert len(doc["exclusions"]) == 1 + + +def test_a_failed_attempt_under_revisions_still_gives_a_verdict_from_the_remaining_pairs(): + docs = run(rounds=4, base_values=(1.0, 2.0, 3.0, 4.0)) + fail(docs[2], "error") + unit = evaluate(docs)["units"][0] + assert unit["verdict"] == "regressed" and unit["rounds"] == 3 and "reason" not in unit + + +def test_variants_the_sides_must_share_the_subject_tree(): + docs = variants_run() + for d in docs: + if d["coordinates"]["workload"]["parameters"]["role"] == VALUES[1]: + d["provenance"]["subject_tree"] = "c" * 40 + doc = vevaluate(docs) + assert invariants(doc) == ["sides-agree"] + assert doc["failed_invariants"][0]["detail"] == "/provenance/subject_tree differ" + + +def test_variants_a_suite_that_changes_between_rounds_fails(): + docs = benchmark_trees(variants_run(), lambda d: "c" * 40) + assert invariants(vevaluate(docs)) == [] + docs[-1]["provenance"]["benchmark_tree"] = "d" * 40 # the suite is edited before the last result + assert invariants(vevaluate(docs)) == ["one-benchmark-tree"] + + +def test_variants_a_dirty_tree_is_compared_and_named(): + doc = vevaluate(dirty(variants_run(), HEAD)) + assert invariants(doc) == [] and doc["local_only"] is True + assert doc["units"][0]["caveats"] == ["baseline: dirty-subject-tree", "contender: dirty-subject-tree"] + assert "[local-only]" in compare.render(doc) + + +def test_variants_a_result_without_the_parameter_cannot_be_placed(): + docs = variants_run() + docs[0]["coordinates"]["workload"]["parameters"] = {"other": "1"} + doc = vevaluate(docs) + assert invariants(doc) == ["sides"] and doc["units"] == [] + assert "no workload parameter role" in doc["failed_invariants"][0]["detail"] + assert "params" in doc["failed_invariants"][0]["detail"] + + +def test_variants_the_baseline_is_matched_exactly_as_recorded(): + with pytest.raises(compare.CompareError) as error: + vevaluate(variants_run(), baseline="baseline") # the recorded value has its quotes + assert "'baseline'" in str(error.value) and "'variant'" in str(error.value) + + +@pytest.mark.parametrize("parameter", [None, "", "a b", "a.b", "a/b", "role]"]) +def test_variants_needs_a_parameter_that_is_a_token(parameter): + with pytest.raises(compare.CompareError): + compare.compare(variants_run(), run_key="run", profile="variants", baseline=VALUES[0], parameter=parameter) + + +# --- which parameter to vary, in read mode ------------------------------------------------------ + +def test_infer_parameter_takes_the_only_one(): + assert compare.infer_parameter(variants_run(), "run") == "role" + assert compare.infer_parameter(variants_run(name="impl"), "run") == "impl" # no name is special + + +def test_infer_parameter_refuses_several_and_names_them(): + with pytest.raises(compare.CompareError, match=r"several workload parameters \(role, size\): give --vary NAME"): + compare.infer_parameter(variants_run(sizes=("1000",)), "run") + + +def test_infer_parameter_counts_an_environment_variable(): + """The runner records an `-e` variable as a workload parameter of every result.""" + docs = variants_run() + for d in docs: + d["coordinates"]["workload"]["parameters"]["OMP_NUM_THREADS"] = "1" + with pytest.raises(compare.CompareError, match="OMP_NUM_THREADS, role"): + compare.infer_parameter(docs, "run") + + +def test_infer_parameter_refuses_a_run_with_no_parameter(): + docs = variants_run() + for d in docs: + del d["coordinates"]["workload"]["parameters"] + with pytest.raises(compare.CompareError, match="nothing to vary"): + compare.infer_parameter(docs, "run") + + +def test_infer_parameter_reads_only_the_run_and_only_measurands(): + docs = variants_run() + other = result(HEAD, 0, 0, 1.0, ingest_key="other", coordinates__workload={ + "name": "x", "parameters": {"size": "1"}}, provenance__run_key="another") + assert compare.infer_parameter([*docs, other], "run") == "role" + with pytest.raises(compare.CompareError, match="no results for run"): + compare.infer_parameter(docs, "missing") diff --git a/tests/test_projection.py b/tests/test_projection.py index 41d458d..59f0dfd 100644 --- a/tests/test_projection.py +++ b/tests/test_projection.py @@ -136,6 +136,28 @@ def test_a_profile_is_named_after_itself_and_its_policy(): b.series_fingerprint(b.apply(ARROW), "wall-time", identity.MEDIAN) +def test_the_variants_profile_follows_one_workload_parameter(): + """Its axis is the parameter, added per comparison; the rest of the result, tree included, must match.""" + assert compare.PROFILES["variants"].axis == [] + variants = compare.profile_projection("variants", parameter="role") + assert variants.identity_schema == "benchx/profile/variants/v1+benchx/benchx-identity/v1" + assert variants.axis == ["/coordinates/workload/parameters/role"] + doc = copy.deepcopy(ARROW) + doc["coordinates"]["workload"]["parameters"] = {"role": "'baseline'", "size": "1000"} + projected = variants.apply(doc) + assert projected.axis == ("'baseline'",) + assert "/coordinates/workload/parameters/role" not in projected.identity + assert projected.identity["/coordinates/workload/parameters/size"] == "1000" + assert {"/revision/key", "/provenance/subject_tree"} <= set(projected.identity) + assert compare.PROFILES["variants"].axis == [] # the profile itself is not changed + + +@pytest.mark.parametrize("parameter", [None, "", "a b", "a.b", "a/b", "a~b", "role]"]) +def test_a_variants_parameter_must_be_a_token_that_needs_no_escaping(parameter): + with pytest.raises(compare.CompareError): + compare.profile_projection("variants", parameter=parameter) + + def test_a_workload_is_labelled_with_its_parameters(): assert workload_label({"name": "bench"}) == "bench" assert workload_label({"name": "bench", "parameters": {}}) == "bench" From 729572567aa6cb987b107910abd60797fc0dcee4 Mon Sep 17 00:00:00 2001 From: ev-br's Claude Code Agent Date: Mon, 2 Nov 2026 17:42:27 +0000 Subject: [PATCH 5/9] 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 --- src/benchx/cli.py | 29 ++- src/benchx/schemas/config/0.1.0/schema.json | 2 +- src/benchx/session.py | 6 +- tests/test_config_schema.py | 12 +- tests/test_session.py | 5 + tests/test_uc02.py | 212 ++++++++++++++++++++ 6 files changed, 259 insertions(+), 7 deletions(-) create mode 100644 tests/test_uc02.py diff --git a/src/benchx/cli.py b/src/benchx/cli.py index fd3d066..25c81c6 100644 --- a/src/benchx/cli.py +++ b/src/benchx/cli.py @@ -379,6 +379,7 @@ def cmd_compare(args): try: mode = _mode(args) policy = _policy(cfg, args) + _profile_flags(args, policy["profile"]) except (UsageError, config_mod.ConfigError) as e: print(f"bx compare: error: {e}", file=sys.stderr) return EXIT_REFUSED @@ -396,6 +397,16 @@ def _policy(cfg, args) -> dict: } +def _profile_flags(args, profile): + """The flags that name the sides of one profile only.""" + if args.vary and profile != "variants": + raise UsageError(f"--vary names the parameter whose values are the sides of a variants comparison, " + f"not of {profile}") + if args.label and profile == "variants": + raise UsageError("--label names the sides of an environments comparison; the sides of a variants " + "comparison are two values of a parameter (--vary)") + + def _read(cfg, args, policy): if args.results: with tempfile.TemporaryDirectory() as tmp: # the ad hoc path: a throwaway store @@ -434,8 +445,14 @@ def _measure(cfg, args, policy): def _compare(store, args, policy): try: - doc = compare_mod.compare(store.documents(args.run), run_key=args.run, profile=policy["profile"], - baseline=args.baseline, label=args.label, k=policy["k"], + documents = store.documents(args.run) + parameter = None + if policy["profile"] == "variants": + parameter = args.vary or compare_mod.infer_parameter(documents, args.run) + if not args.vary: # the choice is recorded in the document's `varying`, and said here too + print(f"varying: {parameter} (the only parameter)", file=sys.stderr) + doc = compare_mod.compare(documents, run_key=args.run, profile=policy["profile"], + baseline=args.baseline, label=args.label, parameter=parameter, k=policy["k"], min_rounds=policy["min_rounds"]) except compare_mod.CompareError as e: print(f"cannot compare: {e}", file=sys.stderr) @@ -587,7 +604,10 @@ def main(argv=None): "BASELINE CONTENDER --suite NAME --rounds R, each target BUILD_DIR[:SOURCE_DIR] " "or @CONFIG[=SOURCE_REF] (the project's target provider).") p.add_argument("targets", nargs="*", metavar="TARGET", help="measure mode: BASELINE CONTENDER") - p.add_argument("--profile", choices=["revisions", "environments"], help="default: compare.profile") + p.add_argument("--profile", choices=list(compare_mod.PROFILES), help="default: compare.profile") + p.add_argument("--vary", metavar="NAME", + help="variants: the workload parameter whose two values are the sides " + "(default: the only parameter the results carry)") p.add_argument("--k", type=float, help="noise multiplier (default: compare.k, else 3)") p.add_argument("--min-rounds", type=int, help="fewest paired rounds for a verdict (default: compare.min_rounds, else 3)") p.add_argument("--json", action="store_true") @@ -595,7 +615,8 @@ def main(argv=None): "measure mode: NAME=BASE,CONTENDER") read = p.add_argument_group("read mode") read.add_argument("--run", help="run key") - read.add_argument("--baseline", help="revisions: a revision or tree id prefix; environments: the label value") + read.add_argument("--baseline", help="revisions: a revision or tree id prefix; environments: the label value; " + "variants: the parameter's recorded value, quotes included ('baseline')") read.add_argument("--results", nargs="+", help="compare these result directories through a throwaway store, not the local one") measure = p.add_argument_group("measure mode") diff --git a/src/benchx/schemas/config/0.1.0/schema.json b/src/benchx/schemas/config/0.1.0/schema.json index c39ba9f..f9f5007 100644 --- a/src/benchx/schemas/config/0.1.0/schema.json +++ b/src/benchx/schemas/config/0.1.0/schema.json @@ -93,7 +93,7 @@ "type": "object", "additionalProperties": false, "properties": { - "profile": { "enum": ["revisions", "environments"] }, + "profile": { "enum": ["revisions", "environments", "variants"] }, "rounds": { "type": "integer", "minimum": 1 }, "k": { "type": "number", "exclusiveMinimum": 0 }, "min_rounds": { "type": "integer", "minimum": 1 } diff --git a/src/benchx/session.py b/src/benchx/session.py index 24a21aa..d764dd2 100644 --- a/src/benchx/session.py +++ b/src/benchx/session.py @@ -22,7 +22,7 @@ from .errors import SessionError from .identity import normalize_remote -PROFILES = ("revisions", "environments") +PROFILES = tuple(compare_mod.PROFILES) def parse_target(text: str) -> tuple[str, str | None]: @@ -236,6 +236,10 @@ def compare_targets(baseline: str, contender: str, *, profile: str, suite: str, """ if profile not in PROFILES: raise SessionError(f"profile {profile!r} is not in the implementation ({', '.join(PROFILES)})") + if profile == "variants": + # Its sides are two values of one parameter of one target, not two targets. + raise SessionError("the variants profile is not available in measure mode yet: measure each value " + "with bx bench --run-key/--round/--slot, then bx compare --run KEY --profile variants") if rounds < 1: raise SessionError("--rounds must be at least 1") if profile == "environments" and label is None: diff --git a/tests/test_config_schema.py b/tests/test_config_schema.py index a1a1c64..4753719 100644 --- a/tests/test_config_schema.py +++ b/tests/test_config_schema.py @@ -5,7 +5,7 @@ import pytest -from benchx import adapters, core, snapshot +from benchx import adapters, compare, core, session, snapshot from benchx.core import catalog from conftest import git @@ -43,6 +43,16 @@ def test_schema_refuses(config, message): assert errors and message in " ".join(str(e) for e in errors) +def test_every_comparison_profile_is_one_the_config_can_name(): + """`compare.profile` takes exactly the profiles the comparator has, variants included.""" + schema = json.loads((catalog.SCHEMAS / "config" / "0.1.0" / "schema.json").read_text()) + enum = schema["properties"]["compare"]["properties"]["profile"]["enum"] + assert sorted(enum) == sorted(compare.PROFILES) == sorted(session.PROFILES) + assert "variants" in enum + for profile in compare.PROFILES: + assert core.check({"schema_version": "benchx/config/0.1.0", "compare": {"profile": profile}}, "config") == [] + + # --- paths benchx writes inside a measured tree ----------------------------------- @pytest.fixture(autouse=True) diff --git a/tests/test_session.py b/tests/test_session.py index 784c10b..4558b88 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -146,6 +146,11 @@ def test_measure_mode_equals_read_mode(repo, tmp, builds, capsys): (["A", "B", "--profile", "revisions", "--suite", "s", "--rounds", "3", "--baseline", "x"], "read mode"), (["--run", "k", "--profile", "revisions"], "needs --baseline"), (["--run", "k", "--profile", "revisions", "--baseline", "x", "--rounds", "3"], "measure mode"), + (["--run", "k", "--profile", "revisions", "--baseline", "x", "--vary", "role"], "--vary names the parameter"), + (["--run", "k", "--profile", "environments", "--label", "e", "--baseline", "x", "--vary", "role"], + "--vary names the parameter"), + (["--run", "k", "--profile", "variants", "--baseline", "x", "--label", "e"], "--label names the sides"), + (["--run", "k", "--profile", "variants"], "needs --baseline"), ]) def test_mode_selection(argv, message, capsys): assert cli.main(["compare", *argv]) == cli.EXIT_REFUSED diff --git a/tests/test_uc02.py b/tests/test_uc02.py new file mode 100644 index 0000000..d3c0871 --- /dev/null +++ b/tests/test_uc02.py @@ -0,0 +1,212 @@ +"""UC-02: two functions of one benchmark compared, with the rounds interleaved. + +The two functions are the two values of one workload parameter (#94's `params` and +`param_names`). Each `bx bench` call runs one value as one side of a session, placed by +--run-key, --round and --slot; the comparison is `bx compare --run ... --profile variants`. +A value is slower on purpose, so that its effect has a sign whatever the noise. +""" + +import json +import textwrap + +import pytest + +pytest.importorskip("pyperf") + +from benchx import cli, core +from conftest import git + +RUN = "uc02" +BASELINE, VARIANT = "'baseline'", "'variant'" +PYPERF = ["--processes", "2", "--values", "2", "--loops", "20", "--no-project"] +ASV = ["--processes", "2", "--values", "2", "--warmups", "0", "--min-time", "0.001", "--no-project"] + +# §2 of the plan: one function, and the two values of `role` select what it runs. +ONE_PARAMETER = textwrap.dedent(''' + def bench_truncate(role): + sum(range(10 if role == "baseline" else 2000)) + + bench_truncate.params = ["baseline", "variant"] + bench_truncate.param_names = ["role"] +''') +TWO_PARAMETERS = textwrap.dedent(''' + def bench_truncate(role, size): + sum(range(size if role == "baseline" else 20 * size)) + + bench_truncate.params = [["baseline", "variant"], [10, 100]] + bench_truncate.param_names = ["role", "size"] +''') + + +def make_tree(root, files): + """A git repository with `files` committed and nothing else: no .gitignore.""" + root.mkdir() + git(root, "init", "-q") + for name, text in files.items(): + (root / name).parent.mkdir(parents=True, exist_ok=True) + (root / name).write_text(text) + git(root, "add", "-A") + git(root, "commit", "-qm", "suite") + return root + + +@pytest.fixture +def tree(tmp, monkeypatch): + path = make_tree(tmp / "tree", {"mod.py": ONE_PARAMETER}) + monkeypatch.chdir(path) + return path + + +def loop(target, selectors, fast, rounds=3, out=False): + """The caller's loop (§3): one `bx bench` call per value per round, in the same order every round.""" + slot = 0 + for round_ in range(rounds): + for selector in selectors: + argv = ["bench", *target, *selector, "--run-key", RUN, "--round", str(round_), "--slot", str(slot), *fast] + if out: + argv += ["--out", f"out/call-{slot}"] + assert cli.main(argv) == cli.EXIT_OK + slot += 1 + + +def compare(capsys, *extra, baseline=BASELINE): + """`bx compare --run`: the exit code, the comparison document (None if not printed) and stderr.""" + capsys.readouterr() + code = cli.main(["compare", "--run", RUN, "--profile", "variants", "--baseline", baseline, "--json", *extra]) + captured = capsys.readouterr() + return code, (json.loads(captured.out) if captured.out.strip() else None), captured.err + + +def by_case(selector_values): + """One selector per value, naming one case each: `bx bench mod.py "bench_truncate('baseline')"`.""" + return [[f"bench_truncate({value})"] for value in selector_values] + + +def test_the_loop_compares_the_two_values(tree, capsys): + loop(["mod.py"], by_case([BASELINE, VARIANT]), PYPERF) + code, doc, err = compare(capsys) + assert code == cli.EXIT_OK and doc["failed_invariants"] == [] + assert core.check(doc, "comparison-document") == [] + assert "varying: role (the only parameter)" in err # inferred, and said so + assert doc["varying"] == "coordinates.workload.parameters.role" + assert (doc["baseline"], doc["contender"]) == (BASELINE, VARIANT) + [unit] = doc["units"] + assert unit["workload"] == {"name": "bench_truncate"} and unit["rounds"] == 3 and unit["effect"] > 0 + assert doc["local_only"] is False # a clean tree: the store and the results are not part of it + + +def test_the_baseline_can_be_the_other_value(tree, capsys): + loop(["mod.py"], by_case([BASELINE, VARIANT]), PYPERF) + code, doc, _ = compare(capsys, baseline=VARIANT) + assert code == cli.EXIT_OK and doc["failed_invariants"] == [] + assert (doc["baseline"], doc["contender"]) == (VARIANT, BASELINE) + assert doc["units"][0]["effect"] < 0 + + +def test_an_unquoted_baseline_is_refused_with_the_recorded_values(tree, capsys): + loop(["mod.py"], by_case([BASELINE, VARIANT]), PYPERF, rounds=2) + code, doc, err = compare(capsys, baseline="baseline") + assert code == cli.EXIT_REFUSED and doc is None + assert "'baseline'" in err and "'variant'" in err + + +def test_a_second_parameter_makes_one_unit_per_value_of_it(tmp, monkeypatch, capsys): + monkeypatch.chdir(make_tree(tmp / "tree", {"mod.py": TWO_PARAMETERS})) + loop(["mod.py"], [["--filter", BASELINE], ["--filter", VARIANT]], PYPERF) + code, doc, err = compare(capsys) # two parameters: it cannot be guessed + assert code == cli.EXIT_REFUSED and doc is None + assert "several workload parameters (role, size): give --vary NAME" in err + code, doc, err = compare(capsys, "--vary", "role") + assert code == cli.EXIT_OK and doc["failed_invariants"] == [] and "varying:" not in err + assert [u["workload"] for u in doc["units"]] == [ + {"name": "bench_truncate", "parameters": {"size": "10"}}, + {"name": "bench_truncate", "parameters": {"size": "100"}}] + assert all(u["effect"] > 0 for u in doc["units"]) + cli.main(["compare", "--run", RUN, "--profile", "variants", "--baseline", BASELINE, "--vary", "role"]) + shown = capsys.readouterr().out + assert "bench_truncate [size=10]" in shown and "bench_truncate [size=100]" in shown + + +def test_a_vary_that_names_no_parameter_of_the_results_fails_the_comparison(tree, capsys): + loop(["mod.py"], by_case([BASELINE, VARIANT]), PYPERF, rounds=2) + code, doc, _ = compare(capsys, "--vary", "size") + assert code == cli.EXIT_OK and doc["units"] == [] + assert [f["invariant"] for f in doc["failed_invariants"]] == ["sides"] + assert "no workload parameter size" in doc["failed_invariants"][0]["detail"] + + +def test_one_bench_call_is_not_an_alternating_run(tree, capsys): + """Both values in one call: one order, no round and no slot. The refusal says how to alternate them.""" + assert cli.main(["bench", "mod.py", "--run-key", RUN, *PYPERF]) == cli.EXIT_OK + code, doc, _ = compare(capsys) + assert code == cli.EXIT_OK and doc["units"] == [] + assert [f["invariant"] for f in doc["failed_invariants"]] == ["alternating-slots"] + detail = doc["failed_invariants"][0]["detail"] + assert "--round and --slot" in detail and "bx compare TARGET --profile variants" in detail + + +def test_a_directory_per_call_inside_the_tree_leaves_the_tree_alone(tree, capsys): + """`--out out/NAME` per call, in a repository with no .gitignore (§5.0): the tree id stays the same.""" + assert not (tree / ".gitignore").exists() + loop(["mod.py"], by_case([BASELINE, VARIANT]), PYPERF, out=True) + assert (tree / "out" / "call-0").is_dir() and (tree / "out" / "call-5").is_dir() + code, doc, _ = compare(capsys) + assert code == cli.EXIT_OK and doc["failed_invariants"] == [] + assert doc["local_only"] is False and len(doc["units"]) == 1 + + +def test_an_uncommitted_edit_is_compared_and_named(tree, capsys): + (tree / "mod.py").write_text(ONE_PARAMETER + "# edited\n") + loop(["mod.py"], by_case([BASELINE, VARIANT]), PYPERF, rounds=2) + code, doc, _ = compare(capsys, "--min-rounds", "2") + assert code == cli.EXIT_OK and doc["failed_invariants"] == [] + # The suite is in the subject's tree, so the benchmark is dirty as the subject is (#89). + assert doc["local_only"] is True and set(doc["units"][0]["caveats"]) == { + f"{side}: dirty-{what}-tree" for side in ("baseline", "contender") for what in ("subject", "benchmark")} + + +# --- the same with an asv benchmark directory --------------------------------------------------- + +ASV_CONF = '{"version": 1, "project": "uc02", "benchmark_dir": "benchmarks"}\n' +ASV_ONE = textwrap.dedent(''' + def time_truncate(role): + sum(range(10 if role == "baseline" else 2000)) + + time_truncate.params = ["baseline", "variant"] + time_truncate.param_names = ["role"] +''') +ASV_TWO = textwrap.dedent(''' + def time_truncate(role, size): + sum(range(size if role == "baseline" else 20 * size)) + + time_truncate.params = [["baseline", "variant"], [10, 100]] + time_truncate.param_names = ["role", "size"] +''') + + +def asv_tree(tmp, monkeypatch, source): + path = make_tree(tmp / "proj", {"asv.conf.json": ASV_CONF, "benchmarks/__init__.py": "", + "benchmarks/trunc.py": source}) + monkeypatch.chdir(path) + return path + + +def test_the_loop_with_an_asv_suite_names_cases_by_module(tmp, monkeypatch, capsys): + """An asv case name carries its module: `trunc.time_truncate('baseline')`, and so does the unit.""" + asv_tree(tmp, monkeypatch, ASV_ONE) + loop([".", "--adapter", "asv"], [[f"trunc.time_truncate({v})"] for v in (BASELINE, VARIANT)], ASV) + code, doc, err = compare(capsys) + assert code == cli.EXIT_OK and doc["failed_invariants"] == [] + assert "varying: role (the only parameter)" in err + [unit] = doc["units"] + assert unit["workload"] == {"name": "trunc.time_truncate"} and unit["effect"] > 0 + + +def test_the_loop_with_an_asv_suite_and_a_second_parameter(tmp, monkeypatch, capsys): + asv_tree(tmp, monkeypatch, ASV_TWO) + loop([".", "--adapter", "asv"], [["--filter", BASELINE], ["--filter", VARIANT]], ASV) + code, doc, _ = compare(capsys, "--vary", "role") + assert code == cli.EXIT_OK and doc["failed_invariants"] == [] + assert [u["workload"] for u in doc["units"]] == [ + {"name": "trunc.time_truncate", "parameters": {"size": "10"}}, + {"name": "trunc.time_truncate", "parameters": {"size": "100"}}] From e807760d4155e9d607cd786c23ba3d02ea49a49f Mon Sep 17 00:00:00 2001 From: ev-br's Claude Code Agent Date: Mon, 2 Nov 2026 17:47:32 +0000 Subject: [PATCH 6/9] MAINT: share the unknown-tree refusal, the dirty-side notes and the filter text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/benchx/adapters/benchx_params.py | 7 ++++- src/benchx/session.py | 40 +++++++++++++++++----------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/benchx/adapters/benchx_params.py b/src/benchx/adapters/benchx_params.py index 00df027..b0cd43b 100644 --- a/src/benchx/adapters/benchx_params.py +++ b/src/benchx/adapters/benchx_params.py @@ -56,6 +56,11 @@ def combination(values, index): return next(itertools.islice(itertools.product(*values), index, index + 1)) +def _search_text(name, combo) -> str: + """The text a case filter is searched in for one combination: `name(p0, p1)`.""" + return f"{name}({', '.join(combo)})" + + def expand(name, ids, case_filter): """The cases of one benchmark that `case_filter` (a regex, or None for all) selects, as (case id, combination index) pairs; the index is None for a benchmark without parameters. @@ -68,7 +73,7 @@ def expand(name, ids, case_filter): if not ids: return [(name, None)] if not case_filter or re.search(case_filter, name) else [] return [(f"{name}-{i}", i) for i, combo in enumerate(itertools.product(*ids)) - if not case_filter or re.search(case_filter, f"{name}({', '.join(combo)})")] + if not case_filter or re.search(case_filter, _search_text(name, combo))] def matches_name_only(name, ids, case_filter) -> bool: diff --git a/src/benchx/session.py b/src/benchx/session.py index d764dd2..d871662 100644 --- a/src/benchx/session.py +++ b/src/benchx/session.py @@ -212,6 +212,28 @@ def bench_target(target: str, *, suite: str, adapter: str, store=None, flags=Non return {**summary, "outcome": outcome, "precision": precision} +def _refuse_unknown_tree(name: str, side: dict) -> None: + """UC-03 §10: a dirty side is compared and labeled local-only. A side with no working-tree id + (its dirty state is `unknown`) cannot be named, so it is refused before any measurement, not + after R rounds.""" + if side["identity"]["dirty"] == "unknown": # schema §4.1: no tree id exactly when unknown + raise SessionError(f"{name} tree {side['source']} has no working-tree id (its dirty state is " + f"{side['identity']['dirty']}): the sides cannot be told apart, or checked for " + "a change during the run. Make every file in the tree readable, or git-ignore " + "it (UC-03 §10)") + + +def _note_local_only(progress, sides) -> None: + """Say before measuring which (name, side) pairs are not clean: the comparison will be local-only. + TODO: log these at logging.WARNING once benchx has logging (plan, Q13).""" + for name, side in sides: + for what, state in (("tree", side["identity"]["dirty"]), + ("benchmark file", side["benchmark_dirty"] if side["benchmark"] else "clean")): + if state != "clean": + where = f" {side['source']}" if what == "tree" else "" + progress(f"{name} {what}{where} is {state}: the comparison will be local-only") + + def compare_targets(baseline: str, contender: str, *, profile: str, suite: str, rounds: int, store, adapter, flags=None, file_precision=None, environment_variables=None, filter=None, quantities=("wall-time",), project=None, label=None, run_key=None, out=None, source_uri=None, @@ -251,22 +273,10 @@ def compare_targets(baseline: str, contender: str, *, profile: str, suite: str, sides = [_side(spec, suite, operation, harness, environment_variables, provider, timeout=orders.ORDER_TIMEOUT_S) for spec in (baseline, contender)] if profile == "revisions": - # UC-03 §10: a dirty side is compared and labeled local-only. A side with no working-tree id - # (its dirty state is `unknown`) cannot be named, so it is refused before any measurement, - # not after R rounds. for name, side in zip(("baseline", "contender"), sides): - if side["identity"]["dirty"] == "unknown": # schema §4.1: no tree id exactly when unknown - raise SessionError(f"{name} tree {side['source']} has no working-tree id (its dirty state is " - f"{side['identity']['dirty']}): the sides cannot be told apart, or checked for " - "a change during the run. Make every file in the tree readable, or git-ignore " - "it (UC-03 §10)") - if progress: # TODO: log these at logging.WARNING once benchx has logging (plan, Q13) - for name, side in zip(("baseline", "contender"), sides): - for what, state in (("tree", side["identity"]["dirty"]), - ("benchmark file", side["benchmark_dirty"] if side["benchmark"] else "clean")): - if state != "clean": - where = f" {side['source']}" if what == "tree" else "" - progress(f"{name} {what}{where} is {state}: the comparison will be local-only") + _refuse_unknown_tree(name, side) + if progress: + _note_local_only(progress, zip(("baseline", "contender"), sides)) source_uri = _source_uri(sides, source_uri) if len({json.dumps((side.get("benchmark") or {}).get("source")) for side in sides}) != 1: raise SessionError("the sides' suites come from different sources; give --suite as one absolute path") From 6c5b2c766b1541624b9d31c6e1d2b3475aba58f7 Mon Sep 17 00:00:00 2001 From: ev-br's Claude Code Agent Date: Mon, 2 Nov 2026 17:57:58 +0000 Subject: [PATCH 7/9] 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 --- src/benchx/adapters/benchx_params.py | 7 + src/benchx/cli.py | 50 +++--- src/benchx/session.py | 169 +++++++++++++++++++- tests/test_compare_harnesses.py | 221 +++++++++++++++++++++++++++ tests/test_params.py | 17 +++ tests/test_session.py | 23 +++ tests/test_uc02.py | 35 +++++ 7 files changed, 497 insertions(+), 25 deletions(-) diff --git a/src/benchx/adapters/benchx_params.py b/src/benchx/adapters/benchx_params.py index b0cd43b..bc27cd5 100644 --- a/src/benchx/adapters/benchx_params.py +++ b/src/benchx/adapters/benchx_params.py @@ -61,6 +61,13 @@ def _search_text(name, combo) -> str: return f"{name}({', '.join(combo)})" +def search_name(name, ids, index) -> str: + """The text a case filter is searched in: `name(p0, p1)` for the `index`-th combination of a + parametrized benchmark (`ids` are its value strings, one list per parameter), else the name. + Selecting a case by this text, exactly, is how a caller picks that one case (see `expand`).""" + return name if index is None else _search_text(name, combination(ids, index)) + + def expand(name, ids, case_filter): """The cases of one benchmark that `case_filter` (a regex, or None for all) selects, as (case id, combination index) pairs; the index is None for a benchmark without parameters. diff --git a/src/benchx/cli.py b/src/benchx/cli.py index 25c81c6..a651d9f 100644 --- a/src/benchx/cli.py +++ b/src/benchx/cli.py @@ -355,19 +355,24 @@ def _flags(args, names): return [f"--{n.replace('_', '-')}" for n in names if getattr(args, n) not in (None, [], False)] -def _mode(args) -> str: +def _mode(args, profile) -> str: + """read or measure. Measure mode takes two targets, except for variants, whose sides are two + values of one parameter of one target; and there `--baseline` picks the baseline value.""" + variants = profile == "variants" + targets = "one target" if variants else "two targets" if args.targets and args.run: - raise UsageError("give two targets or --run, not both") + raise UsageError(f"give {targets} or --run, not both") if not args.targets and not args.run: - raise UsageError("give two targets (measure mode) or --run KEY (read mode)") + raise UsageError(f"give {targets} (measure mode) or --run KEY (read mode)") if args.targets: - if len(args.targets) != 2: - raise UsageError("measure mode takes exactly two targets: BASELINE CONTENDER") - if wrong := _flags(args, _READ_ONLY): + if len(args.targets) != (1 if variants else 2): + raise UsageError("measure mode takes exactly one target for variants: TARGET" if variants + else "measure mode takes exactly two targets: BASELINE CONTENDER") + if wrong := _flags(args, ("results",) if variants else _READ_ONLY): raise UsageError(f"{', '.join(wrong)} belongs to read mode (--run)") return "measure" if wrong := _flags(args, _MEASURE_ONLY): - raise UsageError(f"{', '.join(wrong)} belongs to measure mode (two targets)") + raise UsageError(f"{', '.join(wrong)} belongs to measure mode ({targets})") if not args.baseline: raise UsageError("read mode needs --baseline") return "read" @@ -377,8 +382,8 @@ def cmd_compare(args): if (cfg := _load("compare")) is None: return EXIT_REFUSED try: - mode = _mode(args) policy = _policy(cfg, args) + mode = _mode(args, policy["profile"]) _profile_flags(args, policy["profile"]) except (UsageError, config_mod.ConfigError) as e: print(f"bx compare: error: {e}", file=sys.stderr) @@ -426,11 +431,16 @@ def _measure(cfg, args, policy): settings = _common(cfg, args) _needs_provider(cfg, *args.targets) snapshot.ignore(cfg.store_path, cfg.results_dir, *([args.out] if args.out else [])) - label = session.parse_label(args.label) if args.label else None - doc = session.compare_targets( - *args.targets, rounds=cfg.pick(args.rounds, cfg.section("compare").get("rounds"), flag="--rounds", - key="compare.rounds"), - store=store, label=label, progress=lambda message: print(message, file=sys.stderr), **policy, **settings) + rounds = cfg.pick(args.rounds, cfg.section("compare").get("rounds"), flag="--rounds", key="compare.rounds") + progress = lambda message: print(message, file=sys.stderr) # noqa: E731 + if policy["profile"] == "variants": + doc = session.compare_variants( + *args.targets, rounds=rounds, store=store, parameter=args.vary, baseline=args.baseline, + progress=progress, k=policy["k"], min_rounds=policy["min_rounds"], **settings) + else: + label = session.parse_label(args.label) if args.label else None + doc = session.compare_targets(*args.targets, rounds=rounds, store=store, label=label, progress=progress, + **policy, **settings) except runner.Refused as e: return _refused(e) except (UsageError, config_mod.ConfigError) as e: @@ -602,8 +612,12 @@ def main(argv=None): "Measure mode runs two prepared targets, alternating them over " "--rounds rounds, delivers the results to the local store, then compares: " "BASELINE CONTENDER --suite NAME --rounds R, each target BUILD_DIR[:SOURCE_DIR] " - "or @CONFIG[=SOURCE_REF] (the project's target provider).") - p.add_argument("targets", nargs="*", metavar="TARGET", help="measure mode: BASELINE CONTENDER") + "or @CONFIG[=SOURCE_REF] (the project's target provider). " + "The variants profile takes one target: its sides are two values of one " + "benchmark parameter (--vary), as in TARGET --profile variants --suite NAME " + "--rounds R.") + p.add_argument("targets", nargs="*", metavar="TARGET", + help="measure mode: BASELINE CONTENDER (variants: TARGET)") p.add_argument("--profile", choices=list(compare_mod.PROFILES), help="default: compare.profile") p.add_argument("--vary", metavar="NAME", help="variants: the workload parameter whose two values are the sides " @@ -613,10 +627,12 @@ def main(argv=None): p.add_argument("--json", action="store_true") p.add_argument("--label", help="read mode: the label that names the sides (environments); " "measure mode: NAME=BASE,CONTENDER") + p.add_argument("--baseline", help="revisions: a revision or tree id prefix; environments: the label value; " + "variants: the parameter's recorded value, quotes included ('baseline'); " + "required in read mode, and in measure mode only for variants " + "(default: the first declared value)") read = p.add_argument_group("read mode") read.add_argument("--run", help="run key") - read.add_argument("--baseline", help="revisions: a revision or tree id prefix; environments: the label value; " - "variants: the parameter's recorded value, quotes included ('baseline')") read.add_argument("--results", nargs="+", help="compare these result directories through a throwaway store, not the local one") measure = p.add_argument_group("measure mode") diff --git a/src/benchx/session.py b/src/benchx/session.py index d871662..3d3bc53 100644 --- a/src/benchx/session.py +++ b/src/benchx/session.py @@ -10,6 +10,10 @@ The session builds nothing itself. Both targets must exist when it looks; a target provider it calls may build them first (`prepare`), and plays the target-provider role. + +`compare_variants` is the same loop for one target: its sides are the two values +of one workload parameter of the benchmark (UC-02), so each round runs the cases +of one value, then the other's. """ import json @@ -18,6 +22,7 @@ from . import adapters, harnesses, orders, runner, snapshot, target as target_mod from . import compare as compare_mod +from .adapters import benchx_params from .adapters.base import Unsupported from .errors import SessionError from .identity import normalize_remote @@ -212,12 +217,13 @@ def bench_target(target: str, *, suite: str, adapter: str, store=None, flags=Non return {**summary, "outcome": outcome, "precision": precision} -def _refuse_unknown_tree(name: str, side: dict) -> None: +def _refuse_unknown_tree(side: dict, name: str | None = None) -> None: """UC-03 §10: a dirty side is compared and labeled local-only. A side with no working-tree id (its dirty state is `unknown`) cannot be named, so it is refused before any measurement, not - after R rounds.""" + after R rounds. `name` says which side of a comparison, when there are two.""" if side["identity"]["dirty"] == "unknown": # schema §4.1: no tree id exactly when unknown - raise SessionError(f"{name} tree {side['source']} has no working-tree id (its dirty state is " + who = f"{name} tree" if name else "tree" + raise SessionError(f"{who} {side['source']} has no working-tree id (its dirty state is " f"{side['identity']['dirty']}): the sides cannot be told apart, or checked for " "a change during the run. Make every file in the tree readable, or git-ignore " "it (UC-03 §10)") @@ -231,7 +237,7 @@ def _note_local_only(progress, sides) -> None: ("benchmark file", side["benchmark_dirty"] if side["benchmark"] else "clean")): if state != "clean": where = f" {side['source']}" if what == "tree" else "" - progress(f"{name} {what}{where} is {state}: the comparison will be local-only") + progress(f"{f'{name} ' if name else ''}{what}{where} is {state}: the comparison will be local-only") def compare_targets(baseline: str, contender: str, *, profile: str, suite: str, rounds: int, store, @@ -259,9 +265,8 @@ def compare_targets(baseline: str, contender: str, *, profile: str, suite: str, if profile not in PROFILES: raise SessionError(f"profile {profile!r} is not in the implementation ({', '.join(PROFILES)})") if profile == "variants": - # Its sides are two values of one parameter of one target, not two targets. - raise SessionError("the variants profile is not available in measure mode yet: measure each value " - "with bx bench --run-key/--round/--slot, then bx compare --run KEY --profile variants") + raise SessionError("the variants profile compares two values of one parameter of one target: " + "use compare_variants") if rounds < 1: raise SessionError("--rounds must be at least 1") if profile == "environments" and label is None: @@ -274,7 +279,7 @@ def compare_targets(baseline: str, contender: str, *, profile: str, suite: str, timeout=orders.ORDER_TIMEOUT_S) for spec in (baseline, contender)] if profile == "revisions": for name, side in zip(("baseline", "contender"), sides): - _refuse_unknown_tree(name, side) + _refuse_unknown_tree(side, name) if progress: _note_local_only(progress, zip(("baseline", "contender"), sides)) source_uri = _source_uri(sides, source_uri) @@ -305,3 +310,151 @@ def compare_targets(baseline: str, contender: str, *, profile: str, suite: str, return compare_mod.compare(store.documents(run_key), run_key=run_key, profile=profile, baseline=base_value, label=label[0] if label else None, k=k, min_rounds=min_rounds) + + +def _declared_by(runnable: dict, environment_variables) -> dict[str, list[str]]: + """Each workload parameter an order would record, and who declares it: the selected + benchmarks' `param_names`, and the order's `-e` variables (shown as `-e`), which the + runner records as workload parameters too.""" + found: dict[str, list[str]] = {} + for name, info in sorted(runnable["benchmarks"].items()): + for parameter in info["param_names"]: + found.setdefault(parameter, []).append(name) + for name in sorted(environment_variables or {}): + found.setdefault(name, []).append("-e") + return found + + +def _infer_parameter(runnable: dict, environment_variables) -> str: + """The workload parameter to vary when the caller names none: the only one the selected + benchmarks and the order's variables give. Several are refused, naming who declares each.""" + found = _declared_by(runnable, environment_variables) + if not found: + raise SessionError("nothing to vary: no workload parameter in the selected benchmarks " + "(declare one in `params`/`param_names`)") + if len(found) > 1: + who = ", ".join(f"{parameter} ({', '.join(declared)})" for parameter, declared in sorted(found.items())) + raise SessionError(f"several workload parameters: {who}: give --vary NAME, " + "or --filter to select the benchmarks") + return next(iter(found)) + + +def _split_cases(module, runnable: dict, cases: list[str], parameter: str) -> tuple[dict, list[str]]: + """The selected cases by their value of `parameter`, as the search names that select each + one exactly (`benchx_params.search_name`), in declaration order; and the benchmarks that + have no such parameter, whose cases run on neither side.""" + by_value: dict[str, list[str]] = {} + without: list[str] = [] + for case in cases: + name, index = runnable["cases"][case] + value = module.workload(case, runnable)[1].get(parameter) + if value is None: + if name not in without: + without.append(name) + continue + by_value.setdefault(value, []).append( + benchx_params.search_name(name, runnable["benchmarks"][name]["params"], index)) + return by_value, without + + +def _the_two_values(runnable: dict, cases: list[str], parameter: str) -> list[str]: + """The two values of `parameter` the selected cases take, in declaration order. Every + benchmark that has the parameter must take the same two, or a third value would fail + `two-sides` only after the whole run.""" + taken: dict[str, list[str]] = {} + for case in cases: + name, index = runnable["cases"][case] + info = runnable["benchmarks"][name] + if parameter in info["param_names"]: + value = benchx_params.combination(info["params"], index)[info["param_names"].index(parameter)] + taken.setdefault(name, []) + if value not in taken[name]: + taken[name].append(value) + if not taken: + declared = sorted(_declared_by(runnable, None)) + raise SessionError(f"no selected benchmark declares a parameter {parameter!r}" + + (f" (declared: {', '.join(declared)})" if declared else "")) + values = next(iter(taken.values())) + if len(values) != 2 or any(set(v) != set(values) for v in taken.values()): + shown = "; ".join(f"{name}: {', '.join(v)}" for name, v in taken.items()) + raise SessionError(f"a comparison has two sides: the selected benchmarks must take the same two " + f"values of {parameter}, not [{shown}]. Narrow them with --filter") + return values + + +def compare_variants(target: str, *, suite: str, rounds: int, store, adapter, parameter=None, baseline=None, + flags=None, file_precision=None, environment_variables=None, filter=None, + quantities=("wall-time",), project=None, run_key=None, out=None, source_uri=None, + k=3.0, min_rounds=3, progress=None, build=True, provider=None, results_dir=None, + case_s=orders.CASE_TIMEOUT_S, order_s=orders.ORDER_TIMEOUT_S, config_path=None, + collectors=None) -> dict: + """Alternate the two values of one workload parameter of one target over `rounds` rounds, + deliver every result to `store`, and return the run-mode comparison document (UC-02). + + The benchmark declares the parameter (`params` and `param_names`). `parameter` is its + name, else the only parameter the selected benchmarks and `environment_variables` give. + Each round runs one order per value: that value's cases only, `baseline` first at slot + 2r (a value as recorded, `'baseline'` with its quotes; default the first declared), the + other at slot 2r + 1. Benchmarks without the parameter run on neither side. The other + arguments are as for `compare_targets`. Raises SessionError when the session cannot + start, and runner.Refused when the runner refuses an order. + """ + if rounds < 1: + raise SessionError("--rounds must be at least 1") + harness, precision = _harness(adapter, flags, file_precision, quantities) + module = adapters.get(harness.name) + if not hasattr(module, "workload"): + raise SessionError(f"variants needs a parameter the benchmark declares, and {harness.name} " + "benchmarks have none") + side = _side(target, suite, "prepare" if build else "describe", harness, environment_variables, provider, + timeout=order_s) + _refuse_unknown_tree(side) + if progress: + _note_local_only(progress, [(None, side)]) + source_uri = _source_uri([side], source_uri) + + # List the cases once, as the runner will, to find the parameter and split them by value. + env = {**os.environ, **(environment_variables or {})} + try: + runnable = module.locate(side["path"], suite, env, timeout=order_s) + cases = module.list_cases(runnable, filter, env, timeout=order_s) + except Unsupported as e: + raise SessionError("; ".join(str(problem) for problem in e.problems)) from None + if parameter is None: + parameter = _infer_parameter(runnable, environment_variables) + if progress: + progress(f"varying: {parameter} (the only parameter)") + by_value, without = _split_cases(module, runnable, cases, parameter) + values = _the_two_values(runnable, cases, parameter) + if baseline is None: + baseline = values[0] + elif baseline not in values: + raise SessionError(f"baseline {baseline!r} is not a value of {parameter}; use one of: " + f"{', '.join(values)}") + contender = next(value for value in values if value != baseline) + if without and progress: + progress(f"not run on either side (no parameter {parameter}): {', '.join(without)}") + + run_key = run_key or orders.default_run_key("compare") + out = Path(out) if out else Path(results_dir or "results") / run_key.replace("/", "_") + filters = {value: harnesses.exact_filter(by_value[value], arguments=True) for value in values} + + slot = 0 + for round_ in range(rounds): + for value in (baseline, contender): + summary, outcome = _run_side(side, out / "orders" / f"order-{slot:04d}.json", store, + harness=harness, suite=suite, filter=filters[value], quantities=quantities, + precision=precision, environment_variables=environment_variables, + project=project, run_key=run_key, source_uri=source_uri, + round_=round_, slot=slot, case_s=case_s, order_s=order_s, + collectors=collectors) + if outcome["rejected"]: + codes = sorted({r["code"] for r in outcome["rejected"]}) + raise SessionError(f"the store rejected results of round {round_} ({parameter}={value}): " + f"{', '.join(codes)}") + if progress: + progress(f"round {round_ + 1}/{rounds} {parameter}={value}: {summary['results']} result(s)") + slot += 1 + + return compare_mod.compare(store.documents(run_key), run_key=run_key, profile="variants", + baseline=baseline, parameter=parameter, k=k, min_rounds=min_rounds) diff --git a/tests/test_compare_harnesses.py b/tests/test_compare_harnesses.py index e84dc18..a218b59 100644 --- a/tests/test_compare_harnesses.py +++ b/tests/test_compare_harnesses.py @@ -6,6 +6,7 @@ """ import json +import os import textwrap from pathlib import Path @@ -165,3 +166,223 @@ def test_pyperf_bare_paths_are_working_trees(tmp, worktrees): order = json.loads(Path(tmp / "out" / "orders" / "order-0001.json").read_text()) assert (order["suites"][0]["adapter"], order["target"]["kind"]) == ("pyperf", "working_tree") assert order["target"]["path"] == str(worktrees["head"].resolve()) + + +# --- variants: one target, the two values of one parameter (UC-02) ------------------------------ + +VARIANTS = textwrap.dedent(''' + def bench_truncate(role): + sum(range(10 if role == "baseline" else 2000)) + + bench_truncate.params = ["baseline", "variant"] + bench_truncate.param_names = ["role"] +''') +SIZED = textwrap.dedent(''' + def bench_truncate(role, size): + sum(range(size if role == "baseline" else 20 * size)) + + bench_truncate.params = [["baseline", "variant"], [10, 100]] + bench_truncate.param_names = ["role", "size"] +''') +STRAY = textwrap.dedent(''' + def bench_other(size): + sum(range(size)) + + bench_other.params = [10, 100] + bench_other.param_names = ["size"] +''') +PLAIN = "\ndef bench_plain():\n sum(range(10))\n" +FAST = {"processes": 2, "values": 2, "loops": 50, "warmups": 0} + + +def variants_tree(tmp, source, files=None): + """A clean git repository holding the suite `mod.py` (and `files`, such as an asv directory).""" + tree = tmp / "tree" + tree.mkdir() + git(tree, "init", "-q") + for name, text in {"mod.py": source, **(files or {})}.items(): + (tree / name).parent.mkdir(parents=True, exist_ok=True) + (tree / name).write_text(text) + git(tree, "add", "-A") + git(tree, "commit", "-qm", "suite") + return tree + + +def run_variants(tmp, tree, **kwargs): + """`session.compare_variants` on `tree`: the document, the store, and what the session said.""" + store, said = Store(tmp / "s.parquet"), [] + settings = {"suite": "mod.py", "rounds": 2, "min_rounds": 2, "adapter": "pyperf", "flags": FAST, + "out": tmp / "out", "run_key": "v", **kwargs} + doc = session.compare_variants(str(tree), store=store, progress=said.append, **settings) + return doc, store, said + + +def measurands(store, run_key="v"): + return [d for d in store.documents(run_key) if d["measurement"].get("kind", "measurand") == "measurand"] + + +def test_variants_run_one_order_per_value_and_round_with_that_values_cases_only(tmp): + pytest.importorskip("pyperf") + doc, store, said = run_variants(tmp, variants_tree(tmp, SIZED), parameter="role") + assert doc["failed_invariants"] == [] and doc["missing"] == [] and doc["varying"].endswith("parameters.role") + assert (doc["baseline"], doc["contender"]) == ("'baseline'", "'variant'") + assert [u["workload"] for u in doc["units"]] == [ + {"name": "bench_truncate", "parameters": {"size": "10"}}, + {"name": "bench_truncate", "parameters": {"size": "100"}}] + # Four orders, one per value and round; each holds exactly its own value's cases. + orders = sorted((tmp / "out" / "orders").glob("order-*.json")) + assert [o.name for o in orders] == [f"order-{slot:04d}.json" for slot in range(4)] + for slot, path in enumerate(orders): + value = ("baseline", "variant")[slot % 2] + assert json.loads(path.read_text())["suites"][0]["filter"] == harnesses.exact_filter( + [f"bench_truncate('{value}', 10)", f"bench_truncate('{value}', 100)"], arguments=True) + # The results, by slot: baseline, variant, baseline, variant; both sizes in each. + by_slot = {} + for d in measurands(store): + by_slot.setdefault(d["procedure"]["slot"], []).append(d["coordinates"]["workload"]["parameters"]) + assert sorted(by_slot) == [0, 1, 2, 3] + assert [sorted({p["role"] for p in by_slot[slot]}) for slot in range(4)] == [ + ["'baseline'"], ["'variant'"], ["'baseline'"], ["'variant'"]] + assert all(sorted(p["size"] for p in by_slot[slot]) == ["10", "100"] for slot in range(4)) + assert [m.split(":")[0] for m in said if m.startswith("round ")] == [ + "round 1/2 role='baseline'", "round 1/2 role='variant'", "round 2/2 role='baseline'", + "round 2/2 role='variant'"] + + +def test_variants_the_baseline_is_the_first_declared_value_and_can_be_swapped(tmp): + pytest.importorskip("pyperf") + tree = variants_tree(tmp, VARIANTS) + doc, _, said = run_variants(tmp, tree) + assert doc["failed_invariants"] == [] and (doc["baseline"], doc["contender"]) == ("'baseline'", "'variant'") + assert "varying: role (the only parameter)" in said + assert doc["units"][0]["workload"] == {"name": "bench_truncate"} and doc["units"][0]["effect"] > 0 + swapped, _, _ = run_variants(tmp, tree, baseline="'variant'", out=tmp / "out2", run_key="v2") + assert swapped["failed_invariants"] == [] and (swapped["baseline"], swapped["contender"]) == ("'variant'", "'baseline'") + assert swapped["units"][0]["effect"] < 0 + # Slot 0 of the swapped run is the variant. + assert next(d for d in measurands(Store(tmp / "s.parquet"), "v2") if d["procedure"]["slot"] == 0)[ + "coordinates"]["workload"]["parameters"] == {"role": "'variant'"} + + +def test_variants_an_environment_variable_is_a_second_parameter(tmp): + pytest.importorskip("pyperf") + tree, store = variants_tree(tmp, VARIANTS), Store(tmp / "s.parquet") + with pytest.raises(session.SessionError, match=r"several workload parameters: OMP_NUM_THREADS \(-e\), role " + r"\(bench_truncate\): give --vary NAME"): + run_variants(tmp, tree, environment_variables={"OMP_NUM_THREADS": "1"}) + assert not (tmp / "out").exists() and list(store.documents("v")) == [] + doc, _, _ = run_variants(tmp, tree, environment_variables={"OMP_NUM_THREADS": "1"}, parameter="role") + assert doc["failed_invariants"] == [] + assert doc["units"][0]["workload"]["parameters"] == {"OMP_NUM_THREADS": "1"} + + +def test_variants_a_stray_parametrized_benchmark_needs_vary_or_a_filter(tmp): + pytest.importorskip("pyperf") + tree = variants_tree(tmp, VARIANTS + STRAY) + with pytest.raises(session.SessionError) as error: + run_variants(tmp, tree) + assert "role (bench_truncate)" in str(error.value) and "size (bench_other)" in str(error.value) + assert "--filter" in str(error.value) and not (tmp / "out").exists() + doc, _, said = run_variants(tmp, tree, filter="bench_truncate") # --filter narrows what is inferred from + assert doc["failed_invariants"] == [] and "varying: role (the only parameter)" in said + + +def test_variants_a_benchmark_without_the_parameter_runs_on_neither_side_and_is_reported(tmp): + pytest.importorskip("pyperf") + doc, store, said = run_variants(tmp, variants_tree(tmp, VARIANTS + PLAIN)) # no --vary: bench_plain adds no name + assert doc["failed_invariants"] == [] and [u["workload"]["name"] for u in doc["units"]] == ["bench_truncate"] + assert "not run on either side (no parameter role): bench_plain" in said + assert {d["coordinates"]["workload"]["name"] for d in measurands(store)} == {"bench_truncate"} + + +@pytest.mark.parametrize("source, kwargs, message", [ + (PLAIN, {}, "nothing to vary: no workload parameter in the selected benchmarks"), + (VARIANTS, {"parameter": "size"}, "no selected benchmark declares a parameter 'size' \\(declared: role\\)"), + (VARIANTS.replace('["baseline", "variant"]', '["a", "b", "c"]'), {}, + "must take the same two values of role, not \\[bench_truncate: 'a', 'b', 'c'\\]"), + (VARIANTS + VARIANTS.replace("bench_truncate", "bench_other").replace('["baseline", "variant"]', '["old", "new"]'), + {}, "must take the same two values of role, not \\[bench_other: 'old', 'new'; bench_truncate: 'baseline', " + "'variant'\\]"), + (VARIANTS, {"filter": "'baseline'"}, "must take the same two values of role, not \\[bench_truncate: 'baseline'\\]"), + (VARIANTS, {"baseline": "variant"}, "baseline 'variant' is not a value of role; use one of: 'baseline', 'variant'"), + (VARIANTS, {"baseline": "'varient'"}, "baseline \"'varient'\" is not a value of role; use one of: 'baseline', 'variant'"), +], ids=["no-parameter", "unknown-parameter", "three-values", "benchmarks-disagree", "filter-leaves-one-value", + "unquoted-baseline", "mistyped-baseline"]) +def test_variants_refusals_come_before_anything_runs(tmp, source, kwargs, message): + pytest.importorskip("pyperf") + tree, store = variants_tree(tmp, source), Store(tmp / "s.parquet") + with pytest.raises(session.SessionError, match=message): + run_variants(tmp, tree, **kwargs) + assert not (tmp / "out").exists() and list(store.documents("v")) == [] + + +def test_variants_google_benchmark_has_no_parameters_to_vary(tmp): + with pytest.raises(session.SessionError, match="variants needs a parameter the benchmark declares"): + session.compare_variants("nowhere", suite="s", rounds=1, store=Store(tmp / "s.parquet"), + adapter="google-benchmark") + + +@pytest.mark.skipif(os.geteuid() == 0, reason="root reads any file") +def test_variants_a_tree_with_no_tree_id_is_refused_before_anything_runs(tmp): + pytest.importorskip("pyperf") + tree = variants_tree(tmp, VARIANTS) + unreadable = tree / "unreadable.txt" + unreadable.write_text("x\n") + unreadable.chmod(0) + try: + with pytest.raises(session.SessionError, match=r"^tree .* has no working-tree id.*unknown"): + run_variants(tmp, tree) + finally: + unreadable.chmod(0o644) + assert not (tmp / "out").exists() + + +def test_variants_a_dirty_tree_is_said_before_measuring_and_compared_local_only(tmp): + pytest.importorskip("pyperf") + tree = variants_tree(tmp, VARIANTS) + (tree / "mod.py").write_text(VARIANTS + "# edited\n") + doc, _, said = run_variants(tmp, tree) + assert doc["failed_invariants"] == [] and doc["local_only"] is True + assert f"tree {tree.resolve()} is dirty: the comparison will be local-only" in said + assert said.index(next(m for m in said if "local-only" in m)) < said.index( + next(m for m in said if m.startswith("round 1/2"))) + + +ASV_CONF = '{"version": 1, "project": "uc02", "benchmark_dir": "benchmarks"}\n' +ASV_FILES = {"asv.conf.json": ASV_CONF, "benchmarks/__init__.py": "", "benchmarks/trunc.py": textwrap.dedent(''' + def time_truncate(role): + sum(range(10 if role == "baseline" else 2000)) + + time_truncate.params = ["baseline", "variant"] + time_truncate.param_names = ["role"] + + def time_other(size): + sum(range(size)) + + time_other.params = [10, 100] + time_other.param_names = ["size"] +''')} +ASV_FAST = {"processes": 2, "values": 2, "warmups": 0, "min_time": 0.001} + + +def test_variants_asv_cases_are_split_by_value_under_their_module_qualified_names(tmp): + from benchx import adapters + tree = variants_tree(tmp, "", ASV_FILES) + asv, env = adapters.get("asv"), dict(os.environ) + runnable = asv.locate(tree, "benchmarks", env) + cases = asv.list_cases(runnable, None, env) + by_value, without = session._split_cases(asv, runnable, cases, "role") + assert without == ["trunc.time_other"] # it has no `role`: it runs on neither side + assert by_value == {"'baseline'": ["trunc.time_truncate('baseline')"], + "'variant'": ["trunc.time_truncate('variant')"]} + for value, names in by_value.items(): # each side's filter selects exactly that side's cases + selected = asv.list_cases(runnable, harnesses.exact_filter(names, arguments=True), env) + assert [asv.workload(case, runnable) for case in selected] == [("trunc.time_truncate", {"role": value})] + + +def test_variants_asv_end_to_end(tmp): + tree = variants_tree(tmp, "", ASV_FILES) + doc, store, said = run_variants(tmp, tree, suite="benchmarks", adapter="asv", flags=ASV_FAST, parameter="role") + assert doc["failed_invariants"] == [] and [u["workload"] for u in doc["units"]] == [{"name": "trunc.time_truncate"}] + assert {d["coordinates"]["workload"]["name"] for d in measurands(store)} == {"trunc.time_truncate"} + assert "not run on either side (no parameter role): trunc.time_other" in said diff --git a/tests/test_params.py b/tests/test_params.py index 40ad542..0e3e791 100644 --- a/tests/test_params.py +++ b/tests/test_params.py @@ -1,5 +1,6 @@ """benchx_params: asv's parameter conventions, shared by the adapters whose benchmarks take them.""" +import re from pathlib import Path from benchx.adapters import asv_runtime, base, benchx_params as params @@ -86,3 +87,19 @@ def test_the_note_names_the_benchmarks_it_means(): note = base.name_only_note(["a", "b", "c", "d"]) assert "(a, b, c)" in note # three at most assert "name(p0, p1)" in note + + +def test_search_name_is_exactly_what_expand_searches(): + """The session picks a case by this text, and the runner expands its filter against the same text.""" + one = [["'a'", "'b'"]] + two = [["'a'", "'b'"], ["1", "2"]] + dupes = [["1 (0)", "1 (1)"], ["'x'"]] + assert params.search_name("f", one, 1) == "f('b')" + assert params.search_name("f", two, 3) == "f('b', 2)" + assert params.search_name("f", dupes, 1) == "f(1 (1), 'x')" + assert params.search_name("f", [], None) == "f" # a benchmark without parameters + for ids in (one, two, dupes): + for case, index in params.expand("f", ids, None): + exactly = "^" + re.escape(params.search_name("f", ids, index)) + "$" + assert params.expand("f", ids, exactly) == [(case, index)] + assert params.expand("g", [], "^" + re.escape(params.search_name("g", [], None)) + "$") == [("g", None)] diff --git a/tests/test_session.py b/tests/test_session.py index 4558b88..afda316 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -151,7 +151,30 @@ def test_measure_mode_equals_read_mode(repo, tmp, builds, capsys): "--vary names the parameter"), (["--run", "k", "--profile", "variants", "--baseline", "x", "--label", "e"], "--label names the sides"), (["--run", "k", "--profile", "variants"], "needs --baseline"), + (["A", "B", "--profile", "variants", "--suite", "s", "--rounds", "3"], "exactly one target for variants"), + (["--profile", "variants"], "give one target (measure mode) or --run KEY"), + (["A", "--run", "k", "--profile", "variants"], "give one target or --run, not both"), + (["A", "--profile", "variants", "--results", "r"], "belongs to read mode"), + (["--run", "k", "--profile", "variants", "--baseline", "x", "--rounds", "3"], "belongs to measure mode (one target)"), + (["A", "B", "--profile", "revisions", "--suite", "s", "--rounds", "3", "--vary", "role"], "--vary names the parameter"), + (["A", "--profile", "variants", "--suite", "s", "--rounds", "3", "--label", "e=a,b"], "--label names the sides"), ]) def test_mode_selection(argv, message, capsys): assert cli.main(["compare", *argv]) == cli.EXIT_REFUSED assert message in capsys.readouterr().err + + +@pytest.mark.parametrize("argv, message", [ + # one target, and --baseline in measure mode: both are for variants only; no --vary is fine in either mode + (["nowhere", "--profile", "variants", "--suite", "s", "--rounds", "3", "--adapter", "pyperf"], + "working tree not found"), + (["nowhere", "--profile", "variants", "--suite", "s", "--rounds", "3", "--adapter", "pyperf", + "--baseline", "'variant'", "--vary", "role"], "working tree not found"), + (["--run", "k", "--profile", "variants", "--baseline", "'x'"], "no results for run 'k'"), + (["--run", "k", "--profile", "variants", "--baseline", "'x'", "--vary", "role"], "no results for run 'k'"), +]) +def test_variants_mode_selection_is_accepted(argv, message, capsys): + """These get past mode selection; what refuses them is later (the target, the run).""" + assert cli.main(["compare", *argv]) == cli.EXIT_REFUSED + err = capsys.readouterr().err + assert message in err and "bx compare: error" not in err diff --git a/tests/test_uc02.py b/tests/test_uc02.py index d3c0871..4d70f3d 100644 --- a/tests/test_uc02.py +++ b/tests/test_uc02.py @@ -210,3 +210,38 @@ def test_the_loop_with_an_asv_suite_and_a_second_parameter(tmp, monkeypatch, cap assert [u["workload"] for u in doc["units"]] == [ {"name": "trunc.time_truncate", "parameters": {"size": "10"}}, {"name": "trunc.time_truncate", "parameters": {"size": "100"}}] + + +# --- measure mode: the same loop in one command (§1) -------------------------------------------- + +def measure(capsys, *extra): + capsys.readouterr() + code = cli.main(["compare", ".", "--profile", "variants", "--suite", "mod.py", "--adapter", "pyperf", + "--rounds", "3", "--run-key", RUN, "--json", *PYPERF, *extra]) + captured = capsys.readouterr() + return code, (json.loads(captured.out) if captured.out.strip() else None), captured.err + + +def test_measure_mode_runs_the_loop_in_one_command_and_equals_read_mode(tree, capsys): + code, doc, err = measure(capsys) + assert code == cli.EXIT_OK and doc["failed_invariants"] == [] and core.check(doc, "comparison-document") == [] + assert "varying: role (the only parameter)" in err and "round 3/3 role='variant'" in err + assert (doc["baseline"], doc["contender"]) == (BASELINE, VARIANT) and doc["local_only"] is False + [unit] = doc["units"] + assert unit["workload"] == {"name": "bench_truncate"} and unit["rounds"] == 3 and unit["effect"] > 0 + # What the session stored compares to the same document when read back (criterion 8). + code, again, _ = compare(capsys) + assert code == cli.EXIT_OK and again == doc + + +def test_measure_mode_takes_the_baseline_value(tree, capsys): + code, doc, _ = measure(capsys, "--baseline", VARIANT) + assert code == cli.EXIT_OK and doc["failed_invariants"] == [] + assert (doc["baseline"], doc["contender"]) == (VARIANT, BASELINE) and doc["units"][0]["effect"] < 0 + + +def test_measure_mode_refuses_a_baseline_that_is_not_a_value_before_running(tree, capsys): + code, doc, err = measure(capsys, "--baseline", "variant") # the recorded value has its quotes + assert code == cli.EXIT_REFUSED and doc is None + assert "baseline 'variant' is not a value of role; use one of: 'baseline', 'variant'" in err + assert not (tree / "results").exists() # nothing ran From 46c4c0e975cc3d2db72dba0fd41cb86e76124355 Mon Sep 17 00:00:00 2001 From: ev-br's Claude Code Agent Date: Mon, 2 Nov 2026 18:12:55 +0000 Subject: [PATCH 8/9] 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 --- docs/benchx.md | 57 ++++++++++++++++++++++---- docs/design/benchmark-result-schema.md | 6 +-- docs/design/comparator.md | 15 +++++-- docs/design/prototype-design.md | 57 ++++++++++++++++++++++---- docs/design/prototype-scope.md | 9 +++- 5 files changed, 120 insertions(+), 24 deletions(-) diff --git a/docs/benchx.md b/docs/benchx.md index 4f21ecf..09d4235 100644 --- a/docs/benchx.md +++ b/docs/benchx.md @@ -48,9 +48,13 @@ bx compare BASELINE CONTENDER --profile revisions|environments [--project P] [--label NAME=BASE,CONTENDER] [--run-key KEY] [--out DIR] [--no-build] [--k 3] [--json] +# measure mode, one target: alternate the two values of a benchmark parameter +bx compare TARGET --profile variants --adapter pyperf|asv --suite NAME --rounds R + [--vary NAME] [--baseline VALUE] [--filter REGEX] [...] # the options above, except --label + # read mode: compare results already measured -bx compare --run KEY --profile revisions|environments --baseline VALUE - [--label NAME] [--results DIR] [--k 3] [--json] +bx compare --run KEY --profile revisions|environments|variants --baseline VALUE + [--label NAME] [--vary NAME] [--results DIR] [--k 3] [--json] # low-level command: run a handwritten work oreder bx run [--out DIR] [--no-ingest] @@ -84,8 +88,8 @@ pyperf and for Google Benchmark. The store and the results directory are left ou measured tree's identity when they sit inside it. `.benchx/setup.json` (declared environment facts) is a separate file, written by whoever prepares an environment. -Each target is `BUILD_DIR[:SOURCE_DIR]` or `@CONFIG[=SOURCE_REF]`. Two targets -select measure mode and `--run` selects read mode; mixing them is an error. +Each target is `BUILD_DIR[:SOURCE_DIR]` or `@CONFIG[=SOURCE_REF]`. Two targets (one for +`--profile variants`) select measure mode and `--run` selects read mode; mixing them is an error. `@CONFIG` asks the project's **target provider**, the command named by `provider.command` in `benchx.conf.json` (it runs with the project root as its working directory). The provider speaks the contract of @@ -131,6 +135,39 @@ and `--slot K` place each call in the session, and `bx compare --run KEY --profi `examples/numpy-venvs` in [benchx_examples](https://github.com/benchx-dev/benchx_examples) is the worked example, with a shell script that drives it. +Two functions of one benchmark ([UC-02](https://benchx-dev.github.io/benchx/use-cases/UC-02-variants/)) are the two values of +one parameter, declared as pyperf and asv benchmarks declare theirs. Build the data once, outside the +function (at module level, or behind `functools.cache`), because pyperf times the whole body: + +```python +IMPLS = {"baseline": pandas_native_truncate, "variant": pandas_via_narwhals_truncate} + +def bench_truncate(role): + IMPLS[role](data()) + +bench_truncate.params = ["baseline", "variant"] +bench_truncate.param_names = ["role"] +``` + +One command alternates the two values over the rounds and compares them (`adapter.name` in +`benchx.conf.json` makes `--adapter` optional): + +```console +$ bx compare . --profile variants --adapter pyperf --suite bench_suite.py --rounds 5 +``` + +The values are recorded as `repr()` strings. The baseline is the first declared value, and +`--baseline "'variant'"` swaps the sides (spelled as recorded, quotes included). `--vary NAME` +names the parameter and is needed only when the selected benchmarks have more than one: every +combination of the others is then its own unit (`bench_truncate [size=1000]`), and `--filter` +narrows the benchmarks. A benchmark without the parameter runs on neither side, and a failed +attempt on either side makes its unit indeterminate (`side-failed`). The verdict is the +comparator's noise-aware one; there is no tolerance flag yet. As for UC-04, the loop can be the +caller's: one `bx bench` call per value (a case name such as `"bench_truncate('baseline')"`, or +`--filter "'baseline'"`) with `--run-key`, `--round` and `--slot`, then `bx compare --run KEY +--profile variants --baseline "'baseline'"`. One `bx bench` call that runs both values is not an +alternating run, and the comparison says so. + `bx bench PATH --adapter asv` runs an [asv](https://asv.readthedocs.io) benchmark suite unchanged, reading asv's benchmark format as a language and timing it with pyperf; neither asv nor asv_runner is used. PATH is a directory holding `asv.conf.json` (its @@ -199,9 +236,11 @@ every result is pyperf. Results also record the installed project's version are not clean), writes one order per side and round, alternates them over the rounds, delivers every result to the local store, and hands the run to the comparator. It builds nothing: both build - directories must already exist. -- **The comparator** (`bx compare --run`) runs in run mode for the `revisions` and - `environments` profiles, each a projection built in code from the identity + directories must already exist. For `variants` it has one target: it lists + the cases once, splits them by the two values of one parameter, and writes + one order per value and round. +- **The comparator** (`bx compare --run`) runs in run mode for the `revisions`, + `environments` and `variants` profiles, each a projection built in code from the identity policy (`benchx/compare.py`): the axis tells the sides apart, everything the policy matches must agree, and it checks the profile's invariants (`revisions` also requires one tree id per side and, for a benchmark recorded apart from the @@ -210,7 +249,9 @@ every result is pyperf. Results also record the installed project's version sides by round, and applies `benchx/paired-relative/v1` (effect = mean relative paired difference, noise = its standard error, change when |effect| > *k* × noise). `environments` names its sides by the label - `--label` gives. `--results` compares result files through a throwaway + `--label` gives, and `variants` by the two values of one workload parameter + (`--vary`); under `variants` a failed attempt on either side fails its unit + (`side-failed`). `--results` compares result files through a throwaway store: the ad hoc path. Condition measurements (`measurement.kind: condition`) are not compared, and deterministic quantities are deferred. diff --git a/docs/design/benchmark-result-schema.md b/docs/design/benchmark-result-schema.md index a551974..9c019db 100644 --- a/docs/design/benchmark-result-schema.md +++ b/docs/design/benchmark-result-schema.md @@ -497,16 +497,16 @@ Results carry facts; a **profile** is a comparator-time check that a set of resu | Profile | Varies | Must agree | Invariants checked over the run | |---|---|---|---| | `single-run` ([UC-01](../use-cases/UC-01-short-slug.md)) | workload parameters | protocol | none beyond attempt integrity | -| `variants` ([UC-02](../use-cases/UC-02-variants.md)) | one or more workload parameters chosen by the caller, `parameters.role` by default | subject tree id, subject configuration, environment, protocol, every other workload parameter | equal attempt counts per side; sides alternate by `procedure.slot` | +| `variants` ([UC-02](../use-cases/UC-02-variants.md)) | one or more workload parameters chosen by the caller | subject tree id, subject configuration, environment, protocol, every other workload parameter | equal attempt counts per side; sides alternate by `procedure.slot`; a separately recorded benchmark has one `benchmark_tree` across both sides and all rounds, invariant `one-benchmark-tree` | | `revisions` ([UC-03](../use-cases/UC-03-compare.md)) | subject tree id | workload variant, subject configuration, environment, protocol | equal attempt counts per side; sides alternate by `procedure.slot`; each side has exactly one tree id, so a side whose dirty state is `unknown` and has none fails, and the two differ; a separately recorded benchmark has one `benchmark_tree` across both sides and all rounds, invariant `one-benchmark-tree` | | `environments` ([UC-04](../use-cases/UC-04-cross-env.md)) | one pinned component, subject configuration, or `environment.system` field, named by `labels` | subject revision, workload variant, host environment, protocol | exactly two label values whose reported coordinates differ; equal attempt counts per label; sides alternate by `procedure.slot` | | `cross-machine` (template example, pending a use case) | environment identity | subject tree id, workload variant, subject configuration | each side references an anchor result from the same run via `provenance.references` | -**Sides are chosen at comparison time.** The coordinate a profile varies is an argument to the comparator, not a declaration by the producer. Workload parameters are always recorded, so any parameter or set of parameters can be the varying one: `stable` for a sort-algorithm study, `fast_path` for a fast-path deficit, `{role, size}` for a two-factor design. Each distinct assignment of the chosen parameters is one side, and every other coordinate must agree. `role` is only the conventional name that UC-02's `--baseline`/`--variant` flags write; nothing in the schema privileges it. A comparison that was not planned when the run was made is therefore the same operation as one that was, provided the run contains the sides; what a run did not measure, the comparator cannot invent, and a planned run is what guarantees the interleaving invariant. +**Sides are chosen at comparison time.** The coordinate a profile varies is an argument to the comparator, not a declaration by the producer. Workload parameters are always recorded, so any parameter or set of parameters can be the varying one: `stable` for a sort-algorithm study, `fast_path` for a fast-path deficit, `{role, size}` for a two-factor design. Each distinct assignment of the chosen parameters is one side, and every other coordinate must agree. A comparison has two sides (`comparator.md` §1), so the chosen parameters must take exactly two joint assignments in the run. `role` is only the conventional name that UC-02's `--baseline`/`--variant` flags write; nothing in the schema privileges it. A comparison that was not planned when the run was made is therefore the same operation as one that was, provided the run contains the sides; what a run did not measure, the comparator cannot invent, and a planned run is what guarantees the interleaving invariant. A comparison in which any side is not `clean` is **local-only** under the profiles that allow it (`single-run`, `variants`, `revisions`, `environments`): the comparator produces it, labels it so, names the sides that are not clean, and never promotes it into tracked history. The tree id is what makes each side internally consistent even though neither may have a commit. Under `revisions` this is the comparison of an uncommitted change against HEAD, or against another revision, that the one-off and Arrow local stories want (UC-03 §10). What `revisions` still requires is that every side can be identified: a side whose dirty state is `unknown` has no tree id (§4.1), so it cannot be told apart from the other side or checked for a change during the run, and it fails the invariant that each side has exactly one tree id, so its units get no verdict. -A benchmark is **recorded separately** from the subject when its suite lives outside the subject's checkout (`runner-schema.md` §3.3). Its `benchmark_tree` is then the suite file's own id and differs from the `subject_tree`; this includes a suite in another checkout of the same repository, which has the same source URI but its own file id. A benchmark inside the subject's checkout has `benchmark_tree` equal to `subject_tree` and the subject's own source and revision as `benchmark.source` and `benchmark.revision`; a result whose two trees are equal but whose `benchmark.source` differs from its `source` is malformed. Under `revisions` a separately recorded benchmark must be one file on both sides and in every round (invariant `one-benchmark-tree`): UC-03 requires the sides to agree on everything under code identity, and an edit or commit of the suite during the run is otherwise invisible, because the identity policy maps exact benchmark revisions to declared versions (§4.1) and `benchmark_dirty` only compares the file with its own HEAD. Benchmarks inside the subject's checkout differ between the sides exactly as the subject trees do and are not checked. A result without a `benchmark_tree`, such as a thin result, is outside the rule. +A benchmark is **recorded separately** from the subject when its suite lives outside the subject's checkout (`runner-schema.md` §3.3). Its `benchmark_tree` is then the suite file's own id and differs from the `subject_tree`; this includes a suite in another checkout of the same repository, which has the same source URI but its own file id. A benchmark inside the subject's checkout has `benchmark_tree` equal to `subject_tree` and the subject's own source and revision as `benchmark.source` and `benchmark.revision`; a result whose two trees are equal but whose `benchmark.source` differs from its `source` is malformed. Under `revisions` and `variants` a separately recorded benchmark must be one file on both sides and in every round (invariant `one-benchmark-tree`): the suite is part of the code identity, which UC-03 requires the sides to agree on and UC-02 holds controlled (UC-02 §5), and an edit or commit of the suite during the run is otherwise invisible, because the identity policy maps exact benchmark revisions to declared versions (§4.1) and `benchmark_dirty` only compares the file with its own HEAD. Benchmarks inside the subject's checkout differ between the sides exactly as the subject trees do under `revisions` and are not checked; under `variants` an edit of one changes the subject tree, which the sides must share (`sides-agree`). A result without a `benchmark_tree`, such as a thin result, is outside the rule. Thin results (§4.1) may claim `single-run`, `variants`, and `revisions` within their own run; the other profiles require a full environment identity. diff --git a/docs/design/comparator.md b/docs/design/comparator.md index 2d3ea27..9da057c 100644 --- a/docs/design/comparator.md +++ b/docs/design/comparator.md @@ -111,6 +111,15 @@ Before computing a verdict, the comparator checks, in both modes, that: - the evidence is enough for the configured method, or the method declares a provisional rule that applies. +In run mode the status rule belongs to the profile. Under `variants` it is +strict (UC-02 §10): a unit with an attempt of status `partial`, `censored` or +`error`, on either side, is indeterminate with reason `side-failed`, whatever +the other rounds show. Otherwise a variant that times out in the slow rounds is +judged on the fast ones. The other profiles drop the failed attempt's pair and +keep the verdict from the rest, and a `skipped` attempt is dropped in every +profile. The schema's §5.5 table is not touched, since this is a status rule, +not an invariant. + In history mode it additionally checks that: - the target and every reference point have the same series fingerprint; @@ -124,9 +133,9 @@ In history mode it additionally checks that: In run mode it additionally checks the profile's invariants over the run (schema §5.5): the two sides differ only in the varying coordinate, attempt counts per side are equal, and sides alternate by `procedure.slot`; under `revisions`, -that each side has exactly one tree id and the two differ, and that a -separately recorded benchmark has one `benchmark_tree` across both sides and -all rounds (`one-benchmark-tree`, schema §5.5). A side that is not clean does +that each side has exactly one tree id and the two differ; under `revisions` +and `variants`, that a separately recorded benchmark has one `benchmark_tree` +across both sides and all rounds (`one-benchmark-tree`, schema §5.5). A side that is not clean does not fail the run; it makes the comparison local-only (UC-03 §10). A run that fails an invariant is not compared: the comparison document records the failed invariant and contains no verdicts for the units it affects. diff --git a/docs/design/prototype-design.md b/docs/design/prototype-design.md index 8c5af2f..04e267b 100644 --- a/docs/design/prototype-design.md +++ b/docs/design/prototype-design.md @@ -20,8 +20,8 @@ bx ingest # sweep documents into the store bx series [--workload … --quantity …] # list series bx history # one series' points (a query) bx head [-n 10] [--project P] [--workload W] [--json|--table] # the latest results, newest first -bx compare --run KEY --profile revisions|environments --baseline VALUE - [--label NAME] [--results DIR] [--k 3] [--min-rounds 3] [--json] +bx compare --run KEY --profile revisions|environments|variants --baseline VALUE + [--label NAME] [--vary NAME] [--results DIR] [--k 3] [--min-rounds 3] [--json] # read mode: compare results already measured bx target describe|prepare TARGET # a target's description (a provider's answer, or a build directory's sidecar) bx compare BASELINE CONTENDER --profile revisions|environments @@ -35,6 +35,9 @@ bx compare BASELINE CONTENDER --profile revisions|environments [--run-key KEY] [--out DIR] [--no-build] [--source-uri URI] [--k 3] [--min-rounds 3] [--json] # measure mode: run both sides, then compare +bx compare TARGET --profile variants [--vary NAME] [--baseline VALUE] + --suite NAME --rounds R [--filter REGEX] [...] # the options above, except --label + # measure mode, one target: alternate the two values of a benchmark parameter bx bench TARGET --suite NAME [CASE …] # the single-target form of bx compare: one order, a table [--adapter …] [--filter REGEX] [--quantity NAMES] [-e NAME=VALUE ...] [precision flags, as for bx compare] @@ -70,6 +73,15 @@ when a slot is set, so two sides would otherwise share a key). `bx compare --run environments --label NAME --baseline VALUE` then compares the sides, and refuses a run whose sides do not alternate, have unequal attempt counts, or do not differ. +**The same for variants (UC-02).** The sides of a `variants` comparison are two values of one +benchmark parameter, so there is no label: each call selects one value, by a case name +(`bx bench mod.py "bench_truncate('baseline')"`) or by `--filter` (`--filter "'baseline'"`, which +selects that value's cases of every size), with the shared `--run-key` and its own `--round` and +`--slot`. `bx compare --run KEY --profile variants --baseline "'baseline'" [--vary NAME]` then compares +them, and refuses a run whose sides do not alternate. One `bx bench` call that runs both values has no +round or slot, so it fails `alternating-slots`; the failure names this loop and measure mode as the ways +to get an alternating run. Measure mode (§5) runs the loop in one command. + What differs between the environments is recorded by the user's own commands, so benchx knows no environment kind: a context collector (§3) prints facts about the environment it runs in, which land under `environment.system.`, a part of the varying coordinate; a setup file does the same for @@ -124,10 +136,12 @@ measured tree they sit in, so a run's own output does not mark it dirty. The ear `bx compare` has two modes, chosen by its arguments. Two positional targets (a path read by the harness, or `@CONFIG[=SOURCE_REF]`, a call to the project's -target provider) mean measure mode; `--run` means read mode; giving +target provider; one target for `variants`, whose sides are two values of one +parameter) mean measure mode; `--run` means read mode; giving both or neither is an error, as is a flag that belongs to the other mode (`--baseline` and `--results` are read mode only, since measure mode derives -the baseline from its first target; `--suite`, `--rounds`, `--project`, +the baseline from its first target, except that `variants` takes `--baseline` +in measure mode too, to pick the baseline value; `--suite`, `--rounds`, `--project`, `--run-key`, `--out`, `--repetitions`, `--min-time`, `--no-build`, and `--source-uri` are measure mode only). `--label` names the label in read mode, and carries the two values in measure mode. §5 describes measure mode. @@ -492,6 +506,7 @@ every fact the policy matches must agree across both sides. |---|---|---|---| | `revisions` | subject tree id | a revision or tree id prefix | everything the identity policy matches | | `environments` | the label `--label` gives, then subject configuration, pinned components, and `environment.system` | the label value | everything else the identity policy matches, and subject revision and tree | +| `variants` | the workload parameter `--vary` names, else the only one | that parameter's recorded value, quotes included (`'baseline'`) | everything else the identity policy matches, including subject revision and tree, and every other parameter | A side is one whole axis value, so one label covering two builds is three sides, not one. Before any verdict, the eligibility guard (§3) checks that the @@ -502,10 +517,22 @@ label, equal attempt counts per side, and sides alternating by verdict; under `revisions` the same happens to a side with no tree id (the `sides` invariant), to a tree edited during the run (a third side: `two-sides`), and to a benchmark recorded apart from the subject that is not one file on -both sides and in every round (`one-benchmark-tree`, schema §5.5). A side that +both sides and in every round (`one-benchmark-tree`, schema §5.5), as it does +under `variants`. A side that is not clean does not fail the run: it makes the comparison local-only. A variant present on one side only is listed as missing (criterion 4). Results with a status other than `success` are excluded and -listed. +listed. Under `variants` a failed attempt (`partial`, `censored` or `error`) on either side also makes +its unit indeterminate with reason `side-failed`, and no effect is reported: a variant that +times out in the slow rounds is not judged on the fast ones (UC-02 §10). A `skipped` attempt is +only excluded, as under every profile. + +**What a `variants` comparison varies.** One workload parameter, so a side is one value of it +(schema §5.5 allows several; the prototype compares two sides). It is the `--vary` name, else the +only workload parameter the results carry; an `-e` variable counts, since the runner records it as +one, and with several parameters `--vary` is required. The values are recorded as `repr()` strings, +so `--baseline` is spelled as recorded, quotes included. A unit is named by what both sides share: +the other parameters stay in its workload, so every combination of them is its own unit +(`bench_truncate [size=1000]`), and the parameter itself is not part of the name. Per comparison unit (one workload variant and quantity), the method `benchx/paired-relative/v1`: @@ -519,8 +546,8 @@ Per comparison unit (one workload variant and quantity), the method direction when known; otherwise *no change detected*; - is indeterminate with a reason when fewer than `min_rounds` pairs exist, and never with fewer than two, since the noise is a standard error - (`insufficient-rounds`), when a baseline value is zero, or when the guard - excluded the unit. + (`insufficient-rounds`), when a baseline value is zero, when the guard + excluded the unit, or, under `variants`, when an attempt failed (`side-failed`). Deterministic quantities are deferred: none of the implementation's quantities is one, so a unit whose quantity is declared deterministic is indeterminate with @@ -553,6 +580,20 @@ compares them; it is the workbench's session loop (`system-decomposition.md` derived from the first target: its subject tree id for `revisions`, and the first value of `--label NAME=BASE,CONTENDER` for `environments`. +**Measure mode for variants.** `bx compare TARGET --profile variants --suite NAME --rounds R` has one +target and no label; `session.compare_variants` is its loop. It resolves the side once, with the +refusal above for a tree with no working-tree id, and lists the cases once, with `--filter`, as the +runner will. It takes the parameter (`--vary`, else the only one the selected benchmarks and the `-e` +variables give; several are refused, naming who declares each), reads the two values the selected cases +take, and splits the cases by value. Each round writes two orders, the baseline's value first at slot +2r and the other at slot 2r + 1, and an order's filter selects exactly its value's cases, by the text +`name(p0, p1)` the runner searches the filter in. The baseline is `--baseline`, else the first +declared value. Benchmarks without the parameter run on neither side, and the session says which. +Refused before anything runs: no parameter; benchmarks that do not take the same two values (a +`--filter` can pick two of three); a `--baseline` that is not one of them, with the values as they +must be typed; and Google Benchmark, whose benchmarks have no parameters. The comparison is +local-only when the tree is not clean, as above. + It is exactly `bx run` 2R times followed by read-mode `bx compare`; it adds no document, schema, or contract. It builds nothing itself: both targets must exist when it looks. A target given as `@CONFIG` is first requested from the diff --git a/docs/design/prototype-scope.md b/docs/design/prototype-scope.md index 14b9750..bb8357b 100644 --- a/docs/design/prototype-scope.md +++ b/docs/design/prototype-scope.md @@ -36,6 +36,11 @@ one run: with one build option changed, hardened against plain. The sides differ in subject configuration and are named by a label. +A third comparison, **variants** (UC-02), has one target: a pyperf or asv +benchmark whose function takes a parameter, and the sides are two values of it, +alternated in one run. A comparison varies one parameter, so its two sides are +one baseline value and one contender value. + ## 2. Component scope in the implementation | Component | In implementation | Deferred | Out of scope | @@ -43,8 +48,8 @@ one run: | Runner | execute a `build` or `working_tree` target per a work order; apply the order's `environment_variables` to the launched process; capture the zero-configuration snapshot, the build configuration, and source identity; account for planned cases; refuse an order it cannot apply exactly, including one that names a deferred field | `python_env` targets, context collectors that run after the harness or per case (the implementation runs them once per order, before the cases), environment policy (enforcing launch settings, verifying requested hardware, refusing an unsatisfiable run) | building revisions, tuning the machine (`benchmark-environments.md` §2.3) | | Adapter | Google Benchmark, both halves: drive filter and repetitions, translate output; pyperf, both halves, for Python functions in a working tree, through `bx bench` and `bx compare` (the default adapter of both) | all other harnesses; pyperf's `tags`, `date`, per-run load and frequency, and `pyperf check` | — | | Store | one Parquet file; ingest per schema §5.4 (validated and durable, auto-create, unit-conflict rejection, idempotency, attempt integrity); one default identity policy; queries: list series, series history, get result | continuity events, quarantine, artifact storage, identity-policy authoring | — | -| Comparator | run mode with the `revisions` and `environments` profiles; eligibility guard; comparison document | history mode; `single-run`, `variants`, and `cross-machine` profiles; batch requests; provisional noise rules | — | -| Workbench session loop | the project file `benchx.conf.json` (one file, in the project root: provider, project, harness and suite, quantities, precision, environment variables, timeouts, paths, comparator policy; flags over file over defaults); `bx bench` (one order) and `bx compare` in measure mode: writes one work order per side and round, alternates them, delivers the results, then compares; takes each side as a build directory or as `@CONFIG`, a call to the project's target provider (`describe` or `prepare`), whose description may be a build or a working tree; either harness, with the same `-e` variables on both sides; a bare path is a build directory for Google Benchmark and a working tree for pyperf; `bx bench` is the same path for one order; `bx target` asks for a description directly; copies the description's declared text (`how_built`, `activation`, `shell`, `provider`) into the order | provider-built revisions (a source ref other than `WORKSPACE` is the provider's business), an `artifact` target, a declared `build` | workbench packaging, front ends such as `spin bench` | +| Comparator | run mode with the `revisions`, `environments` and `variants` profiles (a `variants` comparison varies one workload parameter: its sides are two values of it); eligibility guard; comparison document | history mode; `single-run` and `cross-machine` profiles; a `variants` comparison of several parameters at once; batch requests; provisional noise rules | — | +| Workbench session loop | the project file `benchx.conf.json` (one file, in the project root: provider, project, harness and suite, quantities, precision, environment variables, timeouts, paths, comparator policy; flags over file over defaults); `bx bench` (one order) and `bx compare` in measure mode: writes one work order per side and round, alternates them, delivers the results, then compares; takes each side as a build directory or as `@CONFIG`, a call to the project's target provider (`describe` or `prepare`), whose description may be a build or a working tree; either harness, with the same `-e` variables on both sides; a bare path is a build directory for Google Benchmark and a working tree for pyperf; `bx bench` is the same path for one order; `bx compare TARGET --profile variants` is the one-target form for pyperf and asv, which alternates the cases of two values of one benchmark parameter; `bx target` asks for a description directly; copies the description's declared text (`how_built`, `activation`, `shell`, `provider`) into the order | provider-built revisions (a source ref other than `WORKSPACE` is the provider's business), an `artifact` target, a declared `build` | workbench packaging, front ends such as `spin bench` | | Documents | work order (`schemas/work-order/0.1.0`), result (schema §5.2), comparison document | — | — | **Out entirely:** scheduler, change detector, alerting, views beyond query From e88afbf258fa0a621b4f4ba6a1b9e63fbcaefc02 Mon Sep 17 00:00:00 2001 From: ev-br's Claude Code Agent Date: Mon, 2 Nov 2026 18:33:27 +0000 Subject: [PATCH 9/9] 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 --- src/benchx/compare.py | 13 +++++++++++-- tests/test_comparator.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/benchx/compare.py b/src/benchx/compare.py index 4b18f32..39b2b98 100644 --- a/src/benchx/compare.py +++ b/src/benchx/compare.py @@ -16,7 +16,7 @@ from . import identity from .core import canonical, catalog -from .projection import Projection, get, workload_label +from .projection import Projection, axis_order, get, workload_label from .snapshot import DIRTY_WARNINGS from .store import store_project @@ -242,7 +242,8 @@ def compare(documents, *, run_key, profile, baseline, label=None, parameter=None return doc for (_, quantity), unit in sorted( - units.items(), key=lambda item: (_c(_workload(item[1]["docs"], projection.axis)), item[0][1])): + units.items(), + key=lambda item: _unit_order(_workload(item[1]["docs"], projection.axis), item[0][1])): by_side = unit["docs"] where = {"workload": _workload(by_side, projection.axis), "quantity": quantity} absent = [role for role, s in (("baseline", base), ("contender", contender)) if not by_side[s]] @@ -319,6 +320,14 @@ def _workload(by_side, axis=()) -> dict: return {"name": workload["name"], **({"parameters": parameters} if parameters else {})} +def _unit_order(workload: dict, quantity: str) -> tuple: + """The order units are listed in: by workload name, then by parameter name and value, then by + quantity, with numbers inside names and values in natural order (`axis_order`), so that `n=50` + comes before `n=100` and `bench_2` before `bench_10`, as people read them.""" + parameters = sorted((workload.get("parameters") or {}).items()) + return (axis_order(workload["name"]), tuple((name, axis_order(value)) for name, value in parameters), quantity) + + _ABSENT = object() diff --git a/tests/test_comparator.py b/tests/test_comparator.py index 9ec6b78..d5dcbea 100644 --- a/tests/test_comparator.py +++ b/tests/test_comparator.py @@ -582,3 +582,40 @@ def test_infer_parameter_reads_only_the_run_and_only_measurands(): assert compare.infer_parameter([*docs, other], "run") == "role" with pytest.raises(compare.CompareError, match="no results for run"): compare.infer_parameter(docs, "missing") + + +# --- units are listed in natural order ---------------------------------------------------------- + +def test_units_are_listed_with_numbers_in_natural_order(): + """`n=50` before `n=100`, as people read sizes, though "100" sorts before "50" as text.""" + doc = vevaluate(variants_run(sizes=("100", "10", "50", "1000"))) + assert invariants(doc) == [] + assert [u["workload"]["parameters"]["size"] for u in doc["units"]] == ["10", "50", "100", "1000"] + shown = [line.split()[1] for line in compare.render(doc).splitlines() if "bench_truncate [" in line] + assert shown == ["[size=10]", "[size=50]", "[size=100]", "[size=1000]"] + + +def test_units_are_listed_with_numbers_in_workload_names_in_natural_order(): + docs = [] + for name in ("BM_W10", "BM_W2", "BM_W1"): + slot = 0 + for r in range(3): + for side, scale in ((BASE, 1.0), (HEAD, 1.1)): + docs.append(result(side, r, slot, (r + 1) * scale, workload=name)) + slot += 1 + assert [u["workload"]["name"] for u in evaluate(docs)["units"]] == ["BM_W1", "BM_W2", "BM_W10"] + + +def test_units_with_more_than_one_other_parameter_are_ordered_by_name_then_value(): + docs = variants_run() + extra = [] + for n in ("10", "2"): + for size in ("100", "20"): + for d in docs: + clone = copy.deepcopy(d) + clone["ingest_key"] += f":{n}:{size}" + clone["coordinates"]["workload"]["parameters"].update(n=n, size=size) + extra.append(clone) + doc = vevaluate(extra) + assert [(u["workload"]["parameters"]["n"], u["workload"]["parameters"]["size"]) for u in doc["units"]] == [ + ("2", "20"), ("2", "100"), ("10", "20"), ("10", "100")]