From 6b9c5dde179303bff7f7998f9a5f634fa648ab4f Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 06:19:35 -0700 Subject: [PATCH 01/11] chore: add a development harness for agents working on Tsim - `tsim-dev` CLI (dev/, installed by `uv sync`): `check`, `test --changed`, `bench --ref` (interleaved fresh-process A/B, pinned single thread, warm numba cache, round-by-round verdicts, tsim-bench corpus), `inspect`, `profile` (py-spy, split by phase, time charged to kernel callers), and `run`/`job` under systemd memory, CPU and time caps. Short output, logs in .tsim-dev/, exit codes 0 pass, 1 failed or slower, 2 harness error, 3 running. - Benchmark registry of eight circuits in dev/circuits/. - Developer skills in .claude/skills (also .agents/skills): shipping changes, benchmarking, profiling, correctness testing. Not published to users. - .claude/settings.json: a Bash hook that blocks `pkill -f` and uncapped background Python; read-only git/gh allowed, push/PR/merge ask. - AGENTS.md points to the CLI and skills; heavy-job and PR rules updated. - Fix test_the_calling_thread_keeps_parallel_kernels_after_run_tasks on TBB hosts: TBB runs a 4096-element prange on one thread. Co-Authored-By: Claude Opus 5.5 --- .agents/skills | 1 + .claude/hooks/guard_bash.py | 50 +++ .claude/settings.json | 35 ++ .claude/skills/benchmarking-tsim/SKILL.md | 80 ++++ .claude/skills/profiling-tsim/SKILL.md | 62 ++++ .claude/skills/shipping-tsim-changes/SKILL.md | 53 +++ .../skills/testing-tsim-correctness/SKILL.md | 52 +++ .gitignore | 1 + AGENTS.md | 72 ++-- dev/circuits/circuits.toml | 57 +++ dev/pyproject.toml | 16 + dev/tsim_dev/__init__.py | 1 + dev/tsim_dev/_worker.py | 153 ++++++++ dev/tsim_dev/bench.py | 351 ++++++++++++++++++ dev/tsim_dev/checks.py | 161 ++++++++ dev/tsim_dev/cli.py | 141 +++++++ dev/tsim_dev/common.py | 186 ++++++++++ dev/tsim_dev/jobs.py | 195 ++++++++++ dev/tsim_dev/profile.py | 169 +++++++++ pyproject.toml | 11 +- test/unit/test_dev_cli.py | 138 +++++++ test/unit/utils/test_kernels.py | 4 +- uv.lock | 7 + 23 files changed, 1968 insertions(+), 28 deletions(-) create mode 120000 .agents/skills create mode 100755 .claude/hooks/guard_bash.py create mode 100644 .claude/settings.json create mode 100644 .claude/skills/benchmarking-tsim/SKILL.md create mode 100644 .claude/skills/profiling-tsim/SKILL.md create mode 100644 .claude/skills/shipping-tsim-changes/SKILL.md create mode 100644 .claude/skills/testing-tsim-correctness/SKILL.md create mode 100644 dev/circuits/circuits.toml create mode 100644 dev/pyproject.toml create mode 100644 dev/tsim_dev/__init__.py create mode 100644 dev/tsim_dev/_worker.py create mode 100644 dev/tsim_dev/bench.py create mode 100644 dev/tsim_dev/checks.py create mode 100644 dev/tsim_dev/cli.py create mode 100644 dev/tsim_dev/common.py create mode 100644 dev/tsim_dev/jobs.py create mode 100644 dev/tsim_dev/profile.py create mode 100644 test/unit/test_dev_cli.py diff --git a/.agents/skills b/.agents/skills new file mode 120000 index 00000000..454b8427 --- /dev/null +++ b/.agents/skills @@ -0,0 +1 @@ +../.claude/skills \ No newline at end of file diff --git a/.claude/hooks/guard_bash.py b/.claude/hooks/guard_bash.py new file mode 100755 index 00000000..8160a1c2 --- /dev/null +++ b/.claude/hooks/guard_bash.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python3 +"""PreToolUse hook for Bash: block two commands that have taken sessions down. + +Exit code 2 blocks the command and shows stderr to the agent. Everything else +passes. Keep this list short: it is for rules that must hold every time. +""" + +import json +import re +import sys + +COMPUTE = re.compile(r"\b(python3?|pytest|uv run)\b") +CAPPED = re.compile(r"\b(tsim-dev|systemd-run)\b") +DETACHED = re.compile(r"\bnohup\b|\bsetsid\b|&\s*(disown\b|$)", re.MULTILINE) + + +def verdict(tool_input: dict) -> str | None: + """Return why the command is blocked, or None to let it run.""" + command = tool_input.get("command", "") + if re.search(r"\bpkill\b[^|;&\n]*\s-\w*f", command): + return ( + "`pkill -f` also matches this shell's own command line and kills it. " + "Stop a job with `uv run tsim-dev job stop NAME`, or kill by PID." + ) + background = tool_input.get("run_in_background") or DETACHED.search(command) + if background and COMPUTE.search(command) and not CAPPED.search(command): + return ( + "Background compute needs a hard memory cap; uncapped jobs have used " + "55 GB and killed every session on this machine. Start it with " + "`uv run tsim-dev job start NAME -- ` and follow it with " + "`uv run tsim-dev job wait NAME`." + ) + return None + + +def main() -> int: + """Read the hook payload from stdin and decide.""" + try: + payload = json.load(sys.stdin) + except ValueError: + return 0 + reason = verdict(payload.get("tool_input") or {}) + if reason: + print(reason, file=sys.stderr) + return 2 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..5ff4d776 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,35 @@ +{ + "permissions": { + "allow": [ + "Bash(uv run tsim-dev *)", + "Bash(uv sync)", + "Bash(git status *)", + "Bash(git diff *)", + "Bash(git log *)", + "Bash(git show *)", + "Bash(gh pr view *)", + "Bash(gh pr diff *)", + "Bash(gh pr checks *)", + "Bash(gh issue view *)", + "Bash(gh run view *)" + ], + "ask": [ + "Bash(git push *)", + "Bash(gh pr create *)", + "Bash(gh pr merge *)" + ] + }, + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "python3 \"$CLAUDE_PROJECT_DIR/.claude/hooks/guard_bash.py\"" + } + ] + } + ] + } +} diff --git a/.claude/skills/benchmarking-tsim/SKILL.md b/.claude/skills/benchmarking-tsim/SKILL.md new file mode 100644 index 00000000..6e28f42d --- /dev/null +++ b/.claude/skills/benchmarking-tsim/SKILL.md @@ -0,0 +1,80 @@ +--- +name: benchmarking-tsim +description: Measures Tsim throughput and compile time with `tsim-dev bench` and compares a change against a reference commit or checkout. Use when asked to benchmark Tsim, to check a change for performance regressions, to produce before and after numbers for a pull request, or to run the tsim-bench circuit corpus. +--- + +# Benchmarking Tsim + +Use `uv run tsim-dev bench`. Do not write timing scripts: hand-written ones have +measured the wrong thing often enough (compile time included, a cold numba cache, +several threads, a busy machine) that their numbers are not accepted in reviews. + +## Check a change against main + +```bash +git fetch origin main +uv run tsim-dev bench --ref origin/main # quick suite, about 4 minutes +``` + +Use `origin/main`, not `main`: the local branch is often behind. `--ref` also takes +any commit, or the path of another checkout that has a `.venv` +(`--ref ../tsim-other-worktree`). + +``` +circuit k shots/s compile shots/s vs ref (min, max) compile vs ref +cultivation_d5 6 13.9M 0.13 1.20x [1.19, 1.21] faster 0.96x same +``` + +- `shots/s vs ref` is new over reference, so above 1 is faster. The bracket is the + smallest and largest ratio over the rounds. +- A verdict of `faster` or `slower` needs the mean outside the tolerance (5%) and + every round on the same side of 1. `noisy` means the rounds disagree: run again + with `--rounds 9 --min-seconds 1` before you conclude anything. +- Exit code 0: nothing slower. 1: at least one `slower` in rate or compile time. + 2: a circuit could not be measured. +- The JSON under `.tsim-dev/bench/` has every round. + +## What the command controls for you + +- One thread, pinned to a core (`--threads N` to change; say so when you report). +- Each measurement is a fresh process with a warm numba cache, under a memory cap. + Kernels loaded from the cache have behaved differently from freshly compiled + ones, and the cached case is what users get. +- Compile time and the first call are excluded from shots/s and reported apart. +- The two checkouts alternate round by round, so drift affects both. +- Each registry circuit fixes its sampler and postselection + (`dev/circuits/circuits.toml`). With postselection, shots/s counts attempted shots. + +## What you must do + +- Run one benchmark at a time, and nothing else heavy (tests, other agents' jobs) + next to it. Heed the busy-machine warning. +- A change must hold across families. Run `--suite full` before claiming a speedup, + and report the circuits that got slower as well as the ones that got faster. +- For a single circuit: `uv run tsim-dev bench cultivation_d5 --ref origin/main`, or + a path to any `.stim` file with `--sampler` and `--postselect`. +- Paste the table in the pull request, with the CPU model (`lscpu | grep 'Model name'`) + and the thread count. + +## The tsim-bench corpus + +189 circuits from 27 papers (https://github.com/rafaelha/tsim-bench). It takes much +longer, so run it as a job: + +```bash +uv run tsim-dev corpus fetch # once; or set TSIM_BENCH_DIR to a clone +uv run tsim-dev job start corpus --timeout 14400 -- \ + uv run tsim-dev bench --corpus --max-k 12 --ref origin/main +uv run tsim-dev job wait corpus # repeat until it reports the end +``` + +`--only SUBSTRING` selects circuits by name; `--max-k` skips circuits whose recorded +active width is larger. + +## Limits + +- CPU only. GPU runs and comparisons with other simulators (Clifft, SymFT, Magna) + are run from the tsim-bench repository, which pins each simulator's environment. +- To add a circuit to the registry, put the file in `dev/circuits/`, add it to + `circuits.toml` and `git add -f` it (`*.stim` is ignored). Keep the quick suite + to one small circuit per family. diff --git a/.claude/skills/profiling-tsim/SKILL.md b/.claude/skills/profiling-tsim/SKILL.md new file mode 100644 index 00000000..886c0778 --- /dev/null +++ b/.claude/skills/profiling-tsim/SKILL.md @@ -0,0 +1,62 @@ +--- +name: profiling-tsim +description: Finds where Tsim spends compile and sampling time with `tsim-dev profile` and `tsim-dev inspect`, and turns a profile into an optimization plan. Use when asked why a circuit is slow, where time goes, what to optimize next, or before starting performance work on Tsim. +--- + +# Profiling Tsim + +## 1. Size the circuit first + +```bash +uv run tsim-dev inspect cultivation_d5 # or a path to any .stim file +``` + +It prints qubits, T count, detectors, `active_width` (k) and `SamplerStats` without +sampling. Cost per shot grows like 2^k. Do not sample or profile above k ≈ 20 without +a smaller `--batch-size` and a memory estimate (2^k × 16 B × batch). + +## 2. Profile one run + +```bash +uv run tsim-dev profile cultivation_d5 --seconds 5 +``` + +The run is a fresh process, one pinned thread, warm numba cache, under a memory cap. +The output splits samples by phase (`compile`, `sample`) and prints three tables: + +- **innermost Tsim frame (kernel callers):** read this first. Jitted numba code has + no symbols, so time inside a kernel is charged to the Python function that called + it. On cultivation_d5 at 1.0.0 this shows 82% in `sample_words` + (`noise/channels.py`) and 14% in `_run_chunk` (`statevector/shots.py`): noise + sampling, not the state vector, is the cost there. +- **innermost frame:** separates `[jitted numba code]` from numpy, libc (memcpy, + malloc) and Python overhead. A large non-jitted share means work outside kernels. +- **Tsim frames, inclusive:** the call tree, for compile time in particular. + +The folded stacks are saved for flame graphs (`speedscope ` or +`flamegraph.pl`). + +Profile at least two families before choosing a target; the hot spot differs +between circuits with mostly direct detectors (cultivation, surface codes), wide +quantum components (quantum volume) and many rotations (coherent noise). + +## 3. Inside a kernel + +The profile stops at the kernel boundary. To compare variants of a kernel, change +it and use `uv run tsim-dev bench --ref origin/main`: that is the +measurement that counts. Micro-benchmarks of a kernel alone have misled before +(cache effects, struct passing by value), so confirm any gain end to end. + +Hardware counters (`perf stat`) need `/proc/sys/kernel/perf_event_paranoid` at 1 or +lower; ask the user rather than changing it. For GPU work, use Nsight Systems and +Nsight Compute 2026 or newer; older `ncu` releases do not support the RTX 5090. + +## 4. Turn it into a plan + +- Write down the target (phase, function, share of time) and the circuits it + affects, before changing code. +- Estimate the ceiling: removing a function that takes 14% gives at most 1.16x. +- Prefer one general change over special cases for particular circuits, and drop + ideas whose gain is small compared with the code they add. +- After the change, profile again to confirm the share moved, then follow the + `benchmarking-tsim` skill for the numbers that go in the pull request. diff --git a/.claude/skills/shipping-tsim-changes/SKILL.md b/.claude/skills/shipping-tsim-changes/SKILL.md new file mode 100644 index 00000000..769ddfeb --- /dev/null +++ b/.claude/skills/shipping-tsim-changes/SKILL.md @@ -0,0 +1,53 @@ +--- +name: shipping-tsim-changes +description: Takes a change to Tsim's code from a new branch to a reviewed local commit, with the checks, evidence and git rules this repository uses. Use when starting a feature, bug fix or optimization in the Tsim repository, when asked to open or update a pull request, or before saying that a change is done. +--- + +# Shipping a change to Tsim + +## 1. Start + +- Work in a worktree on a new branch off the newest main, not in the main checkout: + `git fetch origin main && git worktree add -b / ../tsim- origin/main`, + then `uv sync` there. Stack on another branch only when the change needs it. +- Agree on what "done" means before writing code: the behavior, the test that shows + it, and for performance work the circuits and the size of the gain that matters. + +## 2. Build + +- For a bug, write the failing test first and watch it fail. +- `uv run tsim-dev test --changed` while iterating. +- Anything long or memory-hungry goes through `uv run tsim-dev run -- ` or + `uv run tsim-dev job start NAME -- `; see AGENTS.md. + +## 3. Verify + +Run these and keep their output. A claim without the command and its result is not +evidence. + +| Change | Required | +|---|---| +| Any | `uv run tsim-dev check` prints PASS for lint and tests | +| Touches `src/tsim` outside docstrings | `uv run tsim-dev bench --ref origin/main` exits 0 | +| Performance | the same with `--suite full`; see the `benchmarking-tsim` skill | +| Bug fix | the new test fails on `origin/main` and passes on the branch | +| User-visible | a `CHANGELOG.md` entry under `## [Unreleased]` | +| Behavior the user skills describe | `skills/` updated; `test/unit/test_skills.py` passes | + +Then read your own diff (`git diff origin/main --stat`, then the diff). Remove +anything the task did not need: special cases for one benchmark, leftover debug +code, unrelated reformatting. Ask whether a simpler change would do. + +## 4. Commit and review + +- Commit locally with a Conventional Commits message. +- Do not push, open a pull request, force-push or merge unless the user asked for + that step in this conversation. Some branches are not meant to be public yet. +- For a review, give a fresh subagent the diff and the task, not your conclusions, + and ask for correctness findings only. Fix what it finds, then run step 3 again. + +## 5. Report + +Say in a few sentences what changed, the verification commands you ran with their +results, and what you did not verify. For performance work include the bench table, +the CPU model and the thread count. Say plainly if something failed or was skipped. diff --git a/.claude/skills/testing-tsim-correctness/SKILL.md b/.claude/skills/testing-tsim-correctness/SKILL.md new file mode 100644 index 00000000..685e9ba3 --- /dev/null +++ b/.claude/skills/testing-tsim-correctness/SKILL.md @@ -0,0 +1,52 @@ +--- +name: testing-tsim-correctness +description: Chooses the oracle and writes statistical tests that show a Tsim change samples the right distribution. Use when adding or changing a gate, noise channel, sampler option, scheduler, plan or kernel in Tsim, when fixing a wrong-results bug, or when asked whether Tsim's output can be trusted for a circuit. +--- + +# Testing Tsim's correctness + +Performance work is only worth anything if the samples stay right. Every change to +`src/tsim` that can change samples needs a test against an independent oracle. + +## Pick the oracle + +| Situation | Oracle | Where | +|---|---|---| +| A component, plan or kernel of `active_statevector` | dense state vector | `test/helpers/statevector.py`: `plan_distribution`, `ops_distribution`, `reference_rotate`, `reference_measure`, `assert_distributions_close` | +| Whole circuit, few qubits, few T gates | `strategy="cat5"`, or `VecSampler` | `test/integration/test_statevector_sampler.py` shows both | +| Clifford circuit or Clifford stand-in | Stim | compile with Stim and Tsim; pass the reference-sample flags to Tsim | +| Single gates | unitaries | `test/helpers/gate_matrices.py`, `test/integration/test_gate_unitaries.py` | +| Random coverage | `gen_stim_circuit` | `test/helpers/gen.py` | + +Exact probabilities beat sampled references: compare samples to an exact +distribution with `assert_matches_distribution`, and two sampled sets with +`assert_counts_match` (both 5σ per outcome). Run `cat5` only on circuits with a +handful of T gates; its memory grows exponentially. + +## Invariants to test when you touch them + +- Same seed gives the same samples for any thread count and any `batch_size`. +- Bit-packed and unpacked outputs agree. Every `sample()` call draws new shots, so + compare two fresh samplers with the same seed, not two calls on one sampler. +- Postselection returns only the accepted rows; compare accepted-row distributions. +- Detectors and observables are raw parities. The reference-sample flags give Stim's + flips; use them only for detectors that are deterministic without noise. +- Direct outputs never enter a state vector (`sampler.stats.num_direct_outputs`). + +## Writing the test + +- For a bug: reproduce it in the smallest circuit you can, write the test, and show + it fails before the fix. Run the new test file against `origin/main` in a + separate worktree; do not `git stash` while other jobs run in this checkout. +- Fix the seed and choose the shot count for the tolerance; a flaky statistical + test is a broken test. +- Stim merges consecutive identical instructions into one with several targets, so + write circuits that exercise both single- and multi-target paths. +- Keep circuits small (a few qubits, tens of thousands of shots) so the suite stays + near 90 s. Long randomized campaigns belong in a job, not in the suite: + `uv run tsim-dev job start fuzz --mem 4G -- `. + +## Before you report + +`uv run tsim-dev check` must pass. Say which oracle you compared against, the +circuits, the shot counts, and anything you could not cover. diff --git a/.gitignore b/.gitignore index 83758178..6a7a990c 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ experiments/ /outputs/ /output/ /tmp/ +/.tsim-dev/ /lean/ /benchmark_instructions.md /clifft_noise_transparent_squeeze.md diff --git a/AGENTS.md b/AGENTS.md index f2944d19..1e629648 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,16 +6,33 @@ user skill in `skills/simulating-circuits-with-tsim/` instead; it also explains ## Commands +`uv sync` installs everything, including the `tsim-dev` CLI (`dev/`). Use it instead of +hand-written timing scripts, `nohup`, sleep loops and bare `pytest`: it caps memory, keeps +output short, writes details to `.tsim-dev/`, and its exit codes mean 0 pass, 1 the code +failed (or got slower), 2 the harness could not run, 3 still running. + ```bash -uv sync # environment, including the dev group -uv run pytest test # full suite, about 90 s; test/unit alone is faster -uv run pre-commit run --all-files # black, isort, ruff, pyright, codespell, deptry, nbstripout -just doc-build # docs build and Mintlify checks (needs `npm i -g mint`) +uv run tsim-dev check # the gate before review: pre-commit, full suite (~90 s) +uv run tsim-dev test --changed # tests of the changed files; or give paths +uv run tsim-dev bench --ref origin/main # quick A/B benchmark, ~4 min; exit 1 if slower +uv run tsim-dev inspect FILE.stim # qubits, T count, active width, stats; no sampling +uv run tsim-dev profile cultivation_d5 # where compile and sampling time goes (py-spy) +uv run tsim-dev run -- CMD # any command under memory, CPU and time caps +uv run tsim-dev job start NAME -- CMD # the same in the background; then `job wait NAME` +just doc-build # docs build and Mintlify checks (needs `npm i -g mint`) ``` -`test/unit/utils/test_kernels.py::test_the_calling_thread_keeps_parallel_kernels_after_run_tasks` -fails on hosts where numba uses the TBB threading layer; that failure does not come from your -change. +`--help` on each subcommand lists the caps and options. + +## Skills for working on Tsim + +`.claude/skills/` (also at `.agents/skills/`) holds procedures for this repository; they +are not published to users. + +- `shipping-tsim-changes`: branch, verify, commit, review; what evidence a change needs. +- `benchmarking-tsim`: A/B benchmarks, reading verdicts, the tsim-bench corpus. +- `profiling-tsim`: sizing a circuit, reading profiles, choosing what to optimize. +- `testing-tsim-correctness`: oracles and statistical tests for sampling changes. ## How Tsim works @@ -71,12 +88,14 @@ more than a handful of T gates: their compile memory grows exponentially. ## Performance -- Benchmark a change against `main` on the same machine, with several circuit families - (cultivation, distillation, coherent-noise surface codes, quantum volume), not one. -- Numba caches compiled kernels on disk (`utils/kernels.py`). Measure with a warm cache in a - fresh process as well as in-process: cached kernels have behaved differently from freshly - compiled ones (passing large structs by value was 5 to 8 times slower when cached). After - changing a jitted function's signature or return type, delete stale `*.nbi`/`*.nbc` files. +- Measure with `tsim-dev bench --ref origin/main`, never with a script of your own. It runs + fresh processes on one pinned thread with a warm numba cache, excludes compile time, and + interleaves the two checkouts. A speedup must hold on `--suite full`, across families. +- Numba caches compiled kernels on disk (`utils/kernels.py`). Cached kernels have behaved + differently from freshly compiled ones (passing large structs by value was 5 to 8 times + slower when cached), which is why the benchmark uses the cache. After changing a jitted + function's signature or return type, delete stale `*.nbi`/`*.nbc` files. +- Run one benchmark at a time and nothing heavy beside it. - Prefer one general solution over special cases for particular benchmarks, and drop optimizations whose gain is small compared with the code they add. @@ -84,14 +103,14 @@ more than a handful of T gates: their compile memory grows exponentially. Agents share this machine with people and other agents. -- Run long or memory-hungry jobs under a hard cap, for example - `systemd-run --user --scope -p MemoryMax=8G -p MemorySwapMax=0 -p CPUQuota=400% timeout 1800 `. - A limit stated only in a prompt is not enough: an uncapped job once used 55 GB and the - out-of-memory killer took down other sessions. -- Check `sampler.active_width` before sampling anything wide, and set `NUMBA_NUM_THREADS` or - `numba.set_num_threads` to the cores you were given. -- Never `pkill -f` or `pgrep -f` a pattern that also appears in your own command line; it - matches your own shell. +- Run anything long or memory-hungry through `tsim-dev run` or `tsim-dev job start` + (defaults: 8 GB, 4 CPUs, no swap). A limit stated only in a prompt is not enough: an + uncapped job once used 55 GB and the out-of-memory killer took down other sessions. A + hook in `.claude/settings.json` blocks uncapped background Python and `pkill -f`. +- Wait for a job with `tsim-dev job wait NAME`, not a sleep loop; stop it with + `tsim-dev job stop NAME`. `pkill -f` matches your own shell and kills it. +- Check the active width (`tsim-dev inspect`) before sampling anything wide; memory per + shot grows like 2^k. ## Style @@ -103,8 +122,11 @@ Agents share this machine with people and other agents. ## Pull requests -- Branch off `main`; use [Conventional Commits](https://www.conventionalcommits.org/). +- Branch off `origin/main` in a worktree; use + [Conventional Commits](https://www.conventionalcommits.org/). +- Commit locally. Push, open, force-push or merge a pull request only when asked. - Add a `CHANGELOG.md` entry under `## [Unreleased]` for any user-visible change. -- Bug fixes include a minimal reproducer as a test; performance changes include before and - after numbers, with the machine, thread count and circuits. -- Run the full test suite and pre-commit before asking for review. +- Bug fixes include a minimal reproducer as a test that fails without the fix; performance + changes include the `tsim-dev bench` table, the CPU model and the thread count. +- `tsim-dev check` must pass before review. Report the commands you ran and their results, + and say what you did not verify. diff --git a/dev/circuits/circuits.toml b/dev/circuits/circuits.toml new file mode 100644 index 00000000..c9390656 --- /dev/null +++ b/dev/circuits/circuits.toml @@ -0,0 +1,57 @@ +# Benchmark circuits for `tsim-dev bench`. One family per row of the quick suite. +# sampler: "detector" or "measurement". postselect: "all" (every detector) or "none". +# suite: "quick" circuits run by default; `--suite full` adds the rest. + +[cultivation_d3] +file = "cultivation_d3.stim" +sampler = "detector" +postselect = "all" +suite = "quick" + +[cultivation_d5] +file = "cultivation_d5.stim" +sampler = "detector" +postselect = "all" +suite = "quick" + +[distillation_d5] +file = "distillation_d5.stim" +sampler = "detector" +postselect = "none" +suite = "quick" + +[coherent_d3_r3] +file = "coherent_d3_r3.stim" +sampler = "detector" +postselect = "none" +suite = "quick" + +[qv_q10] +file = "qv_q10.stim" +sampler = "measurement" +postselect = "none" +suite = "quick" + +[surface_d7_r7] +file = "surface_d7_r7.stim" +sampler = "detector" +postselect = "none" +suite = "quick" + +[distillation_d3] +file = "distillation_d3.stim" +sampler = "detector" +postselect = "all" +suite = "full" + +[coherent_d5_r5] +file = "coherent_d5_r5.stim" +sampler = "detector" +postselect = "none" +suite = "full" + +[cultivation_d5_unselected] +file = "cultivation_d5.stim" +sampler = "detector" +postselect = "none" +suite = "full" diff --git a/dev/pyproject.toml b/dev/pyproject.toml new file mode 100644 index 00000000..28d3ed79 --- /dev/null +++ b/dev/pyproject.toml @@ -0,0 +1,16 @@ +[project] +name = "tsim-dev" +version = "0.1.0" +description = "Development CLI for Tsim: capped jobs, tests, benchmarks and profiles. Not published." +requires-python = ">=3.11" +dependencies = [] + +[project.scripts] +tsim-dev = "tsim_dev.cli:main" + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.wheel] +packages = ["tsim_dev"] diff --git a/dev/tsim_dev/__init__.py b/dev/tsim_dev/__init__.py new file mode 100644 index 00000000..c426236b --- /dev/null +++ b/dev/tsim_dev/__init__.py @@ -0,0 +1 @@ +"""Development CLI for Tsim.""" diff --git a/dev/tsim_dev/_worker.py b/dev/tsim_dev/_worker.py new file mode 100644 index 00000000..4b349b05 --- /dev/null +++ b/dev/tsim_dev/_worker.py @@ -0,0 +1,153 @@ +"""Measure one circuit in a fresh process and print one JSON line. + +This file is run by path with the Python of the checkout under test, which may be +an older Tsim, so it uses only the standard library, numpy and Tsim's public API. +""" + +from __future__ import annotations + +import argparse +import json +import time + + +def phase_import(): + """Import Tsim (a named frame, so profiles can tell the phases apart).""" + import tsim + + return tsim + + +def phase_compile(tsim, args): + """Parse the circuit and compile the sampler.""" + circuit = tsim.Circuit.from_file(args.circuit) + kind = args.sampler + if kind == "auto": + has_outputs = circuit.num_detectors + circuit.num_observables > 0 + kind = "detector" if has_outputs else "measurement" + kwargs = {"seed": args.seed} + if args.strategy: + kwargs["strategy"] = args.strategy + if kind == "detector": + sampler = circuit.compile_detector_sampler(**kwargs) + else: + sampler = circuit.compile_sampler(**kwargs) + return circuit, kind, sampler + + +def make_sample(circuit, kind, sampler, args): + """Return a function that samples a number of shots with the requested options.""" + import numpy as np + + kwargs = {} + if args.batch_size: + kwargs["batch_size"] = args.batch_size + if args.bit_packed: + kwargs["bit_packed"] = True + if args.postselect == "all": + if kind != "detector": + raise ValueError("postselection needs the detector sampler") + kwargs["postselection_mask"] = np.ones(circuit.num_detectors, dtype=np.bool_) + + def sample(shots): + return sampler.sample(shots, **kwargs) + + return sample + + +def phase_sample(sample, shots, seconds): + """Sample repeatedly for about `seconds` (the phase that profiles look at).""" + end = time.perf_counter() + seconds + while time.perf_counter() < end: + sample(shots) + + +def calibrate(sample, min_seconds, max_shots): + """Find a shot count and a number of calls that together take `min_seconds`. + + The shot count doubles until one call is long enough. It stops at `max_shots`, + which bounds the size of the returned array; faster circuits are timed over + several calls instead. + """ + shots = 16 + while True: + start = time.perf_counter() + sample(shots) + elapsed = time.perf_counter() - start + if elapsed >= min_seconds: + return shots, 1 + if shots >= max_shots: + return shots, max(1, round(min_seconds / max(elapsed, 1e-6))) + grow = min(16.0, max(2.0, 1.2 * min_seconds / max(elapsed, 1e-6))) + shots = min(max_shots, int(shots * grow)) + + +def main(): + """Run the requested mode and print `RESULT `.""" + parser = argparse.ArgumentParser() + parser.add_argument("--circuit", required=True) + parser.add_argument("--mode", default="bench") # bench, prime, inspect, profile + parser.add_argument("--sampler", default="auto") + parser.add_argument("--postselect", default="none") + parser.add_argument("--strategy", default="") + parser.add_argument("--seed", type=int, default=1) + parser.add_argument("--batch-size", type=int, default=0) + parser.add_argument("--bit-packed", action="store_true") + parser.add_argument("--min-seconds", type=float, default=0.5) + parser.add_argument("--repeats", type=int, default=3) + args = parser.parse_args() + + start = time.perf_counter() + tsim = phase_import() + imported = time.perf_counter() + circuit, kind, sampler = phase_compile(tsim, args) + compiled = time.perf_counter() + + result = { + "tsim_version": getattr(tsim, "__version__", "?"), + "sampler": kind, + "import_s": imported - start, + "compile_s": compiled - imported, + "active_width": getattr(sampler, "active_width", None), + "qubits": circuit.num_qubits, + "detectors": circuit.num_detectors, + "measurements": circuit.num_measurements, + "tcount": circuit.tcount(), + } + if args.mode == "inspect": + result["stats"] = str(getattr(sampler, "stats", "")) + print("RESULT " + json.dumps(result)) + return + + sample = make_sample(circuit, kind, sampler, args) + warm = time.perf_counter() + sample(16) + result["warmup_s"] = time.perf_counter() - warm + if args.mode == "prime": + print("RESULT " + json.dumps(result)) + return + + outputs = circuit.num_detectors + circuit.num_observables + if kind == "measurement": + outputs = circuit.num_measurements + # NOTE: one call returns at most about 256 MB of unpacked samples. + shots, calls = calibrate(sample, args.min_seconds, (1 << 28) // max(1, outputs)) + if args.mode == "profile": + phase_sample(sample, shots, args.min_seconds * 10) + result["shots"] = shots + print("RESULT " + json.dumps(result)) + return + + times = [] + for _ in range(args.repeats): + start = time.perf_counter() + for _ in range(calls): + sample(shots) + times.append(time.perf_counter() - start) + total = shots * calls + result.update(shots=total, seconds=min(times), shots_per_s=total / min(times)) + print("RESULT " + json.dumps(result)) + + +if __name__ == "__main__": + main() diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py new file mode 100644 index 00000000..a220a083 --- /dev/null +++ b/dev/tsim_dev/bench.py @@ -0,0 +1,351 @@ +"""`tsim-dev bench`: throughput and compile time, optionally against a reference. + +Every measurement runs in a fresh process under a memory cap, pinned to fixed +cores, with numba's on-disk cache warm. With `--ref`, the two checkouts are +measured in interleaved rounds and compared round by round. +""" + +from __future__ import annotations + +import json +import math +import os +import shutil +import statistics +import subprocess +import sys +import tomllib +from dataclasses import dataclass +from pathlib import Path + +from . import common +from .common import FAILED, INFRA, OK, InfraError + + +@dataclass(frozen=True) +class Case: + """One circuit with the sampler options it is measured with.""" + + name: str + path: Path + sampler: str = "auto" + postselect: str = "none" + + +@dataclass(frozen=True) +class Side: + """One checkout under test: a label and the Python that imports its Tsim.""" + + label: str + python: Path + + +def registry() -> dict[str, tuple[Case, str]]: + """Return the named circuits in `dev/circuits/circuits.toml` with their suite.""" + with open(common.CIRCUITS_DIR / "circuits.toml", "rb") as file: + table = tomllib.load(file) + return { + name: ( + Case( + name, + common.CIRCUITS_DIR / entry["file"], + entry.get("sampler", "auto"), + entry.get("postselect", "none"), + ), + entry.get("suite", "full"), + ) + for name, entry in table.items() + } + + +def corpus_dir(explicit: str | None = None) -> Path: + """Return the tsim-bench checkout to read circuits from.""" + for candidate in ( + explicit, + os.environ.get("TSIM_BENCH_DIR"), + common.cache_dir() / "tsim-bench", + ): + if candidate and (Path(candidate).expanduser() / "papers").is_dir(): + return Path(candidate).expanduser() + raise InfraError( + "no tsim-bench corpus: run `tsim-dev corpus fetch`, or set TSIM_BENCH_DIR " + "to a clone of https://github.com/rafaelha/tsim-bench" + ) + + +def corpus_cases(directory: Path, max_k: int) -> list[Case]: + """Return the corpus circuits whose recorded active width is at most `max_k`.""" + cases = [] + for record in sorted(directory.glob("papers/*/k_max.json")): + for entry in json.loads(record.read_text()): + k = entry.get("tsim_k") + path = record.parent / entry["file"] + if isinstance(k, int) and k <= max_k and path.exists(): + cases.append(Case(f"{record.parent.name[:10]}/{path.stem}", path)) + return cases + + +def select(args) -> list[Case]: + """Turn the command line into the list of cases to measure.""" + named = registry() + if args.corpus is not None: + cases = corpus_cases(corpus_dir(args.corpus or None), args.max_k) + elif args.circuits: + cases = [] + for item in args.circuits: + if item in named: + cases.append(named[item][0]) + elif Path(item).is_file(): + path = Path(item).resolve() + cases.append(Case(path.stem, path, args.sampler, args.postselect)) + else: + raise InfraError( + f"unknown circuit {item!r}: give a .stim path or one of " + + ", ".join(named) + ) + else: + cases = [ + case + for case, suite in named.values() + if args.suite == "full" or suite == "quick" + ] + if args.only: + cases = [case for case in cases if args.only in case.name] + if not cases: + raise InfraError("no circuits selected") + return cases + + +def reference(ref: str) -> Side: + """Return the side for a git revision or for the path of another checkout. + + A revision is exported once into the shared cache and gets its own + environment there, so its numba cache is separate from the working tree's. + """ + if Path(ref).is_dir(): + path = Path(ref).resolve() + python = path / ".venv" / "bin" / "python" + if not python.exists(): + raise InfraError(f"{path} has no .venv; run `uv sync` there") + return Side(common.describe(path), python) + sha = common.git("rev-parse", "--verify", f"{ref}^{{commit}}", cwd=common.root()) + target = common.cache_dir() / "refs" / sha[:12] + python = target / ".venv" / "bin" / "python" + if not python.exists(): + print(f"building reference {ref} ({sha[:8]}) in {target} ...", flush=True) + shutil.rmtree(target, ignore_errors=True) + target.mkdir(parents=True) + archive = subprocess.Popen( + ["git", "archive", sha], cwd=common.root(), stdout=subprocess.PIPE + ) + subprocess.run( + ["tar", "-x", "-C", str(target)], stdin=archive.stdout, check=True + ) + if archive.wait(): + raise InfraError(f"git archive {sha} failed") + sync = subprocess.run( + ["uv", "sync", "--frozen", "--no-dev", "-q"], + cwd=target, + env=common.child_env(), + capture_output=True, + text=True, + check=False, + ) + if sync.returncode: + shutil.rmtree(target, ignore_errors=True) + raise InfraError(f"uv sync of {ref} failed: {sync.stderr.strip()[-400:]}") + return Side(f"{ref}@{sha[:8]}", python) + + +def measure(side: Side, case: Case, args, mode: str = "bench") -> dict: + """Run the worker once and return its result, or `{"error": ...}`.""" + cmd = [ + str(side.python), + str(common.WORKER), + "--circuit", + str(case.path), + "--mode", + mode, + "--sampler", + case.sampler, + "--postselect", + case.postselect, + "--min-seconds", + str(args.min_seconds), + ] + if args.strategy: + cmd += ["--strategy", args.strategy] + if args.batch_size: + cmd += ["--batch-size", str(args.batch_size)] + if args.bit_packed: + cmd.append("--bit-packed") + wrapped = common.capped( + cmd, mem=args.mem, timeout=args.timeout, pin=common.pin_cpus(args.threads) + ) + done = subprocess.run( + wrapped, + env=common.child_env(args.threads), + capture_output=True, + text=True, + check=False, + ) + for line in done.stdout.splitlines(): + if line.startswith("RESULT "): + return json.loads(line[7:]) + reason = common.explain_exit(done.returncode, args.mem, args.timeout) + last = (done.stderr.strip().splitlines() or [""])[-1] + return {"error": f"{reason}: {last}"[:200]} + + +def compare(new: list[float], ref: list[float], tolerance: float) -> dict: + """Compare paired rounds of a higher-is-better quantity. + + The verdict is "faster" or "slower" only when the geometric mean of the + per-round ratios is outside the tolerance and every round agrees on the sign; + "noisy" when the mean is outside but the rounds disagree; otherwise "same". + """ + ratios = [a / b for a, b in zip(new, ref, strict=True)] + mean = math.exp(statistics.fmean(math.log(r) for r in ratios)) + low, high = min(ratios), max(ratios) + if mean > 1 + tolerance: + verdict = "faster" if low > 1 else "noisy" + elif mean < 1 - tolerance: + verdict = "slower" if high < 1 else "noisy" + else: + verdict = "same" + return {"ratio": mean, "low": low, "high": high, "verdict": verdict} + + +def run_case(case: Case, sides: list[Side], args) -> dict: + """Prime the caches, then measure every side in interleaved rounds.""" + rounds: dict[str, list[dict]] = {side.label: [] for side in sides} + record: dict = {"circuit": case.name, "path": str(case.path)} + for side in sides: + primed = measure(side, case, args, mode="prime") + if "error" in primed: + record["error"] = f"{side.label}: {primed['error']}" + return record + for index in range(args.rounds): + order = sides if index % 2 == 0 else sides[::-1] + for side in order: + result = measure(side, case, args) + if "error" in result: + record["error"] = f"{side.label}: {result['error']}" + return record + rounds[side.label].append(result) + record["rounds"] = rounds + new = rounds[sides[0].label] + record["k"] = new[0].get("active_width") + record["shots_per_s"] = statistics.median(r["shots_per_s"] for r in new) + record["compile_s"] = statistics.median(r["compile_s"] for r in new) + rates = [r["shots_per_s"] for r in new] + record["spread"] = (max(rates) - min(rates)) / record["shots_per_s"] + if len(sides) == 2: + old = rounds[sides[1].label] + record["ref_shots_per_s"] = statistics.median(r["shots_per_s"] for r in old) + record["ref_compile_s"] = statistics.median(r["compile_s"] for r in old) + record["rate"] = compare(rates, [r["shots_per_s"] for r in old], args.tolerance) + # NOTE: compile time is lower-is-better, so the ratio is ref over new. Small + # absolute differences are ignored: they are import and cache-load noise. + record["compile"] = compare( + [r["compile_s"] for r in old], + [r["compile_s"] for r in new], + max(args.tolerance, 0.10), + ) + if abs(record["compile_s"] - record["ref_compile_s"]) < 0.05: + record["compile"]["verdict"] = "same" + return record + + +def format_row(record: dict, has_ref: bool) -> str: + """Format one result line.""" + name = record["circuit"][:38] + if "error" in record: + return f"{name:<38} ERROR: {record['error']}" + k = "-" if record["k"] is None else record["k"] + row = f"{name:<38} {k!s:>3} {common.si(record['shots_per_s']):>8}" + row += f" {record['compile_s']:>8.2f}" + if not has_ref: + return row + f" ±{record['spread'] * 50:.0f}%" + rate, comp = record["rate"], record["compile"] + row += f" {rate['ratio']:.2f}x [{rate['low']:.2f}, {rate['high']:.2f}]" + row += f" {rate['verdict']:<6} {comp['ratio']:.2f}x {comp['verdict']}" + return row + + +def run(args) -> int: + """Run the benchmark and return the exit code.""" + try: + cases = select(args) + sides = [Side(common.describe(common.root()), Path(sys.executable))] + if args.ref: + sides.append(reference(args.ref)) + except InfraError as error: + common.die(str(error)) + has_ref = len(sides) == 2 + rounds = args.rounds or (5 if has_ref else 3) + args.rounds = rounds + header = f"bench: new={sides[0].label}" + if has_ref: + header += f" ref={sides[1].label}" + busy = common.busy_cores() + header += f" threads={args.threads} rounds={rounds} busy_cores={busy:.1f}" + print(header) + if busy > 1.0: + print("WARNING: other processes are using the CPU; expect noisy numbers") + columns = f"{'circuit':<38} {'k':>3} {'shots/s':>8} {'compile':>8}" + if has_ref: + columns += " shots/s vs ref (min, max) compile vs ref" + print(columns, flush=True) + + records = [] + for case in cases: + record = run_case(case, sides, args) + records.append(record) + print(format_row(record, has_ref), flush=True) + + out = common.out_dir("bench") / f"{common.stamp()}.json" + meta = { + "new": sides[0].label, + "ref": sides[1].label if has_ref else None, + "threads": args.threads, + "rounds": rounds, + "tolerance": args.tolerance, + "busy_cores": busy, + "cpu_count": os.cpu_count(), + } + out.write_text(json.dumps({"meta": meta, "results": records}, indent=1)) + + errors = sum("error" in record for record in records) + verdicts = [ + record[key]["verdict"] + for record in records + if "error" not in record and has_ref + for key in ("rate", "compile") + ] + slower = verdicts.count("slower") + summary = f"SUMMARY: {len(records)} circuits, {errors} errors" + if has_ref: + summary += ( + f", {slower} slower, {verdicts.count('faster')} faster, " + f"{verdicts.count('noisy')} noisy (tolerance {args.tolerance:.0%})" + ) + print(f"{summary}. Details: {out}") + if errors: + return INFRA + return FAILED if slower else OK + + +def fetch_corpus(_args) -> int: + """Clone or update the tsim-bench corpus in the shared cache.""" + target = common.cache_dir() / "tsim-bench" + if (target / ".git").is_dir(): + cmd = ["git", "-C", str(target), "pull", "--ff-only", "-q"] + else: + url = "https://github.com/rafaelha/tsim-bench" + cmd = ["git", "clone", "--depth", "1", "-q", url, str(target)] + if subprocess.run(cmd, check=False).returncode: + common.die("could not fetch the tsim-bench corpus") + count = len(corpus_cases(target, 10**6)) + print(f"corpus at {target}: {count} circuits with a recorded active width") + return OK diff --git a/dev/tsim_dev/checks.py b/dev/tsim_dev/checks.py new file mode 100644 index 00000000..ccfd9d3f --- /dev/null +++ b/dev/tsim_dev/checks.py @@ -0,0 +1,161 @@ +"""`tsim-dev test` and `tsim-dev check`: terse, capped test and lint runs.""" + +from __future__ import annotations + +import subprocess +import sys +import time +from pathlib import Path + +from . import common +from .common import FAILED, OK, InfraError + + +def changed_files() -> list[str]: + """Return files that differ from the merge base with main, plus untracked ones.""" + root = common.root() + base = "HEAD" + for candidate in ("origin/main", "main"): + try: + base = common.git("merge-base", "HEAD", candidate, cwd=root) + break + except InfraError: + continue + names = common.git("diff", "--name-only", base, cwd=root).splitlines() + names += common.git( + "ls-files", "--others", "--exclude-standard", cwd=root + ).splitlines() + return sorted({name for name in names if (root / name).exists()}) + + +def tests_for(files: list[str]) -> list[str] | None: + """Map changed files to test files; None means "run the full suite".""" + root = common.root() + tests: set[str] = set() + for name in files: + path = Path(name) + if path.suffix != ".py" and path.parts[0] not in ("skills", "dev"): + continue + if path.parts[0] == "test": + if path.name.startswith("test_"): + tests.add(name) + elif "helpers" in path.parts: + return None + elif path.parts[0] == "skills": + tests.add("test/unit/test_skills.py") + elif path.parts[0] == "dev": + tests.add("test/unit/test_dev_cli.py") + elif path.parts[:2] == ("src", "tsim"): + inner = Path(*path.parts[2:-1]) + candidates = [ + Path("test/unit") / inner / f"test_{path.name}", + Path("test/unit") / f"test_{path.name}", + Path("test/integration") / f"test_{path.name}", + ] + found = [str(c) for c in candidates if (root / c).exists()] + if not found: + return None + tests.update(found) + return sorted(tests) + + +def summarize(log: Path, limit: int = 40) -> str: + """Return the failures and the final line of a pytest log.""" + lines = log.read_text(errors="replace").splitlines() + if not lines: + return "(no output)" + marker = next( + (i for i, line in enumerate(lines) if "short test summary info" in line), + len(lines) - 3, + ) + out: list[str] = [] + first = next((i for i, line in enumerate(lines) if line.startswith("____")), None) + if first is not None and first < marker: + out += lines[first : min(first + limit, marker)] + out += lines[marker:][-limit:] + return "\n".join(out) + + +def run_pytest(targets: list[str], mem: str, timeout: int) -> tuple[int, Path]: + """Run pytest under the caps with its output in a log file.""" + log = common.out_dir("logs") / f"test-{common.stamp()}.log" + cmd = [sys.executable, "-m", "pytest", "-q", "--no-header", "-rfE", "--tb=short"] + wrapped = common.capped([*cmd, *targets], mem=mem, timeout=timeout) + with open(log, "w") as file: + code = subprocess.run( + wrapped, + cwd=common.root(), + env=common.child_env(), + stdout=file, + stderr=subprocess.STDOUT, + check=False, + ).returncode + return code, log + + +def test(args) -> int: + """Run the tests of the changed files, the full suite, or the given targets.""" + targets = list(args.pytest_args) + if not targets and args.changed: + mapped = tests_for(changed_files()) + if mapped == []: + print( + "no changed files map to tests; use `tsim-dev test` for the full suite" + ) + return OK + if mapped is None: + print("a changed file has no matching test file: running the full suite") + targets = mapped or [] + targets = [*(targets or ["test"]), *args.command] + start = time.time() + code, log = run_pytest(targets, args.mem, args.timeout) + print(summarize(log)) + if code in (124, 137, -9): + print(f"ERROR: {common.explain_exit(code, args.mem, args.timeout)}") + verdict = "PASS" if code == 0 else "FAIL" + print(f"{verdict} tests ({time.time() - start:.0f} s). Log: {log}") + return OK if code == 0 else FAILED + + +def check(args) -> int: + """Run the gate for a finished change: pre-commit on changed files, then tests.""" + files = changed_files() + failed = False + + start = time.time() + log = common.out_dir("logs") / f"lint-{common.stamp()}.log" + cmd = [sys.executable, "-m", "pre_commit", "run"] + cmd += ["--files", *files] if files else ["--all-files"] + with open(log, "w") as file: + code = subprocess.run( + cmd, + cwd=common.root(), + env=common.child_env(), + stdout=file, + stderr=subprocess.STDOUT, + check=False, + ).returncode + if code: + failed = True + text = log.read_text(errors="replace").splitlines() + bad = [i for i, line in enumerate(text) if line.rstrip().endswith("Failed")] + print("\n".join(text[bad[0] :][:40] if bad else text[-20:])) + verdict = "FAIL" if code else "PASS" + print( + f"{verdict} lint, {len(files)} changed files ({time.time() - start:.0f} s). Log: {log}" + ) + if code: + print("pre-commit may have reformatted files; review them and run again") + + if not args.no_tests: + start = time.time() + code, log = run_pytest(["test"], args.mem, args.timeout) + if code: + failed = True + print(summarize(log)) + verdict = "FAIL" if code else "PASS" + last = (log.read_text(errors="replace").splitlines() or [""])[-1] + print( + f"{verdict} tests: {last.strip('= ')} ({time.time() - start:.0f} s). Log: {log}" + ) + return FAILED if failed else OK diff --git a/dev/tsim_dev/cli.py b/dev/tsim_dev/cli.py new file mode 100644 index 00000000..6e924cc1 --- /dev/null +++ b/dev/tsim_dev/cli.py @@ -0,0 +1,141 @@ +"""The `tsim-dev` command line. + +Exit codes: 0 success, 1 the code under test failed (tests, lint, a slower +benchmark, a failed job), 2 the harness could not run, 3 a job is still running. +""" + +from __future__ import annotations + +import argparse +import sys + +from . import bench, checks, jobs, profile + + +def _caps(parser, mem: str, timeout: int, cpus: float | None = None) -> None: + parser.add_argument("--mem", default=mem, help=f"memory cap (default {mem})") + parser.add_argument( + "--timeout", type=int, default=timeout, help=f"seconds (default {timeout})" + ) + if cpus is not None: + parser.add_argument( + "--cpus", type=float, default=cpus, help=f"CPU quota (default {cpus:g})" + ) + + +def _circuit_options(parser) -> None: + parser.add_argument( + "--sampler", default="auto", choices=["auto", "detector", "measurement"] + ) + parser.add_argument("--postselect", default="none", choices=["none", "all"]) + parser.add_argument( + "--threads", type=int, default=1, help="numba threads (default 1)" + ) + + +def build_parser() -> argparse.ArgumentParser: + """Build the argument parser.""" + parser = argparse.ArgumentParser(prog="tsim-dev", description=__doc__) + sub = parser.add_subparsers(dest="command_name", required=True) + + p = sub.add_parser("check", help="gate for a finished change: lint, then all tests") + p.add_argument("--no-tests", action="store_true", help="lint only") + _caps(p, "12G", 1800) + p.set_defaults(func=checks.check) + + p = sub.add_parser("test", help="run tests with a short summary") + p.add_argument("--changed", action="store_true", help="only tests of changed files") + p.add_argument("pytest_args", nargs="*", help="paths; pytest options go after --") + _caps(p, "12G", 1800) + p.set_defaults(func=checks.test) + + p = sub.add_parser("bench", help="throughput and compile time, optionally vs a ref") + p.add_argument("circuits", nargs="*", help="registry names or .stim paths") + p.add_argument("--ref", help="git revision or checkout path to compare against") + p.add_argument("--suite", default="quick", choices=["quick", "full"]) + p.add_argument( + "--corpus", + nargs="?", + const="", + help="use the tsim-bench corpus (optionally its path) instead of the registry", + ) + p.add_argument("--max-k", type=int, default=12, help="corpus: largest active width") + p.add_argument("--only", default="", help="keep circuits whose name contains this") + p.add_argument("--rounds", type=int, default=0, help="default 5 with --ref, else 3") + p.add_argument( + "--min-seconds", type=float, default=0.5, help="length of a timed call" + ) + p.add_argument("--tolerance", type=float, default=0.05, help="default 0.05") + p.add_argument("--strategy", default="") + p.add_argument("--batch-size", type=int, default=0) + p.add_argument("--bit-packed", action="store_true") + _circuit_options(p) + _caps(p, "8G", 600) + p.set_defaults(func=bench.run) + + p = sub.add_parser("corpus", help="fetch the tsim-bench circuit corpus") + p.add_argument("action", choices=["fetch"]) + p.set_defaults(func=bench.fetch_corpus) + + p = sub.add_parser("inspect", help="size and active width of a circuit") + p.add_argument("circuit") + _circuit_options(p) + _caps(p, "8G", 600) + p.set_defaults(func=profile.inspect) + + p = sub.add_parser("profile", help="where compile and sampling time goes (py-spy)") + p.add_argument("circuit") + p.add_argument( + "--seconds", type=float, default=5.0, help="sampling time to profile" + ) + p.add_argument("--rate", type=int, default=500, help="samples per second") + p.add_argument("--top", type=int, default=12, help="frames per table") + _circuit_options(p) + _caps(p, "8G", 600) + p.set_defaults(func=profile.profile) + + p = sub.add_parser( + "run", help="run `-- command ...` under memory, CPU and time caps" + ) + _caps(p, "8G", 1800, cpus=4) + p.set_defaults(func=jobs.run) + + job = sub.add_parser("job", help="capped background jobs") + job_sub = job.add_subparsers(dest="job_command", required=True) + p = job_sub.add_parser("start", help="start `NAME -- command ...` as a job") + p.add_argument("name") + _caps(p, "8G", 3600, cpus=4) + p.set_defaults(func=jobs.start) + p = job_sub.add_parser( + "status", help="state and last log lines of a job, or all jobs" + ) + p.add_argument("name", nargs="?") + p.add_argument("--lines", type=int, default=15) + p.set_defaults(func=jobs.status) + p = job_sub.add_parser( + "wait", help="block until a job ends (exit 3 if still running)" + ) + p.add_argument("name") + p.add_argument( + "--timeout", type=int, default=540, help="seconds to wait (default 540)" + ) + p.add_argument("--lines", type=int, default=15) + p.set_defaults(func=jobs.wait) + p = job_sub.add_parser("stop", help="stop a job and its children") + p.add_argument("name") + p.set_defaults(func=jobs.stop) + return parser + + +def main(argv: list[str] | None = None) -> None: + """Run the command line.""" + argv = list(sys.argv[1:] if argv is None else argv) + # NOTE: everything after "--" is the command to run (or options for pytest), so + # it is split off before argparse can read the command's own options. + rest: list[str] = [] + if "--" in argv: + split = argv.index("--") + argv, rest = argv[:split], argv[split + 1 :] + args = build_parser().parse_args(argv) + args.command = rest + raise SystemExit(args.func(args)) diff --git a/dev/tsim_dev/common.py b/dev/tsim_dev/common.py new file mode 100644 index 00000000..f499966f --- /dev/null +++ b/dev/tsim_dev/common.py @@ -0,0 +1,186 @@ +"""Shared helpers: paths, the environment of child processes, and memory caps.""" + +from __future__ import annotations + +import functools +import os +import shutil +import subprocess +import sys +import time +from pathlib import Path +from typing import NoReturn + +DEV_DIR = Path(__file__).resolve().parent.parent +CIRCUITS_DIR = DEV_DIR / "circuits" +WORKER = Path(__file__).resolve().parent / "_worker.py" + +# Exit codes shared by every subcommand. +OK, FAILED, INFRA, RUNNING = 0, 1, 2, 3 + + +class InfraError(Exception): + """The harness could not run the request (as opposed to the code failing it).""" + + +def die(message: str) -> NoReturn: + """Print one greppable error line and exit with the infrastructure code.""" + print(f"ERROR: {message}", file=sys.stderr) + raise SystemExit(INFRA) + + +def git(*args: str, cwd: Path | None = None) -> str: + """Run git and return its stripped stdout.""" + result = subprocess.run( + ["git", *args], cwd=cwd, capture_output=True, text=True, check=False + ) + if result.returncode: + raise InfraError(f"git {' '.join(args)}: {result.stderr.strip()}") + return result.stdout.strip() + + +@functools.cache +def root() -> Path: + """Return the checkout that the command was started in.""" + try: + return Path(git("rev-parse", "--show-toplevel")) + except InfraError: + return DEV_DIR.parent + + +def out_dir(kind: str) -> Path: + """Return (and create) a directory for logs and results inside the checkout.""" + path = root() / ".tsim-dev" / kind + path.mkdir(parents=True, exist_ok=True) + return path + + +def cache_dir() -> Path: + """Return the cache shared by every checkout (reference builds, corpus).""" + path = Path(os.environ.get("TSIM_DEV_CACHE", "~/.cache/tsim-dev")).expanduser() + path.mkdir(parents=True, exist_ok=True) + return path + + +def stamp() -> str: + """Return a timestamp that sorts and is safe in file names.""" + return time.strftime("%Y%m%d-%H%M%S") + + +def describe(path: Path) -> str: + """Describe a checkout as branch@commit, with -dirty when it has changes.""" + try: + branch = git("rev-parse", "--abbrev-ref", "HEAD", cwd=path) + commit = git("rev-parse", "--short=8", "HEAD", cwd=path) + if git("status", "--porcelain", "--untracked-files=no", cwd=path): + commit += "-dirty" + except InfraError: + return str(path) + return f"{branch}@{commit}" + + +def child_env(threads: int | None = None) -> dict[str, str]: + """Return the environment for Tsim processes. + + JAX is kept on the CPU unless the caller already chose a platform, which also + silences its missing-CUDA warning. The thread variables are set when given. + """ + env = dict(os.environ) + env.setdefault("JAX_PLATFORMS", "cpu") + env.pop("VIRTUAL_ENV", None) + if threads is not None: + for name in ("NUMBA_NUM_THREADS", "OMP_NUM_THREADS", "OPENBLAS_NUM_THREADS"): + env[name] = str(threads) + return env + + +@functools.cache +def has_systemd() -> bool: + """Return whether transient user scopes work on this machine.""" + if not shutil.which("systemd-run"): + return False + probe = subprocess.run( + ["systemd-run", "--user", "--scope", "-q", "true"], + capture_output=True, + check=False, + ) + return probe.returncode == 0 + + +def capped( + cmd: list[str], + *, + mem: str, + cpus: float | None = None, + timeout: int | None = None, + pin: list[int] | None = None, +) -> list[str]: + """Wrap a command in a memory-capped scope, optionally pinned and timed out. + + Without systemd (macOS, some containers) the command runs uncapped, with a + warning, so that the CLI still works there. + """ + if pin and shutil.which("taskset"): + cmd = ["taskset", "-c", ",".join(map(str, pin)), *cmd] + if timeout and shutil.which("timeout"): + cmd = ["timeout", "--signal=TERM", "--kill-after=10", str(timeout), *cmd] + if not has_systemd(): + print("WARNING: no systemd user session, running without a memory cap") + return cmd + props = ["-p", f"MemoryMax={mem}", "-p", "MemorySwapMax=0"] + if cpus: + props += ["-p", f"CPUQuota={round(cpus * 100)}%"] + return ["systemd-run", "--user", "--scope", "-q", *props, *cmd] + + +def explain_exit(code: int, mem: str, timeout: int | None) -> str: + """Name the usual reasons for a capped command's exit code.""" + if code == 124: + return f"timed out after {timeout} s" + if code in (137, -9): + return f"killed, most likely by the {mem} memory cap" + return f"exit code {code}" + + +def pin_cpus(threads: int) -> list[int]: + """Pick the last `threads` CPUs this process may use.""" + if not hasattr(os, "sched_getaffinity"): + return [] + cpus = sorted(os.sched_getaffinity(0)) + return cpus[-threads:] + + +def busy_cores(interval: float = 0.3) -> float: + """Return how many cores other processes keep busy right now.""" + + def snapshot() -> tuple[int, int]: + fields = [ + int(x) for x in Path("/proc/stat").read_text().split("\n")[0].split()[1:] + ] + return sum(fields), fields[3] + fields[4] + + try: + total0, idle0 = snapshot() + time.sleep(interval) + total1, idle1 = snapshot() + except (OSError, ValueError, IndexError): + return os.getloadavg()[0] + if total1 == total0: + return 0.0 + return (1 - (idle1 - idle0) / (total1 - total0)) * (os.cpu_count() or 1) + + +def si(value: float) -> str: + """Format a count or rate the short way: 950, 3.2k, 2.1M.""" + for bound, suffix in ((1e9, "G"), (1e6, "M"), (1e3, "k")): + if abs(value) >= bound: + return f"{value / bound:.3g}{suffix}" + return f"{value:.3g}" + + +def tail(path: Path, lines: int) -> str: + """Return the last lines of a text file.""" + if not path.exists(): + return "" + text = path.read_text(errors="replace").splitlines() + return "\n".join(text[-lines:]) diff --git a/dev/tsim_dev/jobs.py b/dev/tsim_dev/jobs.py new file mode 100644 index 00000000..f2419129 --- /dev/null +++ b/dev/tsim_dev/jobs.py @@ -0,0 +1,195 @@ +"""`tsim-dev run` and `tsim-dev job`: commands under hard memory and time caps. + +A job is a transient systemd user unit, so it can be watched, waited for and +stopped by name. That replaces `nohup ... &`, sleep loops and `pkill -f`. +""" + +from __future__ import annotations + +import json +import os +import re +import subprocess +import time + +from . import common +from .common import FAILED, INFRA, OK, RUNNING + + +def run(args) -> int: + """Run a command in the foreground under the caps.""" + if not args.command: + common.die( + "give a command after `--`, for example: tsim-dev run -- python x.py" + ) + cmd = common.capped( + args.command, mem=args.mem, cpus=args.cpus, timeout=args.timeout + ) + threads = max(1, int(args.cpus)) if args.cpus else None + code = subprocess.run(cmd, env=common.child_env(threads), check=False).returncode + if code in (124, 137, -9): + reason = common.explain_exit(code, args.mem, args.timeout) + print(f"ERROR: {reason}", flush=True) + return 137 if code == -9 else code + + +def _unit(name: str) -> str: + return f"tsim-dev-{name}" + + +def _paths(name: str): + base = common.out_dir("jobs") + return base / f"{name}.json", base / f"{name}.log", base / f"{name}.exit" + + +def _show(name: str) -> dict[str, str]: + result = subprocess.run( + [ + "systemctl", + "--user", + "show", + _unit(name), + "-p", + "ActiveState,Result,MemoryPeak,MemoryCurrent", + ], + capture_output=True, + text=True, + check=False, + ) + return dict( + line.split("=", 1) for line in result.stdout.splitlines() if "=" in line + ) + + +def _state(name: str) -> tuple[str, int]: + """Return a one-line state and the exit code that describes it.""" + meta, _, exit_file = _paths(name) + if not meta.exists(): + return "unknown job", INFRA + show = _show(name) + started = json.loads(meta.read_text())["started"] + age = f"{time.time() - started:.0f} s" + if show.get("ActiveState") in ("active", "activating", "deactivating"): + peak = show.get("MemoryPeak", "") + memory = f", peak memory {int(peak) / 2**30:.1f} GB" if peak.isdigit() else "" + return f"running for {age}{memory}", RUNNING + if exit_file.exists() and exit_file.read_text().strip().isdigit(): + code = int(exit_file.read_text()) + if code == 0: + return "finished, exit code 0", OK + if code == 137: + return "failed: killed, most likely by the memory cap", FAILED + return f"failed, exit code {code}", FAILED + reasons = {"oom-kill": "killed by the memory cap", "timeout": "hit the time limit"} + return reasons.get(show.get("Result", ""), "stopped before it finished"), FAILED + + +def start(args) -> int: + """Start a command as a named background job.""" + if not common.has_systemd(): + common.die("`job` needs a systemd user session; use `tsim-dev run` instead") + if not re.fullmatch(r"[A-Za-z0-9_.-]+", args.name): + common.die("job names may contain letters, digits, '.', '_' and '-'") + if not args.command: + common.die("give a command after `--`") + if _state(args.name)[1] == RUNNING: + common.die(f"job {args.name} is already running; stop it or pick another name") + meta, log, exit_file = _paths(args.name) + exit_file.unlink(missing_ok=True) + log.write_text("") + subprocess.run( + ["systemctl", "--user", "reset-failed", _unit(args.name)], + capture_output=True, + check=False, + ) + env = common.child_env(max(1, int(args.cpus))) + env["PYTHONUNBUFFERED"] = "1" + setenv = [ + f"--setenv={key}={value}" + for key, value in env.items() + if "\n" not in value and re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", key) + ] + # NOTE: the shell records the exit code, so a finished job can be told apart + # from one that systemd killed for its memory or time limit. + script = '"$@"; code=$?; echo $code > "$TSIM_DEV_EXIT"; exit $code' + cmd = [ + "systemd-run", + "--user", + "-q", + f"--unit={_unit(args.name)}", + "--same-dir", + "-p", + f"MemoryMax={args.mem}", + "-p", + "MemorySwapMax=0", + "-p", + f"CPUQuota={round(args.cpus * 100)}%", + "-p", + f"RuntimeMaxSec={args.timeout}", + "-p", + f"StandardOutput=append:{log}", + "-p", + "StandardError=inherit", + f"--setenv=TSIM_DEV_EXIT={exit_file}", + *setenv, + "bash", + "-c", + script, + "job", + *args.command, + ] + done = subprocess.run(cmd, capture_output=True, text=True, check=False) + if done.returncode: + common.die(f"systemd-run failed: {done.stderr.strip()[-300:]}") + meta.write_text( + json.dumps( + {"command": args.command, "started": time.time(), "cwd": os.getcwd()} + ) + ) + print( + f"started job {args.name} (mem {args.mem}, cpus {args.cpus:g}, " + f"limit {args.timeout} s). Log: {log}\n" + f"Next: tsim-dev job wait {args.name}" + ) + return OK + + +def _report(name: str, lines: int) -> int: + state, code = _state(name) + print(f"job {name}: {state}") + if code != INFRA and lines: + text = common.tail(_paths(name)[1], lines) + if text: + print(text) + return code + + +def status(args) -> int: + """Report one job, or list every job of this checkout.""" + if args.name: + return _report(args.name, args.lines) + names = sorted(path.stem for path in common.out_dir("jobs").glob("*.json")) + for name in names: + print(f"job {name}: {_state(name)[0]}") + if not names: + print("no jobs") + return OK + + +def wait(args) -> int: + """Block until the job ends or the wait times out, then report it.""" + deadline = time.time() + args.timeout + while _state(args.name)[1] == RUNNING and time.time() < deadline: + time.sleep(2) + return _report(args.name, args.lines) + + +def stop(args) -> int: + """Stop a job and everything it started.""" + subprocess.run( + ["systemctl", "--user", "stop", _unit(args.name)], + capture_output=True, + check=False, + ) + print(f"job {args.name}: {_state(args.name)[0]}") + return OK diff --git a/dev/tsim_dev/profile.py b/dev/tsim_dev/profile.py new file mode 100644 index 00000000..b93b2bbd --- /dev/null +++ b/dev/tsim_dev/profile.py @@ -0,0 +1,169 @@ +"""`tsim-dev inspect` and `tsim-dev profile`: what a circuit costs, and where.""" + +from __future__ import annotations + +import collections +import json +import re +import subprocess +import sys +from pathlib import Path + +from . import bench, common +from .common import INFRA, OK, InfraError + +PHASES = ("phase_import", "phase_compile", "phase_sample") +UNKNOWN = re.compile(r"0x[0-9a-f]+ \(\?\)") +JITTED = "[jitted numba code]" + + +def _case(args) -> bench.Case: + args.corpus, args.only, args.suite = None, "", "quick" + args.circuits = [args.circuit] + return bench.select(args)[0] + + +def _defaults(args) -> None: + for name, value in (("strategy", ""), ("batch_size", 0), ("bit_packed", False)): + if not hasattr(args, name): + setattr(args, name, value) + + +def inspect(args) -> int: + """Print the size and active width of a circuit, without sampling it.""" + _defaults(args) + args.min_seconds = 0.0 + try: + case = _case(args) + except InfraError as error: + common.die(str(error)) + side = bench.Side("new", Path(sys.executable)) + result = bench.measure(side, case, args, mode="inspect") + if "error" in result: + print(f"ERROR: {result['error']}") + return INFRA + print(f"{case.name}: {result['sampler']} sampler, Tsim {result['tsim_version']}") + for key in ("qubits", "tcount", "measurements", "detectors", "active_width"): + print(f" {key}: {result[key]}") + print(f" compile_s: {result['compile_s']:.2f}") + if result.get("stats"): + print(" stats: " + result["stats"].replace("\n", "\n ")) + return OK + + +def fold(path: Path) -> dict[str, collections.Counter]: + """Split py-spy's folded stacks by phase and count samples per frame. + + Returns, for each phase, counters keyed by ("self", frame) for the innermost + frame, ("leaf", frame) for the innermost Tsim frame, and ("total", frame) for + every Tsim frame on the stack. Jitted numba code has no symbols, so its + addresses are counted together. + """ + phases: dict[str, collections.Counter] = {p: collections.Counter() for p in PHASES} + for line in path.read_text(errors="replace").splitlines(): + stack, _, count = line.rpartition(" ") + if not count.isdigit(): + continue + phase = next((p for p in PHASES if p in stack), None) + if phase is None: + continue + frames = [re.sub(r":\d+\)$", ")", frame) for frame in stack.split(";")] + frames = [JITTED if UNKNOWN.fullmatch(f) else f for f in frames] + ours = [f for f in frames if "tsim/" in f] + counter = phases[phase] + counter["samples", ""] += int(count) + counter["self", frames[-1]] += int(count) + # NOTE: kernels.py only dispatches jitted kernels, so the frame that called + # it says more about where the time goes. + callers = [f for f in ours if "tsim/utils/kernels.py" not in f] + if callers: + counter["leaf", callers[-1]] += int(count) + for frame in set(ours): + counter["total", frame] += int(count) + return phases + + +def profile(args) -> int: + """Sample the stacks of one run with py-spy and print the top frames per phase.""" + _defaults(args) + spy = Path(sys.executable).parent / "py-spy" + if not spy.exists(): + common.die("py-spy is not installed in this environment; run `uv sync`") + try: + case = _case(args) + except InfraError as error: + common.die(str(error)) + side = bench.Side("new", Path(sys.executable)) + args.min_seconds = args.seconds / 10 + primed = bench.measure(side, case, args, mode="prime") + if "error" in primed: + print(f"ERROR: {primed['error']}") + return INFRA + + raw = ( + common.out_dir("profile") + / f"{case.name.replace('/', '_')}-{common.stamp()}.folded" + ) + worker = [ + sys.executable, + str(common.WORKER), + "--circuit", + str(case.path), + "--mode", + "profile", + "--sampler", + case.sampler, + "--postselect", + case.postselect, + "--min-seconds", + str(args.min_seconds), + ] + spy_cmd = [str(spy), "record", "--native", "-f", "raw", "-r", str(args.rate)] + cmd = common.capped( + [*spy_cmd, "-o", str(raw), "--", *worker], + mem=args.mem, + timeout=args.timeout, + pin=common.pin_cpus(args.threads), + ) + done = subprocess.run( + cmd, + env=common.child_env(args.threads), + capture_output=True, + text=True, + check=False, + ) + result = next( + ( + json.loads(x[7:]) + for x in done.stdout.splitlines() + if x.startswith("RESULT ") + ), + None, + ) + if result is None or not raw.exists(): + reason = common.explain_exit(done.returncode, args.mem, args.timeout) + print(f"ERROR: profile failed ({reason}): {done.stderr.strip()[-300:]}") + return INFRA + + print( + f"profile {case.name}: k={result['active_width']} threads={args.threads} " + f"import {result['import_s']:.2f} s, compile {result['compile_s']:.2f} s " + f"(warm numba cache), sampling {args.seconds:g} s at {result['shots']} shots/call" + ) + for phase, counter in fold(raw).items(): + total = counter["samples", ""] + if phase == "phase_import" or not total: + continue + print(f"\n{phase[6:]}: {total} samples") + tables = ( + ("leaf", "innermost Tsim frame (kernel callers)"), + ("self", "innermost frame"), + ("total", "Tsim frames, inclusive"), + ) + for kind, title in tables: + rows = [(n, f) for (k, f), n in counter.items() if k == kind] + print(f" {title}:") + for count, frame in sorted(rows, reverse=True)[: args.top]: + print(f" {count / total:6.1%} {frame[:110]}") + print(f"\nFolded stacks (speedscope, flamegraph.pl): {raw}") + return OK diff --git a/pyproject.toml b/pyproject.toml index fb59fb8f..ff9159ce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -85,6 +85,7 @@ dev = [ "radon>=6.0.1", "zxparam[quimb]", "pyyaml>=6.0", + "tsim-dev", ] doc = [ "griffe>=2.0.2", @@ -94,6 +95,9 @@ doc = [ "nbformat>=5.10.4", ] +[tool.uv.sources] +tsim-dev = { path = "dev", editable = true } + [tool.hatch.build.targets.wheel] packages = ["src/tsim"] @@ -123,7 +127,7 @@ exclude = ''' [tool.ruff] target-version = "py311" -include = ["src/**/*.py", "test/**/*.py"] +include = ["src/**/*.py", "test/**/*.py", "dev/**/*.py", ".claude/**/*.py"] extend-exclude = ["src/tsim/external"] [tool.ruff.lint] @@ -135,6 +139,8 @@ dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$" "docs/**" = ["D"] "test/**" = ["D"] "evals/**" = ["D", "S603", "S607"] +"dev/**" = ["S603", "S607"] +".claude/**" = ["S603", "S607"] [tool.coverage.run] include = ["src/tsim/*"] @@ -144,7 +150,7 @@ omit = ["src/tsim/external/*"] testpaths = "test/" [tool.pyright] -include = ["src", "test"] +include = ["src", "test", "dev"] exclude = ["src/tsim/external"] reportUnusedImport = "error" @@ -154,6 +160,7 @@ ignore_notebooks = true extend_exclude = [ "^accel-cutting-magic-state-main/", "^evals/", + "^dev/", "^benchmarks/", "^scripts/", "^docs/", diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py new file mode 100644 index 00000000..9ef9b414 --- /dev/null +++ b/test/unit/test_dev_cli.py @@ -0,0 +1,138 @@ +"""Tests for the development CLI in `dev/` and the agent hook in `.claude/`.""" + +import json +import subprocess +import sys +from pathlib import Path + +import pytest +from tsim_dev import bench, checks, cli, common + +ROOT = Path(__file__).resolve().parents[2] +HOOK = ROOT / ".claude" / "hooks" / "guard_bash.py" + + +def test_registry_circuits_exist_and_the_quick_suite_is_small(): + registry = bench.registry() + assert all(case.path.is_file() for case, _ in registry.values()) + assert all( + case.sampler in ("detector", "measurement") for case, _ in registry.values() + ) + assert all(case.postselect in ("all", "none") for case, _ in registry.values()) + quick = [name for name, (_, suite) in registry.items() if suite == "quick"] + assert 4 <= len(quick) <= 8 + + +@pytest.mark.parametrize( + ("new", "ref", "verdict"), + [ + ([100, 101, 99], [100, 100, 100], "same"), + ([120, 121, 119], [100, 100, 100], "faster"), + ([80, 81, 79], [100, 100, 100], "slower"), + ([130, 130, 95], [100, 100, 100], "noisy"), + ([70, 70, 105], [100, 100, 100], "noisy"), + ([104, 104, 104], [100, 100, 100], "same"), + ], +) +def test_compare_needs_every_round_to_agree(new, ref, verdict): + result = bench.compare(new, ref, tolerance=0.05) + assert result["verdict"] == verdict + assert result["low"] <= result["ratio"] <= result["high"] + + +def test_changed_source_files_map_to_their_tests(): + assert checks.tests_for(["src/tsim/sampler.py"]) == [ + "test/integration/test_sampler.py", + "test/unit/test_sampler.py", + ] + assert checks.tests_for(["src/tsim/statevector/plan.py"]) == [ + "test/unit/statevector/test_plan.py" + ] + assert checks.tests_for(["test/unit/test_sampler.py", "README.md"]) == [ + "test/unit/test_sampler.py" + ] + assert checks.tests_for(["dev/tsim_dev/bench.py"]) == ["test/unit/test_dev_cli.py"] + assert checks.tests_for(["README.md"]) == [] + + +def test_changes_without_a_matching_test_run_the_full_suite(): + assert checks.tests_for(["src/tsim/__init__.py"]) is None + assert checks.tests_for(["test/helpers/statevector.py"]) is None + + +def test_counts_are_formatted_the_short_way(): + assert [common.si(x) for x in (950, 3200, 2_100_000, 28.4e6)] == [ + "950", + "3.2k", + "2.1M", + "28.4M", + ] + + +def test_everything_after_the_double_dash_is_the_command(monkeypatch): + seen = {} + monkeypatch.setattr(cli.jobs, "run", lambda args: seen.update(vars(args)) or 0) + with pytest.raises(SystemExit) as exit_info: + cli.main(["run", "--mem", "2G", "--", "python", "--mem", "x.py"]) + assert exit_info.value.code == 0 + assert seen["mem"] == "2G" + assert seen["command"] == ["python", "--mem", "x.py"] + + +def test_pytest_summary_keeps_the_first_failure_and_the_last_line(tmp_path): + log = tmp_path / "test.log" + log.write_text( + "..F.\n" + "=== FAILURES ===\n" + "____ test_x ____\n" + "E assert 1 == 2\n" + "=== short test summary info ===\n" + "FAILED test/unit/test_x.py::test_x - assert 1 == 2\n" + "1 failed, 3 passed in 0.10s\n" + ) + summary = checks.summarize(log) + assert "E assert 1 == 2" in summary + assert summary.splitlines()[-1] == "1 failed, 3 passed in 0.10s" + + +@pytest.mark.parametrize( + ("tool_input", "blocked"), + [ + ({"command": "pkill -f fuzz.py"}, True), + ({"command": "pkill -9 -f fuzz"}, True), + ({"command": "nohup python x.py > log &"}, True), + ({"command": "uv run pytest test", "run_in_background": True}, True), + ({"command": "uv run tsim-dev bench", "run_in_background": True}, False), + ({"command": "systemd-run --user --scope python x.py &"}, False), + ({"command": "uv run pytest test/unit -q"}, False), + ({"command": "python a.py && python b.py"}, False), + ({"command": "kill 1234"}, False), + ({"command": "sleep 5 &"}, False), + ], +) +def test_the_bash_hook_blocks_self_kills_and_uncapped_background_compute( + tool_input, blocked +): + done = subprocess.run( # noqa: S603 + [sys.executable, str(HOOK)], + input=json.dumps({"tool_name": "Bash", "tool_input": tool_input}), + capture_output=True, + text=True, + check=False, + ) + assert done.returncode == (2 if blocked else 0) + assert bool(done.stderr) == blocked + + +def test_dev_skills_have_valid_frontmatter(): + skills = sorted((ROOT / ".claude" / "skills").glob("*/SKILL.md")) + assert len(skills) >= 3 + for path in skills: + text = path.read_text() + assert text.startswith("---\n") + front = dict( + line.split(": ", 1) for line in text.split("---\n")[1].strip().splitlines() + ) + assert front["name"] == path.parent.name + assert 0 < len(front["description"]) <= 1024 + assert len(text.splitlines()) < 500 diff --git a/test/unit/utils/test_kernels.py b/test/unit/utils/test_kernels.py index f612b439..c4432bbb 100644 --- a/test/unit/utils/test_kernels.py +++ b/test/unit/utils/test_kernels.py @@ -100,7 +100,9 @@ def test_the_calling_thread_keeps_parallel_kernels_after_run_tasks(): if numba.get_num_threads() < 2: pytest.skip("needs two numba threads") run_tasks(lambda _: _thread_ids(PARALLEL_WORK, np.zeros(64)), range(4), 4) - ids = np.full(1 << 12, -1) + # NOTE: the loop is long enough that every threading layer splits it; TBB + # runs a short loop on one thread. + ids = np.full(1 << 22, -1) _thread_ids(PARALLEL_WORK, ids) assert len(np.unique(ids)) > 1 diff --git a/uv.lock b/uv.lock index d3ec3094..a741f3e7 100644 --- a/uv.lock +++ b/uv.lock @@ -168,6 +168,7 @@ dev = [ { name = "sinter" }, { name = "tesseract-decoder" }, { name = "tqdm" }, + { name = "tsim-dev" }, { name = "vulture" }, { name = "zxparam", extra = ["quimb"] }, ] @@ -220,6 +221,7 @@ dev = [ { name = "sinter", specifier = "~=1.16.dev0" }, { name = "tesseract-decoder", specifier = ">=0.1.1.dev20251113222502" }, { name = "tqdm", specifier = ">=4.67.1" }, + { name = "tsim-dev", editable = "dev" }, { name = "vulture", specifier = ">=2.16" }, { name = "zxparam", extras = ["quimb"] }, ] @@ -3559,6 +3561,11 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/00/c0/8f5d070730d7836adc9c9b6408dec68c6ced86b304a9b26a14df072a6e8c/traitlets-5.14.3-py3-none-any.whl", hash = "sha256:b74e89e397b1ed28cc831db7aea759ba6640cb3de13090ca145426688ff1ac4f", size = 85359, upload-time = "2024-04-19T11:11:46.763Z" }, ] +[[package]] +name = "tsim-dev" +version = "0.1.0" +source = { editable = "dev" } + [[package]] name = "typing-extensions" version = "4.15.0" From ff346848ff47ec95cdeb265d840481ac6909ca6d Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 07:39:46 -0700 Subject: [PATCH 02/11] chore: take benchmark circuits and their postselection from tsim-bench - `tsim-dev bench` reads circuits from tsim-bench at a pinned commit (dev/tsim_dev/corpus.py), fetched into the shared cache on first use, or from a local checkout with `--corpus-dir` / TSIM_BENCH_DIR. Each circuit there records its arXiv paper and how that paper postselects. - Postselection follows the recorded protocol by default; `--no-postselect` turns it off. Rows show the kept fraction. - `--suite quick|full|hard|corpus` (suites defined in tsim-bench), `--max-width K --width-of tsim|clifft` to keep only circuits up to a recorded active width, and circuits named by tsim-bench id or file stem. - `tsim-dev corpus fetch|list`. The timing worker is tsim-bench's. - Drop dev/circuits/ and dev/tsim_dev/_worker.py. The .stim files there were never committed (`*.stim` is ignored), so a fresh clone could not run them. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/benchmarking-tsim/SKILL.md | 49 +++-- dev/circuits/circuits.toml | 57 ------ dev/tsim_dev/_worker.py | 153 ---------------- dev/tsim_dev/bench.py | 207 +++++++++++++--------- dev/tsim_dev/cli.py | 72 ++++++-- dev/tsim_dev/common.py | 2 - dev/tsim_dev/corpus.py | 86 +++++++++ dev/tsim_dev/profile.py | 17 +- test/unit/test_dev_cli.py | 111 ++++++++++-- 9 files changed, 397 insertions(+), 357 deletions(-) delete mode 100644 dev/circuits/circuits.toml delete mode 100644 dev/tsim_dev/_worker.py create mode 100644 dev/tsim_dev/corpus.py diff --git a/.claude/skills/benchmarking-tsim/SKILL.md b/.claude/skills/benchmarking-tsim/SKILL.md index 6e28f42d..34b396bb 100644 --- a/.claude/skills/benchmarking-tsim/SKILL.md +++ b/.claude/skills/benchmarking-tsim/SKILL.md @@ -1,6 +1,6 @@ --- name: benchmarking-tsim -description: Measures Tsim throughput and compile time with `tsim-dev bench` and compares a change against a reference commit or checkout. Use when asked to benchmark Tsim, to check a change for performance regressions, to produce before and after numbers for a pull request, or to run the tsim-bench circuit corpus. +description: Measures Tsim throughput and compile time with `tsim-dev bench` on the tsim-bench circuits and compares a change against a reference commit or checkout. Use when asked to benchmark Tsim, to check a change for performance regressions, to produce before and after numbers for a pull request, to run the tsim-bench corpus, or to add a benchmark circuit. --- # Benchmarking Tsim @@ -21,10 +21,12 @@ any commit, or the path of another checkout that has a `.venv` (`--ref ../tsim-other-worktree`). ``` -circuit k shots/s compile shots/s vs ref (min, max) compile vs ref -cultivation_d5 6 13.9M 0.13 1.20x [1.19, 1.21] faster 0.96x same +circuit k shots/s kept compile shots/s vs ref (min, max) compile vs ref +inject_cultivate_d5_p0.001 6 13.9M 0.141 0.13 1.20x [1.19, 1.21] faster 0.96x same ``` +- `kept` is the fraction of shots that survive postselection (`-` when there is none). + - `shots/s vs ref` is new over reference, so above 1 is faster. The bracket is the smallest and largest ratio over the rounds. - A verdict of `faster` or `slower` needs the mean outside the tolerance (5%) and @@ -42,8 +44,11 @@ cultivation_d5 6 13.9M 0.13 1.20x [1.19, 1.21] faster 0.96x same ones, and the cached case is what users get. - Compile time and the first call are excluded from shots/s and reported apart. - The two checkouts alternate round by round, so drift affects both. -- Each registry circuit fixes its sampler and postselection - (`dev/circuits/circuits.toml`). With postselection, shots/s counts attempted shots. +- Circuits come from tsim-bench (https://github.com/rafaelha/tsim-bench) at the commit + pinned in `dev/tsim_dev/corpus.py`, fetched into `~/.cache/tsim-dev` on first use. + Each circuit's manifest there records its arXiv paper, its sampler and how the paper + postselects; the benchmark postselects that way by default. `--no-postselect` + samples every shot instead. shots/s counts requested shots, kept or not. ## What you must do @@ -51,30 +56,40 @@ cultivation_d5 6 13.9M 0.13 1.20x [1.19, 1.21] faster 0.96x same next to it. Heed the busy-machine warning. - A change must hold across families. Run `--suite full` before claiming a speedup, and report the circuits that got slower as well as the ones that got faster. -- For a single circuit: `uv run tsim-dev bench cultivation_d5 --ref origin/main`, or - a path to any `.stim` file with `--sampler` and `--postselect`. +- For a single circuit: `uv run tsim-dev bench inject_cultivate_d5_p0.001 --ref origin/main` + (a file stem or a full id from `tsim-dev corpus list`), or a path to any `.stim` file + with `--sampler` and `--postselect all|none`. +- Postselection changes what is measured: a change that speeds up rejected shots shows + only with it on. Report which you used, and run both when the change touches the + postselection path. - Paste the table in the pull request, with the CPU model (`lscpu | grep 'Model name'`) and the thread count. -## The tsim-bench corpus +## Suites and the whole corpus -189 circuits from 27 papers (https://github.com/rafaelha/tsim-bench). It takes much -longer, so run it as a job: +Suites are defined in tsim-bench's `suites.toml`: `quick` (the default, a few minutes), +`full` (one or two circuits per family, widths up to about 16) and `hard` (widths 16 to +26, such as the tricycle CCZ factories; minutes per circuit). `--suite corpus` is every +circuit, over 190 from 28 papers, so run it as a job: ```bash -uv run tsim-dev corpus fetch # once; or set TSIM_BENCH_DIR to a clone +uv run tsim-dev corpus list --max-width 12 # what would run uv run tsim-dev job start corpus --timeout 14400 -- \ - uv run tsim-dev bench --corpus --max-k 12 --ref origin/main + uv run tsim-dev bench --suite corpus --max-width 12 --ref origin/main uv run tsim-dev job wait corpus # repeat until it reports the end ``` -`--only SUBSTRING` selects circuits by name; `--max-k` skips circuits whose recorded -active width is larger. +`--max-width K` skips circuits whose recorded active width is larger than K, and those +with no recorded width; `--width-of clifft` (or another backend in tsim-bench's +`k_max.json`) filters on that backend's width instead, for backend work that targets a +different regime. `--only SUBSTRING` keeps circuits whose id contains it. ## Limits - CPU only. GPU runs and comparisons with other simulators (Clifft, SymFT, Magna) are run from the tsim-bench repository, which pins each simulator's environment. -- To add a circuit to the registry, put the file in `dev/circuits/`, add it to - `circuits.toml` and `git add -f` it (`*.stim` is ignored). Keep the quick suite - to one small circuit per family. +- To add a circuit or change a suite, change tsim-bench: put the file in its paper's + folder, add an entry to that folder's `circuits.toml` (arXiv id, sampler, postselection + and the reason for it, see its `BENCHMARKS.md`), run `python -m tsim_bench check` + there, then bump `REV` in `dev/tsim_dev/corpus.py`. Until the commit is pushed, point + the harness at the checkout with `--corpus-dir` or `TSIM_BENCH_DIR`. diff --git a/dev/circuits/circuits.toml b/dev/circuits/circuits.toml deleted file mode 100644 index c9390656..00000000 --- a/dev/circuits/circuits.toml +++ /dev/null @@ -1,57 +0,0 @@ -# Benchmark circuits for `tsim-dev bench`. One family per row of the quick suite. -# sampler: "detector" or "measurement". postselect: "all" (every detector) or "none". -# suite: "quick" circuits run by default; `--suite full` adds the rest. - -[cultivation_d3] -file = "cultivation_d3.stim" -sampler = "detector" -postselect = "all" -suite = "quick" - -[cultivation_d5] -file = "cultivation_d5.stim" -sampler = "detector" -postselect = "all" -suite = "quick" - -[distillation_d5] -file = "distillation_d5.stim" -sampler = "detector" -postselect = "none" -suite = "quick" - -[coherent_d3_r3] -file = "coherent_d3_r3.stim" -sampler = "detector" -postselect = "none" -suite = "quick" - -[qv_q10] -file = "qv_q10.stim" -sampler = "measurement" -postselect = "none" -suite = "quick" - -[surface_d7_r7] -file = "surface_d7_r7.stim" -sampler = "detector" -postselect = "none" -suite = "quick" - -[distillation_d3] -file = "distillation_d3.stim" -sampler = "detector" -postselect = "all" -suite = "full" - -[coherent_d5_r5] -file = "coherent_d5_r5.stim" -sampler = "detector" -postselect = "none" -suite = "full" - -[cultivation_d5_unselected] -file = "cultivation_d5.stim" -sampler = "detector" -postselect = "none" -suite = "full" diff --git a/dev/tsim_dev/_worker.py b/dev/tsim_dev/_worker.py deleted file mode 100644 index 4b349b05..00000000 --- a/dev/tsim_dev/_worker.py +++ /dev/null @@ -1,153 +0,0 @@ -"""Measure one circuit in a fresh process and print one JSON line. - -This file is run by path with the Python of the checkout under test, which may be -an older Tsim, so it uses only the standard library, numpy and Tsim's public API. -""" - -from __future__ import annotations - -import argparse -import json -import time - - -def phase_import(): - """Import Tsim (a named frame, so profiles can tell the phases apart).""" - import tsim - - return tsim - - -def phase_compile(tsim, args): - """Parse the circuit and compile the sampler.""" - circuit = tsim.Circuit.from_file(args.circuit) - kind = args.sampler - if kind == "auto": - has_outputs = circuit.num_detectors + circuit.num_observables > 0 - kind = "detector" if has_outputs else "measurement" - kwargs = {"seed": args.seed} - if args.strategy: - kwargs["strategy"] = args.strategy - if kind == "detector": - sampler = circuit.compile_detector_sampler(**kwargs) - else: - sampler = circuit.compile_sampler(**kwargs) - return circuit, kind, sampler - - -def make_sample(circuit, kind, sampler, args): - """Return a function that samples a number of shots with the requested options.""" - import numpy as np - - kwargs = {} - if args.batch_size: - kwargs["batch_size"] = args.batch_size - if args.bit_packed: - kwargs["bit_packed"] = True - if args.postselect == "all": - if kind != "detector": - raise ValueError("postselection needs the detector sampler") - kwargs["postselection_mask"] = np.ones(circuit.num_detectors, dtype=np.bool_) - - def sample(shots): - return sampler.sample(shots, **kwargs) - - return sample - - -def phase_sample(sample, shots, seconds): - """Sample repeatedly for about `seconds` (the phase that profiles look at).""" - end = time.perf_counter() + seconds - while time.perf_counter() < end: - sample(shots) - - -def calibrate(sample, min_seconds, max_shots): - """Find a shot count and a number of calls that together take `min_seconds`. - - The shot count doubles until one call is long enough. It stops at `max_shots`, - which bounds the size of the returned array; faster circuits are timed over - several calls instead. - """ - shots = 16 - while True: - start = time.perf_counter() - sample(shots) - elapsed = time.perf_counter() - start - if elapsed >= min_seconds: - return shots, 1 - if shots >= max_shots: - return shots, max(1, round(min_seconds / max(elapsed, 1e-6))) - grow = min(16.0, max(2.0, 1.2 * min_seconds / max(elapsed, 1e-6))) - shots = min(max_shots, int(shots * grow)) - - -def main(): - """Run the requested mode and print `RESULT `.""" - parser = argparse.ArgumentParser() - parser.add_argument("--circuit", required=True) - parser.add_argument("--mode", default="bench") # bench, prime, inspect, profile - parser.add_argument("--sampler", default="auto") - parser.add_argument("--postselect", default="none") - parser.add_argument("--strategy", default="") - parser.add_argument("--seed", type=int, default=1) - parser.add_argument("--batch-size", type=int, default=0) - parser.add_argument("--bit-packed", action="store_true") - parser.add_argument("--min-seconds", type=float, default=0.5) - parser.add_argument("--repeats", type=int, default=3) - args = parser.parse_args() - - start = time.perf_counter() - tsim = phase_import() - imported = time.perf_counter() - circuit, kind, sampler = phase_compile(tsim, args) - compiled = time.perf_counter() - - result = { - "tsim_version": getattr(tsim, "__version__", "?"), - "sampler": kind, - "import_s": imported - start, - "compile_s": compiled - imported, - "active_width": getattr(sampler, "active_width", None), - "qubits": circuit.num_qubits, - "detectors": circuit.num_detectors, - "measurements": circuit.num_measurements, - "tcount": circuit.tcount(), - } - if args.mode == "inspect": - result["stats"] = str(getattr(sampler, "stats", "")) - print("RESULT " + json.dumps(result)) - return - - sample = make_sample(circuit, kind, sampler, args) - warm = time.perf_counter() - sample(16) - result["warmup_s"] = time.perf_counter() - warm - if args.mode == "prime": - print("RESULT " + json.dumps(result)) - return - - outputs = circuit.num_detectors + circuit.num_observables - if kind == "measurement": - outputs = circuit.num_measurements - # NOTE: one call returns at most about 256 MB of unpacked samples. - shots, calls = calibrate(sample, args.min_seconds, (1 << 28) // max(1, outputs)) - if args.mode == "profile": - phase_sample(sample, shots, args.min_seconds * 10) - result["shots"] = shots - print("RESULT " + json.dumps(result)) - return - - times = [] - for _ in range(args.repeats): - start = time.perf_counter() - for _ in range(calls): - sample(shots) - times.append(time.perf_counter() - start) - total = shots * calls - result.update(shots=total, seconds=min(times), shots_per_s=total / min(times)) - print("RESULT " + json.dumps(result)) - - -if __name__ == "__main__": - main() diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py index a220a083..bba0da5c 100644 --- a/dev/tsim_dev/bench.py +++ b/dev/tsim_dev/bench.py @@ -14,22 +14,28 @@ import statistics import subprocess import sys -import tomllib from dataclasses import dataclass from pathlib import Path -from . import common +from . import common, corpus from .common import FAILED, INFRA, OK, InfraError @dataclass(frozen=True) class Case: - """One circuit with the sampler options it is measured with.""" + """One circuit with the sampler options it is measured with. + + A corpus circuit (`id` set) is postselected as its tsim-bench manifest says, unless + `postselect` is "none". A `.stim` path outside the corpus uses `sampler` and + `postselect` ("all" or "none") from the command line. + """ name: str path: Path sampler: str = "auto" - postselect: str = "none" + postselect: str = "none" # "recorded", "all" or "none" + id: str | None = None + width: int | None = None @dataclass(frozen=True) @@ -40,77 +46,67 @@ class Side: python: Path -def registry() -> dict[str, tuple[Case, str]]: - """Return the named circuits in `dev/circuits/circuits.toml` with their suite.""" - with open(common.CIRCUITS_DIR / "circuits.toml", "rb") as file: - table = tomllib.load(file) - return { - name: ( - Case( - name, - common.CIRCUITS_DIR / entry["file"], - entry.get("sampler", "auto"), - entry.get("postselect", "none"), - ), - entry.get("suite", "full"), - ) - for name, entry in table.items() - } - - -def corpus_dir(explicit: str | None = None) -> Path: - """Return the tsim-bench checkout to read circuits from.""" - for candidate in ( - explicit, - os.environ.get("TSIM_BENCH_DIR"), - common.cache_dir() / "tsim-bench", - ): - if candidate and (Path(candidate).expanduser() / "papers").is_dir(): - return Path(candidate).expanduser() - raise InfraError( - "no tsim-bench corpus: run `tsim-dev corpus fetch`, or set TSIM_BENCH_DIR " - "to a clone of https://github.com/rafaelha/tsim-bench" +def _from_corpus(entry, args) -> Case: + postselect = ( + "none" if args.no_postselect or entry.postselect == "none" else "recorded" + ) + return Case( + entry.id.split("/", 1)[1], + entry.path, + entry.sampler, + postselect, + entry.id, + entry.width(args.width_of), ) -def corpus_cases(directory: Path, max_k: int) -> list[Case]: - """Return the corpus circuits whose recorded active width is at most `max_k`.""" - cases = [] - for record in sorted(directory.glob("papers/*/k_max.json")): - for entry in json.loads(record.read_text()): - k = entry.get("tsim_k") - path = record.parent / entry["file"] - if isinstance(k, int) and k <= max_k and path.exists(): - cases.append(Case(f"{record.parent.name[:10]}/{path.stem}", path)) - return cases +def _lookup(item: str, everything: dict) -> object | None: + """Return the corpus entry with this id, or the only one whose file stem is `item`.""" + if item in everything: + return everything[item] + matches = [entry for key, entry in everything.items() if key.endswith("/" + item)] + if len(matches) > 1: + raise InfraError( + f"{item!r} is ambiguous: " + ", ".join(entry.id for entry in matches) + ) + return matches[0] if matches else None def select(args) -> list[Case]: """Turn the command line into the list of cases to measure.""" - named = registry() - if args.corpus is not None: - cases = corpus_cases(corpus_dir(args.corpus or None), args.max_k) - elif args.circuits: + bench_pkg = corpus.package(args.corpus_dir) + if args.circuits: + everything = bench_pkg.load(corpus.root(args.corpus_dir)) cases = [] for item in args.circuits: - if item in named: - cases.append(named[item][0]) + entry = _lookup(item, everything) + if entry is not None: + cases.append(_from_corpus(entry, args)) elif Path(item).is_file(): path = Path(item).resolve() cases.append(Case(path.stem, path, args.sampler, args.postselect)) else: raise InfraError( - f"unknown circuit {item!r}: give a .stim path or one of " - + ", ".join(named) + f"unknown circuit {item!r}: give a .stim path or a tsim-bench id " + "(`tsim-dev corpus list`)" ) else: + try: + entries = bench_pkg.cases( + args.suite, width_of=args.width_of, root=corpus.root(args.corpus_dir) + ) + except KeyError as error: + raise InfraError(str(error.args[0])) from None + cases = [_from_corpus(entry, args) for entry in entries] + if args.max_width is not None: cases = [ case - for case, suite in named.values() - if args.suite == "full" or suite == "quick" + for case in cases + if case.id is None + or (case.width is not None and case.width <= args.max_width) ] if args.only: - cases = [case for case in cases if args.only in case.name] + cases = [case for case in cases if args.only in (case.id or case.name)] if not cases: raise InfraError("no circuits selected") return cases @@ -157,28 +153,29 @@ def reference(ref: str) -> Side: return Side(f"{ref}@{sha[:8]}", python) -def measure(side: Side, case: Case, args, mode: str = "bench") -> dict: - """Run the worker once and return its result, or `{"error": ...}`.""" - cmd = [ - str(side.python), - str(common.WORKER), - "--circuit", - str(case.path), - "--mode", - mode, - "--sampler", - case.sampler, - "--postselect", - case.postselect, - "--min-seconds", - str(args.min_seconds), - ] +def worker_command(python: Path, case: Case, args, mode: str) -> list[str]: + """Return the command that times `case` with tsim-bench's worker.""" + cmd = [str(python), str(corpus.worker(args.corpus_dir)), "--mode", mode] + if case.id is not None: + cmd += ["--root", str(corpus.root(args.corpus_dir)), "--case", case.id] + if case.postselect == "none": + cmd.append("--no-postselect") + else: + cmd += ["--circuit", str(case.path), "--sampler", case.sampler] + cmd += ["--postselect", case.postselect] + cmd += ["--min-seconds", str(args.min_seconds)] if args.strategy: cmd += ["--strategy", args.strategy] if args.batch_size: cmd += ["--batch-size", str(args.batch_size)] if args.bit_packed: cmd.append("--bit-packed") + return cmd + + +def measure(side: Side, case: Case, args, mode: str = "bench") -> dict: + """Run the worker once and return its result, or `{"error": ...}`.""" + cmd = worker_command(side.python, case, args, mode) wrapped = common.capped( cmd, mem=args.mem, timeout=args.timeout, pin=common.pin_cpus(args.threads) ) @@ -219,7 +216,12 @@ def compare(new: list[float], ref: list[float], tolerance: float) -> dict: def run_case(case: Case, sides: list[Side], args) -> dict: """Prime the caches, then measure every side in interleaved rounds.""" rounds: dict[str, list[dict]] = {side.label: [] for side in sides} - record: dict = {"circuit": case.name, "path": str(case.path)} + record: dict = { + "circuit": case.name, + "id": case.id, + "path": str(case.path), + "postselect": case.postselect, + } for side in sides: primed = measure(side, case, args, mode="prime") if "error" in primed: @@ -236,6 +238,9 @@ def run_case(case: Case, sides: list[Side], args) -> dict: record["rounds"] = rounds new = rounds[sides[0].label] record["k"] = new[0].get("active_width") + if record["k"] is None: # Tsim before 1.1 does not report it; use the recorded one + record["k"] = case.width + record["acceptance"] = new[0].get("acceptance") record["shots_per_s"] = statistics.median(r["shots_per_s"] for r in new) record["compile_s"] = statistics.median(r["compile_s"] for r in new) rates = [r["shots_per_s"] for r in new] @@ -257,13 +262,20 @@ def run_case(case: Case, sides: list[Side], args) -> dict: return record +NAME = 44 + + def format_row(record: dict, has_ref: bool) -> str: """Format one result line.""" - name = record["circuit"][:38] + name = record["circuit"][:NAME] if "error" in record: - return f"{name:<38} ERROR: {record['error']}" + return f"{name:<{NAME}} ERROR: {record['error']}" k = "-" if record["k"] is None else record["k"] - row = f"{name:<38} {k!s:>3} {common.si(record['shots_per_s']):>8}" + accepted = record.get("acceptance") + kept = ( + "-" if record["postselect"] == "none" or accepted is None else f"{accepted:.3g}" + ) + row = f"{name:<{NAME}} {k!s:>3} {common.si(record['shots_per_s']):>8} {kept:>6}" row += f" {record['compile_s']:>8.2f}" if not has_ref: return row + f" ±{record['spread'] * 50:.0f}%" @@ -290,10 +302,12 @@ def run(args) -> int: header += f" ref={sides[1].label}" busy = common.busy_cores() header += f" threads={args.threads} rounds={rounds} busy_cores={busy:.1f}" + header += f" postselect={'off' if args.no_postselect else 'on'}" + header += f" tsim-bench={corpus.describe(args.corpus_dir)}" print(header) if busy > 1.0: print("WARNING: other processes are using the CPU; expect noisy numbers") - columns = f"{'circuit':<38} {'k':>3} {'shots/s':>8} {'compile':>8}" + columns = f"{'circuit':<{NAME}} {'k':>3} {'shots/s':>8} {'kept':>6} {'compile':>8}" if has_ref: columns += " shots/s vs ref (min, max) compile vs ref" print(columns, flush=True) @@ -313,6 +327,8 @@ def run(args) -> int: "tolerance": args.tolerance, "busy_cores": busy, "cpu_count": os.cpu_count(), + "postselect": not args.no_postselect, + "tsim_bench": corpus.describe(args.corpus_dir), } out.write_text(json.dumps({"meta": meta, "results": records}, indent=1)) @@ -336,16 +352,31 @@ def run(args) -> int: return FAILED if slower else OK -def fetch_corpus(_args) -> int: - """Clone or update the tsim-bench corpus in the shared cache.""" - target = common.cache_dir() / "tsim-bench" - if (target / ".git").is_dir(): - cmd = ["git", "-C", str(target), "pull", "--ff-only", "-q"] - else: - url = "https://github.com/rafaelha/tsim-bench" - cmd = ["git", "clone", "--depth", "1", "-q", url, str(target)] - if subprocess.run(cmd, check=False).returncode: - common.die("could not fetch the tsim-bench corpus") - count = len(corpus_cases(target, 10**6)) - print(f"corpus at {target}: {count} circuits with a recorded active width") +def corpus_command(args) -> int: + """`tsim-dev corpus fetch|list`: fetch the pinned tsim-bench, or list circuits.""" + try: + root = corpus.root(args.corpus_dir) + bench_pkg = corpus.package(args.corpus_dir) + entries = bench_pkg.cases( + args.suite, + max_width=args.max_width, + width_of=args.width_of, + only=args.only or None, + root=root, + ) + except (InfraError, KeyError) as error: + common.die(str(error.args[0])) + if args.action == "fetch": + print( + f"tsim-bench {corpus.describe(args.corpus_dir)} at {root}: {len(entries)} circuits" + ) + return OK + for entry in entries: + masked = len(bench_pkg.mask_indices(entry)) + width = entry.width(args.width_of) + print( + f"{entry.id:<80} k={'-' if width is None else width!s:>3} " + f"arXiv:{entry.arxiv:<10} postselect {masked}/{entry.num_detectors}" + ) + print(f"{len(entries)} circuits") return OK diff --git a/dev/tsim_dev/cli.py b/dev/tsim_dev/cli.py index 6e924cc1..978c45e9 100644 --- a/dev/tsim_dev/cli.py +++ b/dev/tsim_dev/cli.py @@ -23,11 +23,55 @@ def _caps(parser, mem: str, timeout: int, cpus: float | None = None) -> None: ) +def _corpus_dir(parser) -> None: + parser.add_argument( + "--corpus-dir", + default=None, + help="local tsim-bench checkout (default: TSIM_BENCH_DIR, else the pinned commit)", + ) + + +def _selection(parser, suite: str) -> None: + parser.add_argument( + "--suite", + default=suite, + help=f"tsim-bench suite, or `corpus` for every circuit (default {suite})", + ) + parser.add_argument( + "--max-width", + type=int, + default=None, + help="skip circuits whose recorded active width is larger", + ) + parser.add_argument( + "--width-of", + default="tsim", + help="backend whose recorded width --max-width uses (default tsim)", + ) + parser.add_argument( + "--only", default="", help="keep circuits whose id contains this" + ) + _corpus_dir(parser) + + def _circuit_options(parser) -> None: parser.add_argument( - "--sampler", default="auto", choices=["auto", "detector", "measurement"] + "--no-postselect", + action="store_true", + help="ignore the postselection recorded in tsim-bench", + ) + parser.add_argument( + "--sampler", + default="auto", + choices=["auto", "detector", "measurement"], + help="for a .stim path outside tsim-bench", + ) + parser.add_argument( + "--postselect", + default="none", + choices=["none", "all"], + help="for a .stim path outside tsim-bench", ) - parser.add_argument("--postselect", default="none", choices=["none", "all"]) parser.add_argument( "--threads", type=int, default=1, help="numba threads (default 1)" ) @@ -50,17 +94,11 @@ def build_parser() -> argparse.ArgumentParser: p.set_defaults(func=checks.test) p = sub.add_parser("bench", help="throughput and compile time, optionally vs a ref") - p.add_argument("circuits", nargs="*", help="registry names or .stim paths") - p.add_argument("--ref", help="git revision or checkout path to compare against") - p.add_argument("--suite", default="quick", choices=["quick", "full"]) p.add_argument( - "--corpus", - nargs="?", - const="", - help="use the tsim-bench corpus (optionally its path) instead of the registry", + "circuits", nargs="*", help="tsim-bench ids or file stems, or .stim paths" ) - p.add_argument("--max-k", type=int, default=12, help="corpus: largest active width") - p.add_argument("--only", default="", help="keep circuits whose name contains this") + p.add_argument("--ref", help="git revision or checkout path to compare against") + _selection(p, "quick") p.add_argument("--rounds", type=int, default=0, help="default 5 with --ref, else 3") p.add_argument( "--min-seconds", type=float, default=0.5, help="length of a timed call" @@ -73,13 +111,17 @@ def build_parser() -> argparse.ArgumentParser: _caps(p, "8G", 600) p.set_defaults(func=bench.run) - p = sub.add_parser("corpus", help="fetch the tsim-bench circuit corpus") - p.add_argument("action", choices=["fetch"]) - p.set_defaults(func=bench.fetch_corpus) + p = sub.add_parser( + "corpus", help="fetch the pinned tsim-bench, or list its circuits" + ) + p.add_argument("action", choices=["fetch", "list"]) + _selection(p, "corpus") + p.set_defaults(func=bench.corpus_command) p = sub.add_parser("inspect", help="size and active width of a circuit") - p.add_argument("circuit") + p.add_argument("circuit", help="tsim-bench id or file stem, or a .stim path") _circuit_options(p) + _corpus_dir(p) _caps(p, "8G", 600) p.set_defaults(func=profile.inspect) diff --git a/dev/tsim_dev/common.py b/dev/tsim_dev/common.py index f499966f..36d5a575 100644 --- a/dev/tsim_dev/common.py +++ b/dev/tsim_dev/common.py @@ -12,8 +12,6 @@ from typing import NoReturn DEV_DIR = Path(__file__).resolve().parent.parent -CIRCUITS_DIR = DEV_DIR / "circuits" -WORKER = Path(__file__).resolve().parent / "_worker.py" # Exit codes shared by every subcommand. OK, FAILED, INFRA, RUNNING = 0, 1, 2, 3 diff --git a/dev/tsim_dev/corpus.py b/dev/tsim_dev/corpus.py new file mode 100644 index 00000000..e29a3bf8 --- /dev/null +++ b/dev/tsim_dev/corpus.py @@ -0,0 +1,86 @@ +"""The benchmark circuits: the tsim-bench repository at a pinned commit. + +tsim-bench (https://github.com/rafaelha/tsim-bench) holds every benchmark circuit with the +paper it comes from, how that paper postselects, and its recorded active width per backend. +Its `tsim_bench` package (standard library only) reads that, and its worker times one +circuit. The pinned commit is exported once into the shared cache; set TSIM_BENCH_DIR, or +pass `--corpus-dir`, to use a local checkout instead (when changing tsim-bench itself). +""" + +from __future__ import annotations + +import functools +import importlib +import os +import shutil +import subprocess +import sys +from pathlib import Path + +from . import common +from .common import InfraError + +URL = "https://github.com/rafaelha/tsim-bench" +# Bump together with any change that needs newer circuits or manifests. +REV = "f0052960949ff306bc330ad82707d48c4eeab5ef" + + +def _export(rev: str) -> Path: + """Export `rev` of tsim-bench into the cache once and return the directory.""" + target = common.cache_dir() / "tsim-bench" / rev[:12] + if (target / "tsim_bench").is_dir(): + return target + clone = common.cache_dir() / "tsim-bench.git" + if not clone.is_dir(): + print(f"fetching {URL} into {clone} ...", flush=True) + common.git("clone", "--bare", "-q", URL, str(clone)) + try: + common.git("cat-file", "-e", f"{rev}^{{commit}}", cwd=clone) + except InfraError: + common.git("fetch", "-q", "origin", "+refs/heads/*:refs/heads/*", cwd=clone) + shutil.rmtree(target, ignore_errors=True) + target.mkdir(parents=True) + archive = subprocess.Popen( + ["git", "archive", rev], cwd=clone, stdout=subprocess.PIPE + ) + subprocess.run(["tar", "-x", "-C", str(target)], stdin=archive.stdout, check=True) + if archive.wait(): + shutil.rmtree(target, ignore_errors=True) + raise InfraError(f"git archive {rev} of tsim-bench failed") + return target + + +@functools.cache +def root(explicit: str | None = None) -> Path: + """Return the tsim-bench checkout to read circuits from, fetching it if needed.""" + for candidate in (explicit, os.environ.get("TSIM_BENCH_DIR")): + if candidate: + path = Path(candidate).expanduser().resolve() + if not (path / "tsim_bench").is_dir(): + raise InfraError( + f"{path} is not a tsim-bench checkout (no tsim_bench/)" + ) + return path + return _export(REV) + + +@functools.cache +def package(explicit: str | None = None): + """Import `tsim_bench` from the checkout and return the module.""" + path = str(root(explicit)) + if path not in sys.path: + sys.path.insert(0, path) + return importlib.import_module("tsim_bench") + + +def worker(explicit: str | None = None) -> Path: + """Return the path of the timing worker of the checkout.""" + return root(explicit) / "tsim_bench" / "worker.py" + + +def describe(explicit: str | None = None) -> str: + """Name the tsim-bench in use: the pinned commit, or a local checkout's path.""" + if explicit or os.environ.get("TSIM_BENCH_DIR"): + path = root(explicit) + return f"{common.describe(path)} ({path})" + return REV[:8] diff --git a/dev/tsim_dev/profile.py b/dev/tsim_dev/profile.py index b93b2bbd..4428c2ef 100644 --- a/dev/tsim_dev/profile.py +++ b/dev/tsim_dev/profile.py @@ -18,7 +18,7 @@ def _case(args) -> bench.Case: - args.corpus, args.only, args.suite = None, "", "quick" + args.only, args.suite, args.max_width, args.width_of = "", "quick", None, "tsim" args.circuits = [args.circuit] return bench.select(args)[0] @@ -104,20 +104,7 @@ def profile(args) -> int: common.out_dir("profile") / f"{case.name.replace('/', '_')}-{common.stamp()}.folded" ) - worker = [ - sys.executable, - str(common.WORKER), - "--circuit", - str(case.path), - "--mode", - "profile", - "--sampler", - case.sampler, - "--postselect", - case.postselect, - "--min-seconds", - str(args.min_seconds), - ] + worker = bench.worker_command(Path(sys.executable), case, args, "profile") spy_cmd = [str(spy), "record", "--native", "-f", "raw", "-r", str(args.rate)] cmd = common.capped( [*spy_cmd, "-o", str(raw), "--", *worker], diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index 9ef9b414..04957b29 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -6,21 +6,112 @@ from pathlib import Path import pytest -from tsim_dev import bench, checks, cli, common +from tsim_dev import bench, checks, cli, common, corpus ROOT = Path(__file__).resolve().parents[2] HOOK = ROOT / ".claude" / "hooks" / "guard_bash.py" -def test_registry_circuits_exist_and_the_quick_suite_is_small(): - registry = bench.registry() - assert all(case.path.is_file() for case, _ in registry.values()) - assert all( - case.sampler in ("detector", "measurement") for case, _ in registry.values() - ) - assert all(case.postselect in ("all", "none") for case, _ in registry.values()) - quick = [name for name, (_, suite) in registry.items() if suite == "quick"] - assert 4 <= len(quick) <= 8 +FAKE_BENCH = """ +from dataclasses import dataclass, field +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent + + +@dataclass(frozen=True) +class Case: + id: str + path: Path + sampler: str + postselect: str + num_detectors: int = 2 + arxiv: str = "0000.00000" + widths: dict = field(default_factory=dict, compare=False) + + def width(self, backend="tsim"): + return self.widths.get(backend) + + +def load(root=ROOT): + stim = root / "papers" / "p" / "stim" + return { + "p/narrow": Case("p/narrow", stim / "narrow.stim", "detector", "all", widths={"tsim": 4, "clifft": 9}), + "p/wide": Case("p/wide", stim / "wide.stim", "detector", "none", widths={"tsim": 26}), + } + + +def mask_indices(case): + return list(range(case.num_detectors)) if case.postselect == "all" else [] + + +def cases(suite=None, *, max_width=None, width_of="tsim", only=None, root=ROOT): + selected = list(load(root).values()) + if suite not in (None, "corpus"): + if suite != "quick": + raise KeyError(f"unknown suite {suite!r}") + selected = selected[:1] + if max_width is not None: + selected = [c for c in selected if (c.width(width_of) or 10**9) <= max_width] + return selected +""" + + +@pytest.fixture +def fake_bench(tmp_path, monkeypatch): + package = tmp_path / "tsim_bench" + package.mkdir() + (package / "__init__.py").write_text(FAKE_BENCH) + monkeypatch.setenv("TSIM_BENCH_DIR", str(tmp_path)) + monkeypatch.syspath_prepend(str(tmp_path)) + monkeypatch.delitem(sys.modules, "tsim_bench", raising=False) + for cached in (corpus.root, corpus.package): + cached.cache_clear() + yield tmp_path + for cached in (corpus.root, corpus.package): + cached.cache_clear() + + +def _bench_args(*argv): + return cli.build_parser().parse_args(["bench", *argv]) + + +def test_corpus_circuits_are_postselected_unless_turned_off(fake_bench): + (case,) = bench.select(_bench_args()) # the quick suite + assert (case.id, case.postselect, case.width) == ("p/narrow", "recorded", 4) + command = bench.worker_command(Path("python"), case, _bench_args(), "bench") + assert command[command.index("--case") + 1] == "p/narrow" + assert "--no-postselect" not in command + + (case,) = bench.select(_bench_args("--no-postselect")) + assert case.postselect == "none" + command = bench.worker_command(Path("python"), case, _bench_args(), "bench") + assert "--no-postselect" in command + + +def test_max_width_uses_the_recorded_width_of_one_backend(fake_bench): + everything = bench.select(_bench_args("--suite", "corpus")) + assert [case.id for case in everything] == ["p/narrow", "p/wide"] + narrow = bench.select(_bench_args("--suite", "corpus", "--max-width", "8")) + assert [case.id for case in narrow] == ["p/narrow"] + by_clifft = ["--suite", "corpus", "--max-width", "8", "--width-of", "clifft"] + with pytest.raises(common.InfraError, match="no circuits"): + bench.select(_bench_args(*by_clifft)) + + +def test_circuits_are_named_by_id_file_stem_or_path(fake_bench, tmp_path): + assert [c.id for c in bench.select(_bench_args("wide"))] == ["p/wide"] + assert [c.id for c in bench.select(_bench_args("p/narrow"))] == ["p/narrow"] + stim = tmp_path / "mine.stim" + stim.write_text("H 0\nM 0\n") + (case,) = bench.select(_bench_args(str(stim), "--postselect", "all")) + assert (case.id, case.postselect) == (None, "all") + command = bench.worker_command(Path("python"), case, _bench_args(), "bench") + assert command[command.index("--circuit") + 1] == str(stim) + with pytest.raises(common.InfraError, match="unknown circuit"): + bench.select(_bench_args("nope")) + with pytest.raises(common.InfraError, match="unknown suite"): + bench.select(_bench_args("--suite", "nope")) @pytest.mark.parametrize( From 521f5d4d111be37de4ad35257855b9a5591db1b1 Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 08:08:25 -0700 Subject: [PATCH 03/11] chore: compare sides with their own threads, device or strategy in tsim-dev bench `--side REF[:threads=N,device=cpu|gpu,strategy=NAME]`, given twice, replaces `--ref` when the sides differ in more than the checkout: 10 cores against 1, the GPU executor against the CPU, or two strategies. REF is `.`, a git revision, a checkout or a Python executable (an environment built elsewhere). `device=cpu` hides CUDA devices; `device=gpu` fails unless the worker reports Tsim's CUDA executor usable (tsim-bench 4cb8ca2). Co-Authored-By: Claude Opus 5.5 --- .claude/skills/benchmarking-tsim/SKILL.md | 19 ++++ dev/tsim_dev/bench.py | 120 ++++++++++++++++++---- dev/tsim_dev/cli.py | 10 ++ dev/tsim_dev/corpus.py | 2 +- test/unit/test_dev_cli.py | 33 ++++++ 5 files changed, 164 insertions(+), 20 deletions(-) diff --git a/.claude/skills/benchmarking-tsim/SKILL.md b/.claude/skills/benchmarking-tsim/SKILL.md index 34b396bb..7715a7b0 100644 --- a/.claude/skills/benchmarking-tsim/SKILL.md +++ b/.claude/skills/benchmarking-tsim/SKILL.md @@ -36,6 +36,25 @@ inject_cultivate_d5_p0.001 6 13.9M 0.141 0.13 1.20x [1.19, 1.21] fast 2: a circuit could not be measured. - The JSON under `.tsim-dev/bench/` has every round. +## Compare settings: threads, GPU, strategy + +`--side REF[:key=value,...]`, given twice, replaces `--ref` when the two sides differ in +more than the checkout. REF is `.` (this checkout), a git revision, a checkout path, or +a Python executable (an environment built elsewhere, such as one with the GPU extra). +Keys: `threads=N`, `device=cpu|gpu`, `strategy=NAME`. The first side is "new". + +```bash +uv run tsim-dev bench --side .:threads=10 --side .:threads=1 # 10 cores vs 1 +uv run tsim-dev bench --side $GPU_PY:device=gpu --side $GPU_PY:device=cpu +uv run tsim-dev bench --side .:threads=4 --side origin/main:threads=4 +``` + +- `device=cpu` hides CUDA devices; `device=gpu` fails the circuit unless the worker finds + Tsim's CUDA executor usable. Circuits above the executor's width limit still fall back + to the CPU on that side, so check the width before reading a GPU ratio. +- Compile time is compared too; across devices a "slower" compile verdict (and exit code + 1) is expected, so read the rate column. + ## What the command controls for you - One thread, pinned to a core (`--threads N` to change; say so when you report). diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py index bba0da5c..90a9bad3 100644 --- a/dev/tsim_dev/bench.py +++ b/dev/tsim_dev/bench.py @@ -7,6 +7,7 @@ from __future__ import annotations +import argparse import json import math import os @@ -40,10 +41,63 @@ class Case: @dataclass(frozen=True) class Side: - """One checkout under test: a label and the Python that imports its Tsim.""" + """One side of a comparison: the Python that imports its Tsim, and its settings. + + `threads` and `strategy` override the command line's when set. `device` is "" (the + environment decides), "cpu" (CUDA devices hidden) or "gpu" (the run fails unless the + worker finds Tsim's CUDA executor usable). + """ label: str python: Path + threads: int | None = None + device: str = "" + strategy: str | None = None + + def describe(self) -> str: + """Return the label with the settings that this side overrides.""" + extra = [f"threads={self.threads}"] if self.threads is not None else [] + extra += [f"device={self.device}"] if self.device else [] + extra += [f"strategy={self.strategy}"] if self.strategy is not None else [] + return " ".join([self.label, *extra]) + + +SIDE_KEYS = ("threads", "device", "strategy") + + +def parse_side(spec: str) -> Side: + """Parse `--side REF[:key=value,...]`. + + REF is `.` (this checkout), a git revision, the path of another checkout with a + `.venv`, or the path of a Python executable (an environment built elsewhere, such + as one with a GPU extra). Keys: threads=N, device=cpu|gpu, strategy=NAME. + """ + ref, _, options = spec.partition(":") + settings: dict = {} + for item in filter(None, options.split(",")): + key, sep, value = item.partition("=") + if not sep or key not in SIDE_KEYS: + raise InfraError( + f"bad --side option {item!r}: use " + + ", ".join(f"{k}=..." for k in SIDE_KEYS) + ) + settings[key] = value + if "threads" in settings: + if not settings["threads"].isdigit() or int(settings["threads"]) < 1: + raise InfraError(f"bad thread count in --side {spec!r}") + settings["threads"] = int(settings["threads"]) + if settings.get("device", "cpu") not in ("cpu", "gpu"): + raise InfraError(f"device must be cpu or gpu in --side {spec!r}") + if ref in ("", "."): + base = Side(common.describe(common.root()), Path(sys.executable)) + elif Path(ref).expanduser().is_file(): + # NOTE: not resolved: a venv's python is a symlink to the base interpreter, + # and resolving it would lose the environment. + python = Path(ref).expanduser().absolute() + base = Side(str(python), python) + else: + base = reference(ref) + return Side(base.label, base.python, **settings) def _from_corpus(entry, args) -> Case: @@ -173,22 +227,39 @@ def worker_command(python: Path, case: Case, args, mode: str) -> list[str]: return cmd +def side_env(side: Side, threads: int) -> dict[str, str]: + """Return the environment of one side's worker.""" + env = common.child_env(threads) + if side.device == "cpu": + env["CUDA_VISIBLE_DEVICES"] = "" + return env + + def measure(side: Side, case: Case, args, mode: str = "bench") -> dict: """Run the worker once and return its result, or `{"error": ...}`.""" - cmd = worker_command(side.python, case, args, mode) + threads = side.threads or args.threads + options = args + if side.strategy is not None: + options = argparse.Namespace(**{**vars(args), "strategy": side.strategy}) + cmd = worker_command(side.python, case, options, mode) wrapped = common.capped( - cmd, mem=args.mem, timeout=args.timeout, pin=common.pin_cpus(args.threads) + cmd, mem=args.mem, timeout=args.timeout, pin=common.pin_cpus(threads) ) done = subprocess.run( wrapped, - env=common.child_env(args.threads), + env=side_env(side, threads), capture_output=True, text=True, check=False, ) for line in done.stdout.splitlines(): if line.startswith("RESULT "): - return json.loads(line[7:]) + result = json.loads(line[7:]) + if side.device == "gpu" and not result.get("cuda"): + return { + "error": "device=gpu, but this Tsim has no usable CUDA executor" + } + return result reason = common.explain_exit(done.returncode, args.mem, args.timeout) last = (done.stderr.strip().splitlines() or [""])[-1] return {"error": f"{reason}: {last}"[:200]} @@ -215,7 +286,7 @@ def compare(new: list[float], ref: list[float], tolerance: float) -> dict: def run_case(case: Case, sides: list[Side], args) -> dict: """Prime the caches, then measure every side in interleaved rounds.""" - rounds: dict[str, list[dict]] = {side.label: [] for side in sides} + rounds: dict[str, list[dict]] = {side.describe(): [] for side in sides} record: dict = { "circuit": case.name, "id": case.id, @@ -225,18 +296,18 @@ def run_case(case: Case, sides: list[Side], args) -> dict: for side in sides: primed = measure(side, case, args, mode="prime") if "error" in primed: - record["error"] = f"{side.label}: {primed['error']}" + record["error"] = f"{side.describe()}: {primed['error']}" return record for index in range(args.rounds): order = sides if index % 2 == 0 else sides[::-1] for side in order: result = measure(side, case, args) if "error" in result: - record["error"] = f"{side.label}: {result['error']}" + record["error"] = f"{side.describe()}: {result['error']}" return record - rounds[side.label].append(result) + rounds[side.describe()].append(result) record["rounds"] = rounds - new = rounds[sides[0].label] + new = rounds[sides[0].describe()] record["k"] = new[0].get("active_width") if record["k"] is None: # Tsim before 1.1 does not report it; use the recorded one record["k"] = case.width @@ -246,7 +317,7 @@ def run_case(case: Case, sides: list[Side], args) -> dict: rates = [r["shots_per_s"] for r in new] record["spread"] = (max(rates) - min(rates)) / record["shots_per_s"] if len(sides) == 2: - old = rounds[sides[1].label] + old = rounds[sides[1].describe()] record["ref_shots_per_s"] = statistics.median(r["shots_per_s"] for r in old) record["ref_compile_s"] = statistics.median(r["compile_s"] for r in old) record["rate"] = compare(rates, [r["shots_per_s"] for r in old], args.tolerance) @@ -288,20 +359,31 @@ def format_row(record: dict, has_ref: bool) -> str: def run(args) -> int: """Run the benchmark and return the exit code.""" try: + if args.side and args.ref: + raise InfraError("--side replaces --ref: give `--side . --side REF`") + if len(args.side) > 2: + raise InfraError("give at most two --side options") cases = select(args) - sides = [Side(common.describe(common.root()), Path(sys.executable))] - if args.ref: - sides.append(reference(args.ref)) + if args.side: + sides = [parse_side(spec) for spec in args.side] + else: + sides = [Side(common.describe(common.root()), Path(sys.executable))] + if args.ref: + sides.append(reference(args.ref)) + if len({side.describe() for side in sides}) < len(sides): + raise InfraError("the two sides are the same; change a setting of one") except InfraError as error: common.die(str(error)) has_ref = len(sides) == 2 rounds = args.rounds or (5 if has_ref else 3) args.rounds = rounds - header = f"bench: new={sides[0].label}" + header = f"bench: new={sides[0].describe()}" if has_ref: - header += f" ref={sides[1].label}" + header += f" ref={sides[1].describe()}" busy = common.busy_cores() - header += f" threads={args.threads} rounds={rounds} busy_cores={busy:.1f}" + if not any(side.threads for side in sides): + header += f" threads={args.threads}" + header += f" rounds={rounds} busy_cores={busy:.1f}" header += f" postselect={'off' if args.no_postselect else 'on'}" header += f" tsim-bench={corpus.describe(args.corpus_dir)}" print(header) @@ -320,8 +402,8 @@ def run(args) -> int: out = common.out_dir("bench") / f"{common.stamp()}.json" meta = { - "new": sides[0].label, - "ref": sides[1].label if has_ref else None, + "new": sides[0].describe(), + "ref": sides[1].describe() if has_ref else None, "threads": args.threads, "rounds": rounds, "tolerance": args.tolerance, diff --git a/dev/tsim_dev/cli.py b/dev/tsim_dev/cli.py index 978c45e9..238070d7 100644 --- a/dev/tsim_dev/cli.py +++ b/dev/tsim_dev/cli.py @@ -98,6 +98,16 @@ def build_parser() -> argparse.ArgumentParser: "circuits", nargs="*", help="tsim-bench ids or file stems, or .stim paths" ) p.add_argument("--ref", help="git revision or checkout path to compare against") + p.add_argument( + "--side", + action="append", + default=[], + metavar="REF[:KEY=VALUE,...]", + help="a side with its own settings, instead of --ref; give two to compare. " + "REF: `.`, a git revision, a checkout or a python executable. " + "Keys: threads=N, device=cpu|gpu, strategy=NAME. " + "Example: --side .:threads=10 --side .:threads=1", + ) _selection(p, "quick") p.add_argument("--rounds", type=int, default=0, help="default 5 with --ref, else 3") p.add_argument( diff --git a/dev/tsim_dev/corpus.py b/dev/tsim_dev/corpus.py index e29a3bf8..6299b2e5 100644 --- a/dev/tsim_dev/corpus.py +++ b/dev/tsim_dev/corpus.py @@ -22,7 +22,7 @@ URL = "https://github.com/rafaelha/tsim-bench" # Bump together with any change that needs newer circuits or manifests. -REV = "f0052960949ff306bc330ad82707d48c4eeab5ef" +REV = "4cb8ca2963b2b982a4e37372b7168b4f590c4e97" def _export(rev: str) -> Path: diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index 04957b29..33acdd89 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -227,3 +227,36 @@ def test_dev_skills_have_valid_frontmatter(): assert front["name"] == path.parent.name assert 0 < len(front["description"]) <= 1024 assert len(text.splitlines()) < 500 + + +def test_sides_carry_their_own_settings(fake_bench): + side = bench.parse_side(".:threads=10,device=cpu,strategy=cat5") + assert (side.python, side.threads, side.device, side.strategy) == ( + Path(sys.executable), + 10, + "cpu", + "cat5", + ) + assert side.describe().endswith("threads=10 device=cpu strategy=cat5") + assert bench.side_env(side, 10)["CUDA_VISIBLE_DEVICES"] == "" + assert bench.side_env(side, 10)["NUMBA_NUM_THREADS"] == "10" + assert "CUDA_VISIBLE_DEVICES" not in bench.side_env( + bench.parse_side(".:device=gpu"), 1 + ) + # A python executable is used as given: resolving a venv's symlink would lose the venv. + python = Path(sys.executable).absolute() + assert bench.parse_side(f"{python}:threads=2").python == python + for bad in (".:cores=2", ".:threads=0", ".:device=tpu", ".:threads"): + with pytest.raises(common.InfraError): + bench.parse_side(bad) + + +def test_a_gpu_side_fails_when_cuda_is_not_used(fake_bench, monkeypatch): + done = subprocess.CompletedProcess([], 0, 'RESULT {"cuda": false}\n', "") + monkeypatch.setattr(bench.subprocess, "run", lambda *a, **k: done) + (case,) = bench.select(_bench_args()) + args = _bench_args() + assert "error" in bench.measure(bench.parse_side(".:device=gpu"), case, args) + assert bench.measure(bench.parse_side(".:device=cpu"), case, args) == { + "cuda": False + } From 04413e4d788e5681ff8997cd599b555e7b4d515b Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 08:22:51 -0700 Subject: [PATCH 04/11] chore: review fixes for the tsim-bench harness - Fetching tsim-bench takes a lock and renames a complete export into place, so overlapping runs share one export and an interrupted one is redone; a pinned commit that is not pushed gives an error, not a tar traceback. - `tsim-dev profile` accepts `--corpus-dir` (it crashed without it). - A corpus circuit given by its path keeps its recorded postselection, and `--no-postselect` also applies to `.stim` paths outside the corpus. - A/B runs compare the two sides' kept fractions: if they differ beyond binomial noise the rate is marked `kept-differs` and the run fails. - `k` falls back to the recorded Tsim width (not the `--width-of` backend's); the JSON records each side's thread count; malformed manifests are harness errors (exit 2). - Pin tsim-bench 88f3b72 (consistent end-to-end cultivation masks). Co-Authored-By: Claude Opus 5.5 --- .claude/skills/benchmarking-tsim/SKILL.md | 11 +++- dev/tsim_dev/bench.py | 64 ++++++++++++++++--- dev/tsim_dev/cli.py | 1 + dev/tsim_dev/corpus.py | 76 ++++++++++++++++------- test/unit/test_dev_cli.py | 74 ++++++++++++++++++++++ 5 files changed, 195 insertions(+), 31 deletions(-) diff --git a/.claude/skills/benchmarking-tsim/SKILL.md b/.claude/skills/benchmarking-tsim/SKILL.md index 7715a7b0..f4417066 100644 --- a/.claude/skills/benchmarking-tsim/SKILL.md +++ b/.claude/skills/benchmarking-tsim/SKILL.md @@ -32,7 +32,11 @@ inject_cultivate_d5_p0.001 6 13.9M 0.141 0.13 1.20x [1.19, 1.21] fast - A verdict of `faster` or `slower` needs the mean outside the tolerance (5%) and every round on the same side of 1. `noisy` means the rounds disagree: run again with `--rounds 9 --min-seconds 1` before you conclude anything. -- Exit code 0: nothing slower. 1: at least one `slower` in rate or compile time. +- `kept-differs`: the two sides kept different fractions of shots (beyond binomial + noise), so they did not sample the same thing and the rate is not compared. It + counts as a failure: a change to postselection or to the sampled distribution. +- Exit code 0: nothing slower. 1: at least one `slower` in rate or compile time, or + a `kept-differs`. 2: a circuit could not be measured. - The JSON under `.tsim-dev/bench/` has every round. @@ -105,8 +109,9 @@ different regime. `--only SUBSTRING` keeps circuits whose id contains it. ## Limits -- CPU only. GPU runs and comparisons with other simulators (Clifft, SymFT, Magna) - are run from the tsim-bench repository, which pins each simulator's environment. +- Tsim only, on the CPU or (with `--side ...:device=gpu`) its CUDA executor. + Comparisons with other simulators (Clifft, SymFT, Magna) are run from the + tsim-bench repository, which pins each simulator's environment. - To add a circuit or change a suite, change tsim-bench: put the file in its paper's folder, add an entry to that folder's `circuits.toml` (arXiv id, sampler, postselection and the reason for it, see its `BENCHMARKS.md`), run `python -m tsim_bench check` diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py index 90a9bad3..cca85fcf 100644 --- a/dev/tsim_dev/bench.py +++ b/dev/tsim_dev/bench.py @@ -36,7 +36,8 @@ class Case: sampler: str = "auto" postselect: str = "none" # "recorded", "all" or "none" id: str | None = None - width: int | None = None + width: int | None = None # recorded width of the --width-of backend + tsim_width: int | None = None # recorded Tsim width @dataclass(frozen=True) @@ -111,13 +112,17 @@ def _from_corpus(entry, args) -> Case: postselect, entry.id, entry.width(args.width_of), + entry.width("tsim"), ) def _lookup(item: str, everything: dict) -> object | None: - """Return the corpus entry with this id, or the only one whose file stem is `item`.""" + """Return the corpus entry with this id or path, or the only one with this file stem.""" if item in everything: return everything[item] + if item.endswith(".stim") and Path(item).is_file(): + path = Path(item).resolve() + return next((e for e in everything.values() if e.path.resolve() == path), None) matches = [entry for key, entry in everything.items() if key.endswith("/" + item)] if len(matches) > 1: raise InfraError( @@ -128,6 +133,13 @@ def _lookup(item: str, everything: dict) -> object | None: def select(args) -> list[Case]: """Turn the command line into the list of cases to measure.""" + try: + return _select(args) + except (ValueError, RuntimeError) as error: # a malformed tsim-bench manifest + raise InfraError(f"tsim-bench: {error}") from None + + +def _select(args) -> list[Case]: bench_pkg = corpus.package(args.corpus_dir) if args.circuits: everything = bench_pkg.load(corpus.root(args.corpus_dir)) @@ -138,7 +150,8 @@ def select(args) -> list[Case]: cases.append(_from_corpus(entry, args)) elif Path(item).is_file(): path = Path(item).resolve() - cases.append(Case(path.stem, path, args.sampler, args.postselect)) + postselect = "none" if args.no_postselect else args.postselect + cases.append(Case(path.stem, path, args.sampler, postselect)) else: raise InfraError( f"unknown circuit {item!r}: give a .stim path or a tsim-bench id " @@ -310,7 +323,7 @@ def run_case(case: Case, sides: list[Side], args) -> dict: new = rounds[sides[0].describe()] record["k"] = new[0].get("active_width") if record["k"] is None: # Tsim before 1.1 does not report it; use the recorded one - record["k"] = case.width + record["k"] = case.tsim_width record["acceptance"] = new[0].get("acceptance") record["shots_per_s"] = statistics.median(r["shots_per_s"] for r in new) record["compile_s"] = statistics.median(r["compile_s"] for r in new) @@ -330,9 +343,37 @@ def run_case(case: Case, sides: list[Side], args) -> dict: ) if abs(record["compile_s"] - record["ref_compile_s"]) < 0.05: record["compile"]["verdict"] = "same" + record["ref_acceptance"] = old[0].get("acceptance") + if not same_acceptance(new, old): + # NOTE: a different kept fraction means the sides sample different + # things, so their rates are not comparable. + record["rate"]["verdict"] = KEPT_DIFFERS return record +KEPT_DIFFERS = "kept-differs" + + +def same_acceptance(new: list[dict], old: list[dict], sigmas: float = 6.0) -> bool: + """Return whether two sides' kept fractions agree within binomial noise. + + Each round reports the kept fraction of `shots` requested shots per timed + repeat; the count over all repeats is larger, so this test is conservative. + """ + + def pooled(rounds: list[dict]) -> tuple[float, float]: + kept = sum(r["acceptance"] * r["shots"] for r in rounds) + total = sum(r["shots"] for r in rounds) + return kept / total, total + + if any(r.get("acceptance") is None for r in new + old): + return True + (p, n), (q, m) = pooled(new), pooled(old) + both = (p * n + q * m) / (n + m) + sigma = math.sqrt(max(both * (1 - both), 1e-12) * (1 / n + 1 / m)) + return abs(p - q) <= sigmas * sigma + 1e-9 + + NAME = 44 @@ -353,6 +394,10 @@ def format_row(record: dict, has_ref: bool) -> str: rate, comp = record["rate"], record["compile"] row += f" {rate['ratio']:.2f}x [{rate['low']:.2f}, {rate['high']:.2f}]" row += f" {rate['verdict']:<6} {comp['ratio']:.2f}x {comp['verdict']}" + if rate["verdict"] == KEPT_DIFFERS: + row += ( + f" (kept {record['acceptance']:.4g} vs ref {record['ref_acceptance']:.4g})" + ) return row @@ -404,7 +449,7 @@ def run(args) -> int: meta = { "new": sides[0].describe(), "ref": sides[1].describe() if has_ref else None, - "threads": args.threads, + "threads": [side.threads or args.threads for side in sides], "rounds": rounds, "tolerance": args.tolerance, "busy_cores": busy, @@ -422,16 +467,21 @@ def run(args) -> int: for key in ("rate", "compile") ] slower = verdicts.count("slower") + differs = verdicts.count(KEPT_DIFFERS) summary = f"SUMMARY: {len(records)} circuits, {errors} errors" if has_ref: summary += ( f", {slower} slower, {verdicts.count('faster')} faster, " f"{verdicts.count('noisy')} noisy (tolerance {args.tolerance:.0%})" ) + if differs: + summary += ( + f", {differs} with a different kept fraction (rates not comparable)" + ) print(f"{summary}. Details: {out}") if errors: return INFRA - return FAILED if slower else OK + return FAILED if slower or differs else OK def corpus_command(args) -> int: @@ -446,7 +496,7 @@ def corpus_command(args) -> int: only=args.only or None, root=root, ) - except (InfraError, KeyError) as error: + except (InfraError, KeyError, ValueError, RuntimeError) as error: common.die(str(error.args[0])) if args.action == "fetch": print( diff --git a/dev/tsim_dev/cli.py b/dev/tsim_dev/cli.py index 238070d7..9ac49a54 100644 --- a/dev/tsim_dev/cli.py +++ b/dev/tsim_dev/cli.py @@ -143,6 +143,7 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument("--rate", type=int, default=500, help="samples per second") p.add_argument("--top", type=int, default=12, help="frames per table") _circuit_options(p) + _corpus_dir(p) _caps(p, "8G", 600) p.set_defaults(func=profile.profile) diff --git a/dev/tsim_dev/corpus.py b/dev/tsim_dev/corpus.py index 6299b2e5..da6ee7bd 100644 --- a/dev/tsim_dev/corpus.py +++ b/dev/tsim_dev/corpus.py @@ -9,12 +9,14 @@ from __future__ import annotations +import fcntl import functools import importlib import os import shutil import subprocess import sys +import tempfile from pathlib import Path from . import common @@ -22,31 +24,63 @@ URL = "https://github.com/rafaelha/tsim-bench" # Bump together with any change that needs newer circuits or manifests. -REV = "4cb8ca2963b2b982a4e37372b7168b4f590c4e97" +REV = "88f3b7264aecb5b7d0e83e0be651e2038e2232cb" +COMPLETE = ".tsim-dev-complete" def _export(rev: str) -> Path: - """Export `rev` of tsim-bench into the cache once and return the directory.""" - target = common.cache_dir() / "tsim-bench" / rev[:12] - if (target / "tsim_bench").is_dir(): + """Export `rev` of tsim-bench into the cache once and return the directory. + + Runs that start together take a lock, and the export is extracted into a + temporary directory and renamed into place, so a reader sees either no + export or a complete one, and a finished export is never deleted. + """ + cache = common.cache_dir() + target = cache / "tsim-bench" / rev[:12] + if (target / COMPLETE).exists(): return target - clone = common.cache_dir() / "tsim-bench.git" - if not clone.is_dir(): - print(f"fetching {URL} into {clone} ...", flush=True) - common.git("clone", "--bare", "-q", URL, str(clone)) - try: - common.git("cat-file", "-e", f"{rev}^{{commit}}", cwd=clone) - except InfraError: - common.git("fetch", "-q", "origin", "+refs/heads/*:refs/heads/*", cwd=clone) - shutil.rmtree(target, ignore_errors=True) - target.mkdir(parents=True) - archive = subprocess.Popen( - ["git", "archive", rev], cwd=clone, stdout=subprocess.PIPE - ) - subprocess.run(["tar", "-x", "-C", str(target)], stdin=archive.stdout, check=True) - if archive.wait(): - shutil.rmtree(target, ignore_errors=True) - raise InfraError(f"git archive {rev} of tsim-bench failed") + target.parent.mkdir(parents=True, exist_ok=True) + with open(cache / "tsim-bench.lock", "w") as lock: + fcntl.flock(lock, fcntl.LOCK_EX) + if (target / COMPLETE).exists(): + return target + clone = cache / "tsim-bench.git" + if not (clone / "HEAD").exists(): + print(f"fetching {URL} into {clone} ...", flush=True) + shutil.rmtree(clone, ignore_errors=True) + common.git("clone", "--bare", "-q", URL, str(clone)) + try: + common.git("cat-file", "-e", f"{rev}^{{commit}}", cwd=clone) + except InfraError: + common.git("fetch", "-q", "origin", "+refs/heads/*:refs/heads/*", cwd=clone) + try: + common.git("cat-file", "-e", f"{rev}^{{commit}}", cwd=clone) + except InfraError: + raise InfraError( + f"tsim-bench has no commit {rev[:12]}: push it, or point " + "--corpus-dir / TSIM_BENCH_DIR at the checkout that has it" + ) from None + staging = Path(tempfile.mkdtemp(prefix=f".{rev[:12]}-", dir=target.parent)) + try: + tarball = staging / "export.tar" + common.git("archive", "-o", str(tarball), rev, cwd=clone) + extract = subprocess.run( + ["tar", "-x", "-f", str(tarball), "-C", str(staging)], + capture_output=True, + text=True, + check=False, + ) + if extract.returncode: + raise InfraError( + f"extracting tsim-bench {rev[:12]}: {extract.stderr.strip()}" + ) + tarball.unlink() + (staging / COMPLETE).touch() + # NOTE: a leftover from an interrupted run before this lock existed. + shutil.rmtree(target, ignore_errors=True) + staging.rename(target) + finally: + shutil.rmtree(staging, ignore_errors=True) return target diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index 33acdd89..cefaf463 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -260,3 +260,77 @@ def test_a_gpu_side_fails_when_cuda_is_not_used(fake_bench, monkeypatch): assert bench.measure(bench.parse_side(".:device=cpu"), case, args) == { "cuda": False } + + +def test_profile_and_inspect_accept_a_corpus_dir(): + for command in ("profile", "inspect"): + args = cli.build_parser().parse_args([command, "qv_q10"]) + assert args.corpus_dir is None + + +def test_paths_follow_the_manifest_and_no_postselect(fake_bench, tmp_path): + stim = tmp_path / "papers" / "p" / "stim" / "narrow.stim" + stim.parent.mkdir(parents=True) + stim.write_text("H 0\nM 0\n") + (case,) = bench.select(_bench_args(str(stim))) + assert (case.id, case.postselect) == ("p/narrow", "recorded") + other = tmp_path / "mine.stim" + other.write_text("H 0\nM 0\n") + args = _bench_args(str(other), "--postselect", "all", "--no-postselect") + assert bench.select(args)[0].postselect == "none" + + +def test_k_falls_back_to_the_recorded_tsim_width(fake_bench): + (case,) = bench.select(_bench_args("narrow", "--width-of", "clifft")) + assert (case.width, case.tsim_width) == (9, 4) + + +def test_sides_with_different_kept_fractions_are_flagged(): + def rounds(p, shots=100_000): + return [{"acceptance": p, "shots": shots}] * 3 + + assert bench.same_acceptance(rounds(0.144), rounds(0.1445)) + assert not bench.same_acceptance(rounds(0.144), rounds(0.16)) + assert bench.same_acceptance([{"acceptance": None}], rounds(0.5)) + + +def test_the_corpus_export_is_complete_and_shared(tmp_path, monkeypatch): + import threading + + source = tmp_path / "source" + (source / "tsim_bench").mkdir(parents=True) + (source / "tsim_bench" / "__init__.py").write_text("") + common.git("init", "-q", cwd=source) + common.git("add", ".", cwd=source) + common.git( + "-c", + "user.name=t", + "-c", + "user.email=t@t", + "commit", + "-q", + "-m", + "x", + cwd=source, + ) + rev = common.git("rev-parse", "HEAD", cwd=source) + monkeypatch.setattr(corpus, "URL", str(source)) + monkeypatch.setenv("TSIM_DEV_CACHE", str(tmp_path / "cache")) + results, errors = [], [] + + def export(): + try: + results.append(corpus._export(rev)) + except Exception as error: + errors.append(error) + + threads = [threading.Thread(target=export) for _ in range(4)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + assert not errors and len(set(results)) == 1 + assert (results[0] / corpus.COMPLETE).exists() + assert (results[0] / "tsim_bench" / "__init__.py").exists() + with pytest.raises(common.InfraError, match="has no commit"): + corpus._export("0" * 40) From 24575dd48b5586e4b13cc97a38687c93a42b2390 Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 08:53:42 -0700 Subject: [PATCH 05/11] chore: hide development skills from the skill installer `npx skills add QuEraComputing/tsim` also searches `.claude/skills` and `.agents/skills`, so it offered the four development skills next to the user skill. Mark them `metadata.internal: true` (shown only with INSTALL_INTERNAL_SKILLS=1), name the user skill in the documented install command, and test the flag. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/benchmarking-tsim/SKILL.md | 2 ++ .claude/skills/profiling-tsim/SKILL.md | 2 ++ .claude/skills/shipping-tsim-changes/SKILL.md | 2 ++ .claude/skills/testing-tsim-correctness/SKILL.md | 2 ++ CHANGELOG.md | 4 +++- README.md | 2 +- docs/install.mdx | 2 +- test/unit/test_dev_cli.py | 7 ++++--- 8 files changed, 17 insertions(+), 6 deletions(-) diff --git a/.claude/skills/benchmarking-tsim/SKILL.md b/.claude/skills/benchmarking-tsim/SKILL.md index f4417066..cf14280b 100644 --- a/.claude/skills/benchmarking-tsim/SKILL.md +++ b/.claude/skills/benchmarking-tsim/SKILL.md @@ -1,6 +1,8 @@ --- name: benchmarking-tsim description: Measures Tsim throughput and compile time with `tsim-dev bench` on the tsim-bench circuits and compares a change against a reference commit or checkout. Use when asked to benchmark Tsim, to check a change for performance regressions, to produce before and after numbers for a pull request, to run the tsim-bench corpus, or to add a benchmark circuit. +metadata: + internal: true --- # Benchmarking Tsim diff --git a/.claude/skills/profiling-tsim/SKILL.md b/.claude/skills/profiling-tsim/SKILL.md index 886c0778..3b91fa36 100644 --- a/.claude/skills/profiling-tsim/SKILL.md +++ b/.claude/skills/profiling-tsim/SKILL.md @@ -1,6 +1,8 @@ --- name: profiling-tsim description: Finds where Tsim spends compile and sampling time with `tsim-dev profile` and `tsim-dev inspect`, and turns a profile into an optimization plan. Use when asked why a circuit is slow, where time goes, what to optimize next, or before starting performance work on Tsim. +metadata: + internal: true --- # Profiling Tsim diff --git a/.claude/skills/shipping-tsim-changes/SKILL.md b/.claude/skills/shipping-tsim-changes/SKILL.md index 769ddfeb..aa961cb0 100644 --- a/.claude/skills/shipping-tsim-changes/SKILL.md +++ b/.claude/skills/shipping-tsim-changes/SKILL.md @@ -1,6 +1,8 @@ --- name: shipping-tsim-changes description: Takes a change to Tsim's code from a new branch to a reviewed local commit, with the checks, evidence and git rules this repository uses. Use when starting a feature, bug fix or optimization in the Tsim repository, when asked to open or update a pull request, or before saying that a change is done. +metadata: + internal: true --- # Shipping a change to Tsim diff --git a/.claude/skills/testing-tsim-correctness/SKILL.md b/.claude/skills/testing-tsim-correctness/SKILL.md index 685e9ba3..56985caa 100644 --- a/.claude/skills/testing-tsim-correctness/SKILL.md +++ b/.claude/skills/testing-tsim-correctness/SKILL.md @@ -1,6 +1,8 @@ --- name: testing-tsim-correctness description: Chooses the oracle and writes statistical tests that show a Tsim change samples the right distribution. Use when adding or changing a gate, noise channel, sampler option, scheduler, plan or kernel in Tsim, when fixing a wrong-results bug, or when asked whether Tsim's output can be trusted for a circuit. +metadata: + internal: true --- # Testing Tsim's correctness diff --git a/CHANGELOG.md b/CHANGELOG.md index a8fcee97..cdbf0fb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - An agent skill, `simulating-circuits-with-tsim`, in `skills/`, that teaches coding agents to use Tsim: active width, sampling and postselection, QEC studies with decoders, and porting published circuits. Install it with - `npx skills add QuEraComputing/tsim`; the docs site also serves it. + `npx skills add QuEraComputing/tsim --skill simulating-circuits-with-tsim`; + the docs site also serves it. The repository's development skills in + `.claude/skills` are marked internal, so the installer does not offer them. - `Circuit(...)` accepts a `stim.Circuit` or another `Circuit` and copies it, like `Circuit.from_stim_program`. Before, passing a `stim.Circuit` raised an `AttributeError`. diff --git a/README.md b/README.md index 7d080489..7c16e7d4 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ agents (Claude Code, Codex, Cursor and others) how to judge whether a circuit is sample and postselect it, run logical error rate studies, and port published Stim circuits. Install it with: ```bash -npx skills add QuEraComputing/tsim +npx skills add QuEraComputing/tsim --skill simulating-circuits-with-tsim ``` ## Quick Start diff --git a/docs/install.mdx b/docs/install.mdx index 319e7154..35a273c2 100644 --- a/docs/install.mdx +++ b/docs/install.mdx @@ -42,7 +42,7 @@ how to judge whether a circuit is feasible, sample and postselect it, run logica studies, and port published Stim circuits. Install it with: ```bash -npx skills add QuEraComputing/tsim +npx skills add QuEraComputing/tsim --skill simulating-circuits-with-tsim ``` The skill is also served by this site at `/.well-known/skills/`. diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index cefaf463..41fad978 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -6,6 +6,7 @@ from pathlib import Path import pytest +import yaml from tsim_dev import bench, checks, cli, common, corpus ROOT = Path(__file__).resolve().parents[2] @@ -221,10 +222,10 @@ def test_dev_skills_have_valid_frontmatter(): for path in skills: text = path.read_text() assert text.startswith("---\n") - front = dict( - line.split(": ", 1) for line in text.split("---\n")[1].strip().splitlines() - ) + front = yaml.safe_load(text.split("---\n")[1]) assert front["name"] == path.parent.name + # NOTE: hides development skills from `npx skills add QuEraComputing/tsim`. + assert front["metadata"]["internal"] is True assert 0 < len(front["description"]) <= 1024 assert len(text.splitlines()) < 500 From b88563b96ebaacd3c2a7d223f5bc30569b9c1f24 Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 09:00:50 -0700 Subject: [PATCH 06/11] feat(dev): `tsim-dev bench --width` compares active widths without sampling Each side compiles every selected circuit once (tsim-bench's worker in width mode, 4 jobs in parallel by default) and prints its width next to the one recorded in tsim-bench; the verdict per circuit is wider, narrower, same or unknown, and any wider width exits 1. A circuit over Tsim's limit shows `>=N`, the width from Tsim's refusal. A compile longer than --case-timeout (300 s) is stopped and recorded as an error; the remaining circuits continue in a new worker. The default suite is the whole corpus: 190 circuits, both sides, in 519 s on a Ryzen 9 9900X, all the same as main. Pin tsim-bench cd0acb6 (width mode; Tsim 1.1 worker). Co-Authored-By: Claude Opus 5.5 --- .claude/skills/benchmarking-tsim/SKILL.md | 18 +- AGENTS.md | 1 + dev/tsim_dev/bench.py | 6 +- dev/tsim_dev/cli.py | 25 ++- dev/tsim_dev/corpus.py | 2 +- dev/tsim_dev/width.py | 205 ++++++++++++++++++++++ test/unit/test_dev_cli.py | 48 ++++- 7 files changed, 298 insertions(+), 7 deletions(-) create mode 100644 dev/tsim_dev/width.py diff --git a/.claude/skills/benchmarking-tsim/SKILL.md b/.claude/skills/benchmarking-tsim/SKILL.md index cf14280b..f2474204 100644 --- a/.claude/skills/benchmarking-tsim/SKILL.md +++ b/.claude/skills/benchmarking-tsim/SKILL.md @@ -42,7 +42,23 @@ inject_cultivate_d5_p0.001 6 13.9M 0.141 0.13 1.20x [1.19, 1.21] fast 2: a circuit could not be measured. - The JSON under `.tsim-dev/bench/` has every round. -## Compare settings: threads, GPU, strategy +## Active width only (`--width`) + +For changes to compilation, scheduling or the plan, compare widths without sampling: + +```bash +uv run tsim-dev bench --width --ref origin/main # whole corpus, compile only +uv run tsim-dev bench --width --suite full --ref origin/main +``` + +Each side compiles every circuit once, in one process, and prints the width next to +the one recorded in tsim-bench. The verdict is `wider`, `narrower`, `same` or +`unknown`; any `wider` exits 1. A circuit over Tsim's limit shows `>=N`: N is the width +in Tsim's refusal, a lower bound. Widths do not depend on the machine, so this needs +no rounds or quiet machine, but it says nothing about speed: a change that keeps the +width can still be slower. Compile times are shown for each side. + +## Compare settings: threads, strategy `--side REF[:key=value,...]`, given twice, replaces `--ref` when the two sides differ in more than the checkout. REF is `.` (this checkout), a git revision, a checkout path, or diff --git a/AGENTS.md b/AGENTS.md index 1e629648..23383480 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,6 +15,7 @@ failed (or got slower), 2 the harness could not run, 3 still running. uv run tsim-dev check # the gate before review: pre-commit, full suite (~90 s) uv run tsim-dev test --changed # tests of the changed files; or give paths uv run tsim-dev bench --ref origin/main # quick A/B benchmark, ~4 min; exit 1 if slower +uv run tsim-dev bench --width --ref origin/main # active widths only, nothing sampled uv run tsim-dev inspect FILE.stim # qubits, T count, active width, stats; no sampling uv run tsim-dev profile cultivation_d5 # where compile and sampling time goes (py-spy) uv run tsim-dev run -- CMD # any command under memory, CPU and time caps diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py index cca85fcf..eae23fb5 100644 --- a/dev/tsim_dev/bench.py +++ b/dev/tsim_dev/bench.py @@ -18,7 +18,7 @@ from dataclasses import dataclass from pathlib import Path -from . import common, corpus +from . import common, corpus, width from .common import FAILED, INFRA, OK, InfraError @@ -133,6 +133,8 @@ def _lookup(item: str, everything: dict) -> object | None: def select(args) -> list[Case]: """Turn the command line into the list of cases to measure.""" + if args.suite is None: # compiling only is fast enough for the whole corpus + args.suite = "corpus" if getattr(args, "width", False) else "quick" try: return _select(args) except (ValueError, RuntimeError) as error: # a malformed tsim-bench manifest @@ -419,6 +421,8 @@ def run(args) -> int: raise InfraError("the two sides are the same; change a setting of one") except InfraError as error: common.die(str(error)) + if args.width: + return width.run(args, cases, sides) has_ref = len(sides) == 2 rounds = args.rounds or (5 if has_ref else 3) args.rounds = rounds diff --git a/dev/tsim_dev/cli.py b/dev/tsim_dev/cli.py index 9ac49a54..10c4d776 100644 --- a/dev/tsim_dev/cli.py +++ b/dev/tsim_dev/cli.py @@ -31,11 +31,12 @@ def _corpus_dir(parser) -> None: ) -def _selection(parser, suite: str) -> None: +def _selection(parser, suite: str | None) -> None: + shown = suite or "quick, or corpus with --width" parser.add_argument( "--suite", default=suite, - help=f"tsim-bench suite, or `corpus` for every circuit (default {suite})", + help=f"tsim-bench suite, or `corpus` for every circuit (default {shown})", ) parser.add_argument( "--max-width", @@ -98,6 +99,24 @@ def build_parser() -> argparse.ArgumentParser: "circuits", nargs="*", help="tsim-bench ids or file stems, or .stim paths" ) p.add_argument("--ref", help="git revision or checkout path to compare against") + p.add_argument( + "--width", + action="store_true", + help="compile only and compare active widths; nothing is sampled " + "(default suite: corpus)", + ) + p.add_argument( + "--case-timeout", + type=int, + default=300, + help="--width: seconds to compile one circuit (default 300)", + ) + p.add_argument( + "--jobs", + type=int, + default=4, + help="--width: circuits compiled at once, each under --mem (default 4)", + ) p.add_argument( "--side", action="append", @@ -108,7 +127,7 @@ def build_parser() -> argparse.ArgumentParser: "Keys: threads=N, device=cpu|gpu, strategy=NAME. " "Example: --side .:threads=10 --side .:threads=1", ) - _selection(p, "quick") + _selection(p, None) p.add_argument("--rounds", type=int, default=0, help="default 5 with --ref, else 3") p.add_argument( "--min-seconds", type=float, default=0.5, help="length of a timed call" diff --git a/dev/tsim_dev/corpus.py b/dev/tsim_dev/corpus.py index da6ee7bd..9c2f31d1 100644 --- a/dev/tsim_dev/corpus.py +++ b/dev/tsim_dev/corpus.py @@ -24,7 +24,7 @@ URL = "https://github.com/rafaelha/tsim-bench" # Bump together with any change that needs newer circuits or manifests. -REV = "88f3b7264aecb5b7d0e83e0be651e2038e2232cb" +REV = "cd0acb6488a8ac357cefd2418b69609f637fb3e6" COMPLETE = ".tsim-dev-complete" diff --git a/dev/tsim_dev/width.py b/dev/tsim_dev/width.py new file mode 100644 index 00000000..0bd85643 --- /dev/null +++ b/dev/tsim_dev/width.py @@ -0,0 +1,205 @@ +"""`tsim-dev bench --width`: active width of each circuit, compiled but not sampled. + +Each side compiles all selected circuits in one worker process (tsim-bench's worker in +`width` mode), so the whole corpus takes minutes rather than hours. If the process dies +(memory cap) or one circuit compiles for longer than `--case-timeout`, that circuit is +recorded as an error and a new process continues with the next one. +""" + +from __future__ import annotations + +import concurrent.futures +import json +import os +import select +import statistics +import subprocess +import time + +from . import common, corpus +from .common import FAILED, INFRA, OK + +NAME = 44 + + +def _items(cases) -> list[tuple[str, str, str]]: + """Return (key, flag, value) for each case: `--case ID` or `--circuit PATH`.""" + return [ + ( + (case.id, "--case", case.id) + if case.id + else (str(case.path), "--circuit", str(case.path)) + ) + for case in cases + ] + + +def _stream(cmd, env, case_timeout: int): + """Run the worker; yield its lines, and stop it if one circuit compiles too long. + + Yields `("line", text)` for each output line, then `("exit", (code, stderr))`; + `code` is "timeout" when the worker was stopped. + """ + with subprocess.Popen( + cmd, env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True + ) as process: + assert process.stdout is not None and process.stderr is not None + deadline = time.monotonic() + case_timeout + while True: + ready, _, _ = select.select([process.stdout], [], [], 1.0) + if ready: + line = process.stdout.readline() + if not line: + break + deadline = time.monotonic() + case_timeout + yield "line", line.rstrip("\n") + elif time.monotonic() > deadline: + process.kill() + process.wait() + yield "exit", ("timeout", "") + return + code = process.wait() + yield "exit", (code, process.stderr.read()) + + +def measure(side, cases, args) -> dict[str, dict]: + """Compile every case on one side and return its result by case key.""" + from .bench import side_env + + results: dict[str, dict] = {} + pending = _items(cases) + while pending: + cmd = [str(side.python), "-u", str(corpus.worker(args.corpus_dir))] + cmd += ["--mode", "width", "--root", str(corpus.root(args.corpus_dir))] + if side.strategy or args.strategy: + cmd += ["--strategy", side.strategy or args.strategy] + for _, flag, value in pending: + cmd += [flag, value] + env = side_env(side, side.threads or args.threads) + started, code, stderr = None, 0, "" + for kind, value in _stream( + common.capped(cmd, mem=args.mem), env, args.case_timeout + ): + if kind == "exit": + code, stderr = value + elif value.startswith("START "): + started = value[6:] + elif value.startswith("RESULT "): + result = json.loads(value[7:]) + results[result.pop("name")] = result + missing = [item for item in pending if item[0] not in results] + if not missing: + break + # NOTE: the worker died or was stopped while compiling `started`. + if code == "timeout": + reason = f"compile took over {args.case_timeout} s" + else: + reason = common.explain_exit(code, args.mem, None) + reason += ": " + (stderr.strip().splitlines() or [""])[-1] + error = {"error": reason[:200]} + if started is None or started not in {key for key, _, _ in missing}: + results.update({key: dict(error) for key, _, _ in missing}) + break + results[started] = error + pending = [item for item in missing if item[0] != started] + return results + + +def measure_parallel(side, cases, args) -> dict[str, dict]: + """Split the cases over `args.jobs` workers, largest files first, and merge.""" + ordered = sorted(cases, key=lambda case: -case.path.stat().st_size) + jobs = max(1, min(args.jobs, len(ordered))) + chunks = [ordered[i::jobs] for i in range(jobs)] + results: dict[str, dict] = {} + with concurrent.futures.ThreadPoolExecutor(jobs) as pool: + for part in pool.map(lambda chunk: measure(side, chunk, args), chunks): + results.update(part) + return results + + +def _shown(result: dict | None) -> str: + if result is None or "error" in result: + return "err" + width = result.get("width") + if width is None: + return "-" + return f">={width}" if result.get("refused") else str(width) + + +def verdict(new: dict, ref: dict) -> str: + """Compare two widths, lower is better; a refused width is only a lower bound.""" + if "error" in new or "error" in ref: + return "error" + a, b = new.get("width"), ref.get("width") + if a is None or b is None: + return "same" if a == b else "n/a" + if new.get("refused") and ref.get("refused"): + return "same" if a == b else "unknown" # first refused component, not the max + if new.get("refused"): + return "wider" if a > b else "unknown" + if ref.get("refused"): + return "narrower" if a < b else "unknown" + return "same" if a == b else ("narrower" if a < b else "wider") + + +def run(args, cases, sides) -> int: + """Compile every case on each side, print the widths and the verdicts.""" + has_ref = len(sides) == 2 + print(f"bench --width: new={sides[0].describe()}", end="") + print(f" ref={sides[1].describe()}" if has_ref else "", end="") + print( + f" circuits={len(cases)} tsim-bench={corpus.describe(args.corpus_dir)}", + flush=True, + ) + start = time.monotonic() + per_side = [measure_parallel(side, cases, args) for side in sides] + elapsed = time.monotonic() - start + keys = [key for key, _, _ in _items(cases)] + + header = f"{'circuit':<{NAME}} {'k':>5} {'recorded':>8} {'compile':>8}" + if has_ref: + header += f" {'ref k':>6} {'compile':>8} verdict" + print(header) + records, counts = [], {} + for case, key in zip(cases, keys, strict=True): + new = per_side[0][key] + record = {"circuit": case.id or str(case.path), "new": new} + row = f"{case.name[:NAME]:<{NAME}} {_shown(new):>5}" + row += f" {'-' if case.tsim_width is None else case.tsim_width!s:>8}" + row += f" {new.get('compile_s', float('nan')):>8.2f}" + if has_ref: + ref = per_side[1][key] + record["ref"] = ref + record["verdict"] = verdict(new, ref) + counts[record["verdict"]] = counts.get(record["verdict"], 0) + 1 + row += f" {_shown(ref):>6} {ref.get('compile_s', float('nan')):>8.2f}" + row += f" {record['verdict']}" + if "error" in new: + row += f" ERROR: {new['error']}" + elif has_ref and "error" in record["ref"]: + row += f" ref ERROR: {record['ref']['error']}" + records.append(record) + print(row, flush=True) + + out = common.out_dir("bench") / f"width-{common.stamp()}.json" + meta = { + "new": sides[0].describe(), + "ref": sides[1].describe() if has_ref else None, + "tsim_bench": corpus.describe(args.corpus_dir), + "cpu_count": os.cpu_count(), + } + out.write_text(json.dumps({"meta": meta, "results": records}, indent=1)) + + errors = sum("error" in r["new"] or "error" in r.get("ref", {}) for r in records) + compiles = [r["new"]["compile_s"] for r in records if "compile_s" in r["new"]] + summary = f"SUMMARY: {len(records)} circuits, {errors} errors in {elapsed:.0f} s" + if compiles: + summary += f", median compile {statistics.median(compiles):.2f} s" + if has_ref: + summary += ", " + ", ".join( + f"{counts.get(v, 0)} {v}" for v in ("wider", "narrower", "same", "unknown") + ) + print(f"{summary}. Details: {out}") + if errors: + return INFRA + return FAILED if counts.get("wider") else OK diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index 41fad978..c629f5af 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -7,7 +7,7 @@ import pytest import yaml -from tsim_dev import bench, checks, cli, common, corpus +from tsim_dev import bench, checks, cli, common, corpus, width ROOT = Path(__file__).resolve().parents[2] HOOK = ROOT / ".claude" / "hooks" / "guard_bash.py" @@ -335,3 +335,49 @@ def export(): assert (results[0] / "tsim_bench" / "__init__.py").exists() with pytest.raises(common.InfraError, match="has no commit"): corpus._export("0" * 40) + + +@pytest.mark.parametrize( + ("new", "ref", "expected"), + [ + ({"width": 4}, {"width": 4}, "same"), + ({"width": 3}, {"width": 4}, "narrower"), + ({"width": 5}, {"width": 4}, "wider"), + ({"width": 40, "refused": True}, {"width": 26}, "wider"), + ({"width": 26}, {"width": 40, "refused": True}, "narrower"), + ({"width": 63, "refused": True}, {"width": 63, "refused": True}, "same"), + ({"width": 20, "refused": True}, {"width": 26}, "unknown"), + ({"error": "x"}, {"width": 4}, "error"), + ], +) +def test_width_verdicts_treat_refused_widths_as_lower_bounds(new, ref, expected): + assert width.verdict(new, ref) == expected + + +def test_width_mode_resumes_after_the_worker_dies(fake_bench, monkeypatch): + calls = [] + + def fake_stream(cmd, env, case_timeout): + cases = [cmd[i + 1] for i, x in enumerate(cmd) if x == "--case"] + calls.append(cases) + for case in cases: + yield "line", f"START {case}" + if case == "p/narrow" and len(calls) == 1: + yield "exit", ("timeout", "") + return + yield "line", "RESULT " + json.dumps({"name": case, "width": 3}) + yield "exit", (0, "") + + monkeypatch.setattr(width, "_stream", fake_stream) + cases = bench.select(_bench_args("--width", "--suite", "corpus")) + results = width.measure(bench.parse_side("."), cases, _bench_args("--width")) + assert results["p/narrow"]["error"].startswith("compile took over") + assert results["p/wide"] == {"width": 3} + assert calls == [["p/narrow", "p/wide"], ["p/wide"]] + + +def test_the_width_stream_stops_a_stuck_compile(): + script = "import time; print('START a', flush=True); time.sleep(30)" + events = list(width._stream([sys.executable, "-c", script], None, case_timeout=1)) + assert events[0] == ("line", "START a") + assert events[-1] == ("exit", ("timeout", "")) From bbf11efa7eb57f906bdaf50367eb817c4186fb0b Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 09:01:17 -0700 Subject: [PATCH 07/11] chore(dev): remove the GPU options from tsim-dev bench Drop `--side ...:device=cpu|gpu`, the CUDA check and the GPU notes in the benchmarking and profiling skills until Tsim has a GPU backend on main. Sides take `threads` and `strategy`. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/benchmarking-tsim/SKILL.md | 17 ++++++---------- .claude/skills/profiling-tsim/SKILL.md | 3 +-- dev/tsim_dev/bench.py | 21 ++++---------------- dev/tsim_dev/cli.py | 2 +- test/unit/test_dev_cli.py | 24 ++++------------------- 5 files changed, 16 insertions(+), 51 deletions(-) diff --git a/.claude/skills/benchmarking-tsim/SKILL.md b/.claude/skills/benchmarking-tsim/SKILL.md index f2474204..fd83d6d2 100644 --- a/.claude/skills/benchmarking-tsim/SKILL.md +++ b/.claude/skills/benchmarking-tsim/SKILL.md @@ -62,20 +62,16 @@ width can still be slower. Compile times are shown for each side. `--side REF[:key=value,...]`, given twice, replaces `--ref` when the two sides differ in more than the checkout. REF is `.` (this checkout), a git revision, a checkout path, or -a Python executable (an environment built elsewhere, such as one with the GPU extra). -Keys: `threads=N`, `device=cpu|gpu`, `strategy=NAME`. The first side is "new". +a Python executable (an environment built elsewhere). Keys: `threads=N`, +`strategy=NAME`. The first side is "new". ```bash uv run tsim-dev bench --side .:threads=10 --side .:threads=1 # 10 cores vs 1 -uv run tsim-dev bench --side $GPU_PY:device=gpu --side $GPU_PY:device=cpu uv run tsim-dev bench --side .:threads=4 --side origin/main:threads=4 ``` -- `device=cpu` hides CUDA devices; `device=gpu` fails the circuit unless the worker finds - Tsim's CUDA executor usable. Circuits above the executor's width limit still fall back - to the CPU on that side, so check the width before reading a GPU ratio. -- Compile time is compared too; across devices a "slower" compile verdict (and exit code - 1) is expected, so read the rate column. +- Compile time is compared too; when the sides differ only in threads, read the rate + column. ## What the command controls for you @@ -127,9 +123,8 @@ different regime. `--only SUBSTRING` keeps circuits whose id contains it. ## Limits -- Tsim only, on the CPU or (with `--side ...:device=gpu`) its CUDA executor. - Comparisons with other simulators (Clifft, SymFT, Magna) are run from the - tsim-bench repository, which pins each simulator's environment. +- Tsim on the CPU only. Comparisons with other simulators (Clifft, SymFT, Magna) are + run from the tsim-bench repository, which pins each simulator's environment. - To add a circuit or change a suite, change tsim-bench: put the file in its paper's folder, add an entry to that folder's `circuits.toml` (arXiv id, sampler, postselection and the reason for it, see its `BENCHMARKS.md`), run `python -m tsim_bench check` diff --git a/.claude/skills/profiling-tsim/SKILL.md b/.claude/skills/profiling-tsim/SKILL.md index 3b91fa36..08e384ba 100644 --- a/.claude/skills/profiling-tsim/SKILL.md +++ b/.claude/skills/profiling-tsim/SKILL.md @@ -50,8 +50,7 @@ measurement that counts. Micro-benchmarks of a kernel alone have misled before (cache effects, struct passing by value), so confirm any gain end to end. Hardware counters (`perf stat`) need `/proc/sys/kernel/perf_event_paranoid` at 1 or -lower; ask the user rather than changing it. For GPU work, use Nsight Systems and -Nsight Compute 2026 or newer; older `ncu` releases do not support the RTX 5090. +lower; ask the user rather than changing it. ## 4. Turn it into a plan diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py index eae23fb5..eebda446 100644 --- a/dev/tsim_dev/bench.py +++ b/dev/tsim_dev/bench.py @@ -44,26 +44,22 @@ class Case: class Side: """One side of a comparison: the Python that imports its Tsim, and its settings. - `threads` and `strategy` override the command line's when set. `device` is "" (the - environment decides), "cpu" (CUDA devices hidden) or "gpu" (the run fails unless the - worker finds Tsim's CUDA executor usable). + `threads` and `strategy` override the command line's when set. """ label: str python: Path threads: int | None = None - device: str = "" strategy: str | None = None def describe(self) -> str: """Return the label with the settings that this side overrides.""" extra = [f"threads={self.threads}"] if self.threads is not None else [] - extra += [f"device={self.device}"] if self.device else [] extra += [f"strategy={self.strategy}"] if self.strategy is not None else [] return " ".join([self.label, *extra]) -SIDE_KEYS = ("threads", "device", "strategy") +SIDE_KEYS = ("threads", "strategy") def parse_side(spec: str) -> Side: @@ -71,7 +67,7 @@ def parse_side(spec: str) -> Side: REF is `.` (this checkout), a git revision, the path of another checkout with a `.venv`, or the path of a Python executable (an environment built elsewhere, such - as one with a GPU extra). Keys: threads=N, device=cpu|gpu, strategy=NAME. + as one with other dependencies). Keys: threads=N, strategy=NAME. """ ref, _, options = spec.partition(":") settings: dict = {} @@ -87,8 +83,6 @@ def parse_side(spec: str) -> Side: if not settings["threads"].isdigit() or int(settings["threads"]) < 1: raise InfraError(f"bad thread count in --side {spec!r}") settings["threads"] = int(settings["threads"]) - if settings.get("device", "cpu") not in ("cpu", "gpu"): - raise InfraError(f"device must be cpu or gpu in --side {spec!r}") if ref in ("", "."): base = Side(common.describe(common.root()), Path(sys.executable)) elif Path(ref).expanduser().is_file(): @@ -245,8 +239,6 @@ def worker_command(python: Path, case: Case, args, mode: str) -> list[str]: def side_env(side: Side, threads: int) -> dict[str, str]: """Return the environment of one side's worker.""" env = common.child_env(threads) - if side.device == "cpu": - env["CUDA_VISIBLE_DEVICES"] = "" return env @@ -269,12 +261,7 @@ def measure(side: Side, case: Case, args, mode: str = "bench") -> dict: ) for line in done.stdout.splitlines(): if line.startswith("RESULT "): - result = json.loads(line[7:]) - if side.device == "gpu" and not result.get("cuda"): - return { - "error": "device=gpu, but this Tsim has no usable CUDA executor" - } - return result + return json.loads(line[7:]) reason = common.explain_exit(done.returncode, args.mem, args.timeout) last = (done.stderr.strip().splitlines() or [""])[-1] return {"error": f"{reason}: {last}"[:200]} diff --git a/dev/tsim_dev/cli.py b/dev/tsim_dev/cli.py index 10c4d776..408cec38 100644 --- a/dev/tsim_dev/cli.py +++ b/dev/tsim_dev/cli.py @@ -124,7 +124,7 @@ def build_parser() -> argparse.ArgumentParser: metavar="REF[:KEY=VALUE,...]", help="a side with its own settings, instead of --ref; give two to compare. " "REF: `.`, a git revision, a checkout or a python executable. " - "Keys: threads=N, device=cpu|gpu, strategy=NAME. " + "Keys: threads=N, strategy=NAME. " "Example: --side .:threads=10 --side .:threads=1", ) _selection(p, None) diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index c629f5af..42dcf14e 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -231,38 +231,22 @@ def test_dev_skills_have_valid_frontmatter(): def test_sides_carry_their_own_settings(fake_bench): - side = bench.parse_side(".:threads=10,device=cpu,strategy=cat5") - assert (side.python, side.threads, side.device, side.strategy) == ( + side = bench.parse_side(".:threads=10,strategy=cat5") + assert (side.python, side.threads, side.strategy) == ( Path(sys.executable), 10, - "cpu", "cat5", ) - assert side.describe().endswith("threads=10 device=cpu strategy=cat5") - assert bench.side_env(side, 10)["CUDA_VISIBLE_DEVICES"] == "" + assert side.describe().endswith("threads=10 strategy=cat5") assert bench.side_env(side, 10)["NUMBA_NUM_THREADS"] == "10" - assert "CUDA_VISIBLE_DEVICES" not in bench.side_env( - bench.parse_side(".:device=gpu"), 1 - ) # A python executable is used as given: resolving a venv's symlink would lose the venv. python = Path(sys.executable).absolute() assert bench.parse_side(f"{python}:threads=2").python == python - for bad in (".:cores=2", ".:threads=0", ".:device=tpu", ".:threads"): + for bad in (".:cores=2", ".:threads=0", ".:device=gpu", ".:threads"): with pytest.raises(common.InfraError): bench.parse_side(bad) -def test_a_gpu_side_fails_when_cuda_is_not_used(fake_bench, monkeypatch): - done = subprocess.CompletedProcess([], 0, 'RESULT {"cuda": false}\n', "") - monkeypatch.setattr(bench.subprocess, "run", lambda *a, **k: done) - (case,) = bench.select(_bench_args()) - args = _bench_args() - assert "error" in bench.measure(bench.parse_side(".:device=gpu"), case, args) - assert bench.measure(bench.parse_side(".:device=cpu"), case, args) == { - "cuda": False - } - - def test_profile_and_inspect_accept_a_corpus_dir(): for command in ("profile", "inspect"): args = cli.build_parser().parse_args([command, "qv_q10"]) From 9d378be55957beba53c9faa8b4c6c746a7eda5ad Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 09:01:40 -0700 Subject: [PATCH 08/11] feat(dev): show and check the active width in shot benchmarks With a reference, each row compares the two sides' active widths and ends in `WIDER: k 6 -> 7` or `NARROWER: ...` when they differ; a wider width fails the run. A width that changes between runs of one side is an error, since it means compilation is not deterministic. `k` is the sampler's own `active_width` (Tsim 1.1), without a fallback to the recorded width. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/benchmarking-tsim/SKILL.md | 8 ++++--- dev/tsim_dev/bench.py | 28 ++++++++++++++++++++--- test/unit/test_dev_cli.py | 27 +++++++++++++++++++++- 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/.claude/skills/benchmarking-tsim/SKILL.md b/.claude/skills/benchmarking-tsim/SKILL.md index fd83d6d2..5351e41d 100644 --- a/.claude/skills/benchmarking-tsim/SKILL.md +++ b/.claude/skills/benchmarking-tsim/SKILL.md @@ -28,6 +28,9 @@ inject_cultivate_d5_p0.001 6 13.9M 0.141 0.13 1.20x [1.19, 1.21] fast ``` - `kept` is the fraction of shots that survive postselection (`-` when there is none). +- `k` is the active width of the compiled sampler. With `--ref`, a row that ends in + `WIDER: k 6 -> 7` or `NARROWER: ...` means the two sides compiled different widths; + `wider` fails the run. A width that changes between runs of one side is an error. - `shots/s vs ref` is new over reference, so above 1 is faster. The bracket is the smallest and largest ratio over the rounds. @@ -37,9 +40,8 @@ inject_cultivate_d5_p0.001 6 13.9M 0.141 0.13 1.20x [1.19, 1.21] fast - `kept-differs`: the two sides kept different fractions of shots (beyond binomial noise), so they did not sample the same thing and the rate is not compared. It counts as a failure: a change to postselection or to the sampled distribution. -- Exit code 0: nothing slower. 1: at least one `slower` in rate or compile time, or - a `kept-differs`. - 2: a circuit could not be measured. +- Exit code 0: nothing slower. 1: at least one `slower` in rate or compile time, a + `kept-differs`, or a wider active width. 2: a circuit could not be measured. - The JSON under `.tsim-dev/bench/` has every round. ## Active width only (`--width`) diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py index eebda446..6368d007 100644 --- a/dev/tsim_dev/bench.py +++ b/dev/tsim_dev/bench.py @@ -309,10 +309,17 @@ def run_case(case: Case, sides: list[Side], args) -> dict: return record rounds[side.describe()].append(result) record["rounds"] = rounds + # NOTE: the width is fixed by the compiled sampler, so it must not change between + # runs of one side; a change means compilation is not deterministic. + for side in sides: + widths = {r.get("active_width") for r in rounds[side.describe()]} + if len(widths) > 1: + record["error"] = ( + f"{side.describe()}: active width changed between runs: {sorted(widths)}" + ) + return record new = rounds[sides[0].describe()] record["k"] = new[0].get("active_width") - if record["k"] is None: # Tsim before 1.1 does not report it; use the recorded one - record["k"] = case.tsim_width record["acceptance"] = new[0].get("acceptance") record["shots_per_s"] = statistics.median(r["shots_per_s"] for r in new) record["compile_s"] = statistics.median(r["compile_s"] for r in new) @@ -333,6 +340,8 @@ def run_case(case: Case, sides: list[Side], args) -> dict: if abs(record["compile_s"] - record["ref_compile_s"]) < 0.05: record["compile"]["verdict"] = "same" record["ref_acceptance"] = old[0].get("acceptance") + record["ref_k"] = old[0].get("active_width") + record["width"] = width_verdict(record["k"], record["ref_k"]) if not same_acceptance(new, old): # NOTE: a different kept fraction means the sides sample different # things, so their rates are not comparable. @@ -343,6 +352,13 @@ def run_case(case: Case, sides: list[Side], args) -> dict: KEPT_DIFFERS = "kept-differs" +def width_verdict(new: int | None, ref: int | None) -> str: + """Compare the active widths of the two sides; lower is better.""" + if new is None or ref is None or new == ref: + return "same" + return "narrower" if new < ref else "wider" + + def same_acceptance(new: list[dict], old: list[dict], sigmas: float = 6.0) -> bool: """Return whether two sides' kept fractions agree within binomial noise. @@ -383,6 +399,8 @@ def format_row(record: dict, has_ref: bool) -> str: rate, comp = record["rate"], record["compile"] row += f" {rate['ratio']:.2f}x [{rate['low']:.2f}, {rate['high']:.2f}]" row += f" {rate['verdict']:<6} {comp['ratio']:.2f}x {comp['verdict']}" + if record.get("width", "same") != "same": + row += f" {record['width'].upper()}: k {record['ref_k']} -> {record['k']}" if rate["verdict"] == KEPT_DIFFERS: row += ( f" (kept {record['acceptance']:.4g} vs ref {record['ref_acceptance']:.4g})" @@ -459,6 +477,8 @@ def run(args) -> int: ] slower = verdicts.count("slower") differs = verdicts.count(KEPT_DIFFERS) + widths = [r.get("width") for r in records if "error" not in r] + wider, narrower = widths.count("wider"), widths.count("narrower") summary = f"SUMMARY: {len(records)} circuits, {errors} errors" if has_ref: summary += ( @@ -469,10 +489,12 @@ def run(args) -> int: summary += ( f", {differs} with a different kept fraction (rates not comparable)" ) + if wider or narrower: + summary += f", active width: {wider} wider, {narrower} narrower" print(f"{summary}. Details: {out}") if errors: return INFRA - return FAILED if slower or differs else OK + return FAILED if slower or differs or wider else OK def corpus_command(args) -> int: diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index 42dcf14e..08a6872b 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -265,7 +265,7 @@ def test_paths_follow_the_manifest_and_no_postselect(fake_bench, tmp_path): assert bench.select(args)[0].postselect == "none" -def test_k_falls_back_to_the_recorded_tsim_width(fake_bench): +def test_cases_carry_the_recorded_tsim_width(fake_bench): (case,) = bench.select(_bench_args("narrow", "--width-of", "clifft")) assert (case.width, case.tsim_width) == (9, 4) @@ -365,3 +365,28 @@ def test_the_width_stream_stops_a_stuck_compile(): events = list(width._stream([sys.executable, "-c", script], None, case_timeout=1)) assert events[0] == ("line", "START a") assert events[-1] == ("exit", ("timeout", "")) + + +def test_shot_benchmarks_compare_and_check_the_active_width(fake_bench, monkeypatch): + widths = {"new": [6, 6, 6], "ref": [6, 6, 6]} + + def fake_measure(side, case, args, mode="bench"): + result = {"shots_per_s": 1e6, "compile_s": 0.1, "acceptance": None, "shots": 1} + result["active_width"] = widths[side.label].pop(0) + return result + + monkeypatch.setattr(bench, "measure", fake_measure) + (case,) = bench.select(_bench_args()) + sides = [bench.Side("new", Path("python")), bench.Side("ref", Path("python"))] + args = _bench_args("--rounds", "2") + args.rounds = 2 + record = bench.run_case(case, sides, args) + assert (record["k"], record["ref_k"], record["width"]) == (6, 6, "same") + + widths.update(new=[7, 7, 7], ref=[6, 6, 6]) + record = bench.run_case(case, sides, args) + assert record["width"] == "wider" + assert "WIDER: k 6 -> 7" in bench.format_row(record, has_ref=True) + + widths.update(new=[6, 6, 7], ref=[6, 6, 6]) + assert "changed between runs" in bench.run_case(case, sides, args)["error"] From e70e43d3fd1f5e6d7a0c6f74622074d159f703b2 Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 09:01:52 -0700 Subject: [PATCH 09/11] docs(dev): scale the shipping workflow to the change - Benchmark only changes that could affect compile or sampling cost (the compile and sampling paths, dependencies, numba settings), and run `bench --width` for changes that could change the active width. - A `slower` verdict is a finding, not a veto: confirm it, profile it, and remove it or explain it; a correctness fix may cost performance if stated. - Distinguish new work (a new branch off main) from updating an existing pull request (keep its branch and commit on top). - Infer acceptance criteria and ask only when they are unclear. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/shipping-tsim-changes/SKILL.md | 65 ++++++++++++++----- AGENTS.md | 14 ++-- 2 files changed, 57 insertions(+), 22 deletions(-) diff --git a/.claude/skills/shipping-tsim-changes/SKILL.md b/.claude/skills/shipping-tsim-changes/SKILL.md index aa961cb0..5781a6fc 100644 --- a/.claude/skills/shipping-tsim-changes/SKILL.md +++ b/.claude/skills/shipping-tsim-changes/SKILL.md @@ -1,6 +1,6 @@ --- name: shipping-tsim-changes -description: Takes a change to Tsim's code from a new branch to a reviewed local commit, with the checks, evidence and git rules this repository uses. Use when starting a feature, bug fix or optimization in the Tsim repository, when asked to open or update a pull request, or before saying that a change is done. +description: Takes a change to Tsim's code from a branch to a reviewed local commit, with the checks, evidence and git rules this repository uses, scaled to what the change touches. Use when starting a feature, bug fix or optimization in the Tsim repository, when updating an existing pull request, or before saying that a change is done. metadata: internal: true --- @@ -9,11 +9,21 @@ metadata: ## 1. Start -- Work in a worktree on a new branch off the newest main, not in the main checkout: - `git fetch origin main && git worktree add -b / ../tsim- origin/main`, - then `uv sync` there. Stack on another branch only when the change needs it. -- Agree on what "done" means before writing code: the behavior, the test that shows - it, and for performance work the circuits and the size of the gain that matters. +**New work.** Work in a worktree on a new branch off the newest main, not in the main +checkout: `git fetch origin main && git worktree add -b / ../tsim- +origin/main`, then `uv sync` there. Stack on another branch only when the change needs it. + +**Updating an existing pull request** (review comments, a follow-up, a fix the user asks +for on that PR). Keep its branch: find it with `gh pr view --json headRefName`, use +the worktree that already has it (`git worktree list`) or add one for that branch, and +commit on top. Do not start a new branch or a new PR for it. Rebase only when the user +asks or the branch conflicts with its base; a rebase rewrites a pushed branch, so say +that it needs a force-push. + +**Acceptance criteria.** Infer them from the task and the code, and state them in a line +or two before you start: the behavior, the test that shows it, and for performance work +the circuits and the gain that matters. Ask only when they are unclear, for example an +unstated performance target, two plausible behaviors, or a change to public API. ## 2. Build @@ -24,32 +34,51 @@ metadata: ## 3. Verify -Run these and keep their output. A claim without the command and its result is not +Run what applies and keep the output. A claim without the command and its result is not evidence. | Change | Required | |---|---| | Any | `uv run tsim-dev check` prints PASS for lint and tests | -| Touches `src/tsim` outside docstrings | `uv run tsim-dev bench --ref origin/main` exits 0 | -| Performance | the same with `--suite full`; see the `benchmarking-tsim` skill | +| Could affect compile or sampling cost (see below) | `uv run tsim-dev bench --ref origin/main`; any regression investigated and explained | +| Aims to make Tsim faster | the same with `--suite full`; see the `benchmarking-tsim` skill | +| Could change the active width (graph, simplification, scheduling, plan) | `uv run tsim-dev bench --width --ref origin/main`; any `wider` explained | | Bug fix | the new test fails on `origin/main` and passes on the branch | | User-visible | a `CHANGELOG.md` entry under `## [Unreleased]` | | Behavior the user skills describe | `skills/` updated; `test/unit/test_skills.py` passes | -Then read your own diff (`git diff origin/main --stat`, then the diff). Remove -anything the task did not need: special cases for one benchmark, leftover debug -code, unrelated reformatting. Ask whether a simpler change would do. +**When to benchmark.** Code on the compile path (parsing, the ZX graph and its +simplification, components, the plan, scheduling, lowering to the action tape) or the +sampling path (samplers, noise, the state-vector executor, numba kernels), and changes +to dependencies or numba settings, can change performance. Docs, tests, type hints, +error messages, printing and code that runs once per call outside those paths do not. +When unsure, run the quick suite: it takes a few minutes. + +**A regression is a finding, not a veto.** If the benchmark reports `slower` (exit 1), +do not stop at the exit code and do not hide it: + +1. Confirm it: rerun the affected circuits with `--rounds 9 --min-seconds 1`. +2. Find the cause: `tsim-dev profile` on one affected circuit, on both checkouts. +3. Remove it if the change does not need it. If it does, for example a correctness fix + that has to do more work, keep it and say so: which circuits, by how much, and why. + A wrong result is never the faster option. + +Then read your own diff (`git diff origin/main --stat`, then the diff). Remove anything +the task did not need: special cases for one benchmark, leftover debug code, unrelated +reformatting. Ask whether a simpler change would do. ## 4. Commit and review - Commit locally with a Conventional Commits message. -- Do not push, open a pull request, force-push or merge unless the user asked for - that step in this conversation. Some branches are not meant to be public yet. -- For a review, give a fresh subagent the diff and the task, not your conclusions, - and ask for correctness findings only. Fix what it finds, then run step 3 again. +- Do not push, open a pull request, force-push or merge unless the user asked for that + step in this conversation. Some branches are not meant to be public yet. When updating + an existing PR, pushing to its branch still needs the user's go-ahead. +- For a review, give a fresh subagent the diff and the task, not your conclusions, and + ask for correctness findings only. Fix what it finds, then run step 3 again. ## 5. Report Say in a few sentences what changed, the verification commands you ran with their -results, and what you did not verify. For performance work include the bench table, -the CPU model and the thread count. Say plainly if something failed or was skipped. +results, and what you did not verify. For performance-relevant changes include the bench +table, the CPU model and the thread count, and the explanation of any regression. Say +plainly if something failed or was skipped. diff --git a/AGENTS.md b/AGENTS.md index 23383480..19f54801 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -92,6 +92,11 @@ more than a handful of T gates: their compile memory grows exponentially. - Measure with `tsim-dev bench --ref origin/main`, never with a script of your own. It runs fresh processes on one pinned thread with a warm numba cache, excludes compile time, and interleaves the two checkouts. A speedup must hold on `--suite full`, across families. +- Benchmark changes that could affect compile or sampling cost (see the + `shipping-tsim-changes` skill), not every change. A regression is investigated and + explained, not hidden; a correctness fix may cost performance if the cost is stated. +- `tsim-dev bench --width --ref origin/main` compares active widths without sampling, for + changes to the graph, simplification, scheduling or the plan. - Numba caches compiled kernels on disk (`utils/kernels.py`). Cached kernels have behaved differently from freshly compiled ones (passing large structs by value was 5 to 8 times slower when cached), which is why the benchmark uses the cache. After changing a jitted @@ -123,11 +128,12 @@ Agents share this machine with people and other agents. ## Pull requests -- Branch off `origin/main` in a worktree; use - [Conventional Commits](https://www.conventionalcommits.org/). +- New work: branch off `origin/main` in a worktree. Updating an existing pull request: keep + its branch and commit on top. Use [Conventional Commits](https://www.conventionalcommits.org/). - Commit locally. Push, open, force-push or merge a pull request only when asked. - Add a `CHANGELOG.md` entry under `## [Unreleased]` for any user-visible change. -- Bug fixes include a minimal reproducer as a test that fails without the fix; performance - changes include the `tsim-dev bench` table, the CPU model and the thread count. +- Bug fixes include a minimal reproducer as a test that fails without the fix; + performance-relevant changes include the `tsim-dev bench` table, the CPU model, the + thread count and the explanation of any regression. - `tsim-dev check` must pass before review. Report the commands you ran and their results, and say what you did not verify. From 749d4d66fd952f971df7fff3f2a5ca5ea3b126cb Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 09:14:11 -0700 Subject: [PATCH 10/11] fix(dev): pyright errors in the width harness; type-check dev/ in pre-commit CI runs pyright over the whole repository, the pre-commit hook only over src and test, so these were missed locally. The width worker stream now yields lines and records how the worker ended in an `Exit` dataclass, and the width-change message no longer sorts a set that may hold None. The hook now also checks dev/. Co-Authored-By: Claude Opus 5.5 --- .pre-commit-config.yaml | 2 +- dev/tsim_dev/bench.py | 3 ++- dev/tsim_dev/width.py | 50 +++++++++++++++++++++++---------------- test/unit/test_dev_cli.py | 15 ++++++------ 4 files changed, 39 insertions(+), 31 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 925c186f..ab8e5328 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -40,7 +40,7 @@ repos: always_run: false - id: pyright-check name: pyright-check - entry: uv run pyright src test + entry: uv run pyright src test dev language: system pass_filenames: false always_run: false diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py index 6368d007..640bf4ab 100644 --- a/dev/tsim_dev/bench.py +++ b/dev/tsim_dev/bench.py @@ -315,7 +315,8 @@ def run_case(case: Case, sides: list[Side], args) -> dict: widths = {r.get("active_width") for r in rounds[side.describe()]} if len(widths) > 1: record["error"] = ( - f"{side.describe()}: active width changed between runs: {sorted(widths)}" + f"{side.describe()}: active width changed between runs: " + + ", ".join(sorted(str(w) for w in widths)) ) return record new = rounds[sides[0].describe()] diff --git a/dev/tsim_dev/width.py b/dev/tsim_dev/width.py index 0bd85643..8cf63df9 100644 --- a/dev/tsim_dev/width.py +++ b/dev/tsim_dev/width.py @@ -15,6 +15,8 @@ import statistics import subprocess import time +from collections.abc import Iterator +from dataclasses import dataclass from . import common, corpus from .common import FAILED, INFRA, OK @@ -34,11 +36,20 @@ def _items(cases) -> list[tuple[str, str, str]]: ] -def _stream(cmd, env, case_timeout: int): - """Run the worker; yield its lines, and stop it if one circuit compiles too long. +@dataclass +class Exit: + """How a worker ended: its exit code and stderr, or a stop for taking too long.""" - Yields `("line", text)` for each output line, then `("exit", (code, stderr))`; - `code` is "timeout" when the worker was stopped. + code: int = 0 + stderr: str = "" + timed_out: bool = False + + +def _stream(cmd, env, case_timeout: int, outcome: Exit) -> Iterator[str]: + """Run the worker, yield its output lines, and record how it ended in `outcome`. + + The worker is stopped if no line arrives for `case_timeout` seconds, that is if + one circuit compiles for longer than that. """ with subprocess.Popen( cmd, env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True @@ -52,14 +63,14 @@ def _stream(cmd, env, case_timeout: int): if not line: break deadline = time.monotonic() + case_timeout - yield "line", line.rstrip("\n") + yield line.rstrip("\n") elif time.monotonic() > deadline: process.kill() process.wait() - yield "exit", ("timeout", "") + outcome.timed_out = True return - code = process.wait() - yield "exit", (code, process.stderr.read()) + outcome.code = process.wait() + outcome.stderr = process.stderr.read() def measure(side, cases, args) -> dict[str, dict]: @@ -76,26 +87,23 @@ def measure(side, cases, args) -> dict[str, dict]: for _, flag, value in pending: cmd += [flag, value] env = side_env(side, side.threads or args.threads) - started, code, stderr = None, 0, "" - for kind, value in _stream( - common.capped(cmd, mem=args.mem), env, args.case_timeout - ): - if kind == "exit": - code, stderr = value - elif value.startswith("START "): - started = value[6:] - elif value.startswith("RESULT "): - result = json.loads(value[7:]) + started, outcome = None, Exit() + capped = common.capped(cmd, mem=args.mem) + for line in _stream(capped, env, args.case_timeout, outcome): + if line.startswith("START "): + started = line[6:] + elif line.startswith("RESULT "): + result = json.loads(line[7:]) results[result.pop("name")] = result missing = [item for item in pending if item[0] not in results] if not missing: break # NOTE: the worker died or was stopped while compiling `started`. - if code == "timeout": + if outcome.timed_out: reason = f"compile took over {args.case_timeout} s" else: - reason = common.explain_exit(code, args.mem, None) - reason += ": " + (stderr.strip().splitlines() or [""])[-1] + reason = common.explain_exit(outcome.code, args.mem, None) + reason += ": " + (outcome.stderr.strip().splitlines() or [""])[-1] error = {"error": reason[:200]} if started is None or started not in {key for key, _, _ in missing}: results.update({key: dict(error) for key, _, _ in missing}) diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index 08a6872b..af788595 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -341,16 +341,15 @@ def test_width_verdicts_treat_refused_widths_as_lower_bounds(new, ref, expected) def test_width_mode_resumes_after_the_worker_dies(fake_bench, monkeypatch): calls = [] - def fake_stream(cmd, env, case_timeout): + def fake_stream(cmd, env, case_timeout, outcome): cases = [cmd[i + 1] for i, x in enumerate(cmd) if x == "--case"] calls.append(cases) for case in cases: - yield "line", f"START {case}" + yield f"START {case}" if case == "p/narrow" and len(calls) == 1: - yield "exit", ("timeout", "") + outcome.timed_out = True return - yield "line", "RESULT " + json.dumps({"name": case, "width": 3}) - yield "exit", (0, "") + yield "RESULT " + json.dumps({"name": case, "width": 3}) monkeypatch.setattr(width, "_stream", fake_stream) cases = bench.select(_bench_args("--width", "--suite", "corpus")) @@ -362,9 +361,9 @@ def fake_stream(cmd, env, case_timeout): def test_the_width_stream_stops_a_stuck_compile(): script = "import time; print('START a', flush=True); time.sleep(30)" - events = list(width._stream([sys.executable, "-c", script], None, case_timeout=1)) - assert events[0] == ("line", "START a") - assert events[-1] == ("exit", ("timeout", "")) + outcome = width.Exit() + lines = list(width._stream([sys.executable, "-c", script], None, 1, outcome)) + assert lines == ["START a"] and outcome.timed_out def test_shot_benchmarks_compare_and_check_the_active_width(fake_bench, monkeypatch): From 9f0733681edf3d583afc72bb28df2d62cf4a10d1 Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 06:55:38 -0700 Subject: [PATCH 11/11] chore: add a tiered review workflow and merge gate - `tsim-dev review plan|status|record`: the tier of a change comes from the paths and size of its diff. Tier 0 (no src change) needs the check gate; tier 1 adds a benchmark and one fresh-context review; tier 2 (sampling code or more than 300 source lines) adds a second model family, a fuzz run and a last round without findings, capped at 3 rounds. `status` reads the evidence that `check`, `bench` and `fuzz` record for the current commit, the pull request's CI, and its review threads, and exits 0 only when nothing is owed. - Unanswered review threads (Copilot, Codex, posted reviews) block the gate. - `tsim-dev fuzz`: random small circuits against the dense VecSampler, with a confirmation rerun before a mismatch counts. - `reviewing-tsim-changes` skill and a `tsim-reviewer` subagent that reports only findings it reproduced. - The Bash hook blocks `gh pr merge` unless it follows `tsim-dev review status &&`. - AGENTS.md: review rules for automatic and human reviewers. Rebased on the harness's width mode and scaled shipping rules: a benchmark is required by path (the compile and sampling paths and the dependencies), not by tier, and `bench --width` for changes that can alter the active width. A benchmark that finds a regression counts as run; `review status` notes the regression to be explained in the pull request. The reviewing skill is marked internal for the skill installer. Co-Authored-By: Claude Fable 5.2 Co-Authored-By: Claude Opus 5.5 --- .claude/agents/tsim-reviewer.md | 42 ++ .claude/hooks/guard_bash.py | 12 +- .../skills/reviewing-tsim-changes/SKILL.md | 95 ++++ .claude/skills/shipping-tsim-changes/SKILL.md | 5 +- AGENTS.md | 16 + dev/tsim_dev/_fuzz.py | 100 ++++ dev/tsim_dev/bench.py | 16 +- dev/tsim_dev/checks.py | 4 +- dev/tsim_dev/cli.py | 25 +- dev/tsim_dev/review.py | 535 ++++++++++++++++++ dev/tsim_dev/width.py | 13 +- test/unit/test_dev_cli.py | 95 +++- 12 files changed, 950 insertions(+), 8 deletions(-) create mode 100644 .claude/agents/tsim-reviewer.md create mode 100644 .claude/skills/reviewing-tsim-changes/SKILL.md create mode 100644 dev/tsim_dev/_fuzz.py create mode 100644 dev/tsim_dev/review.py diff --git a/.claude/agents/tsim-reviewer.md b/.claude/agents/tsim-reviewer.md new file mode 100644 index 00000000..419ea545 --- /dev/null +++ b/.claude/agents/tsim-reviewer.md @@ -0,0 +1,42 @@ +--- +name: tsim-reviewer +description: Reviews a Tsim diff for correctness and performance bugs in a fresh context and reports only findings it has reproduced. Use for each review round of the reviewing-tsim-changes skill. +tools: Read, Grep, Glob, Bash +--- + +You review a change to Tsim, a sampler for quantum circuits with non-Clifford gates. +The owner will merge this change without reading it, so a bug you miss ships, and a +finding you cannot show wastes a round. Read `AGENTS.md` first for the pipeline and +the invariants. + +You are given a diff command, the task the change is meant to do, and findings that +are already known. Judge the change against the task and the invariants; form your +own view of what the code does by reading it and running it. + +Look for, in this order: + +1. Wrong samples: a changed distribution, a broken invariant from `AGENTS.md` (seeds + independent of threads and `batch_size`, raw parities, direct outputs, + postselection returning only accepted rows), precision loss. +2. Crashes or wrong results on valid input forms the tests do not cover: empty and + single-element cases, zero shots, circuits without detectors or without noise, + merged multi-target instructions, more than 63 outputs, large `batch_size`. +3. Memory or time that grows with shots or 2^k where it did not before. +4. A public API or documented behavior that changed without the task asking for it. +5. Tests that do not test what they claim, or that pass without the change. + +For every finding, write a reproducer and run it: a pytest function or a short +script under `.tsim-dev/review/`, run with `uv run tsim-dev run -- uv run python ...` +or `uv run tsim-dev test `. Use small circuits; compare against the dense +references in `test/helpers/`, against Stim on Clifford circuits, or against exact +values. Do not edit files outside `.tsim-dev/review/`, and do not run long jobs. + +Report, most severe first, only what you reproduced: + +- `file:line`, one sentence stating the defect; +- the input, the expected result and the observed result; +- the path of the reproducer and the command that runs it. + +Leave out style, naming, refactoring ideas, missing comments, anything a linter or +the type checker reports, and anything you could not reproduce. If nothing +survives, say "no findings" and list in two or three lines what you checked. diff --git a/.claude/hooks/guard_bash.py b/.claude/hooks/guard_bash.py index 8160a1c2..fe5ce10a 100755 --- a/.claude/hooks/guard_bash.py +++ b/.claude/hooks/guard_bash.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""PreToolUse hook for Bash: block two commands that have taken sessions down. +"""PreToolUse hook for Bash: block commands that have cost sessions or let bugs out. Exit code 2 blocks the command and shows stderr to the agent. Everything else passes. Keep this list short: it is for rules that must hold every time. @@ -11,6 +11,10 @@ COMPUTE = re.compile(r"\b(python3?|pytest|uv run)\b") CAPPED = re.compile(r"\b(tsim-dev|systemd-run)\b") +MERGE = re.compile(r"\bgh\s+pr\s+merge\b") +GATED = re.compile( + r"\btsim-dev\s+review\s+status\b[^;|\n]*&&[^;|\n]*\bgh\s+pr\s+merge\b" +) DETACHED = re.compile(r"\bnohup\b|\bsetsid\b|&\s*(disown\b|$)", re.MULTILINE) @@ -22,6 +26,12 @@ def verdict(tool_input: dict) -> str | None: "`pkill -f` also matches this shell's own command line and kills it. " "Stop a job with `uv run tsim-dev job stop NAME`, or kill by PID." ) + if MERGE.search(command) and not GATED.search(command): + return ( + "Merge only through the gate, from the pull request's branch: " + "`uv run tsim-dev review status && gh pr merge --squash`. " + "Unanswered review comments have been merged past before." + ) background = tool_input.get("run_in_background") or DETACHED.search(command) if background and COMPUTE.search(command) and not CAPPED.search(command): return ( diff --git a/.claude/skills/reviewing-tsim-changes/SKILL.md b/.claude/skills/reviewing-tsim-changes/SKILL.md new file mode 100644 index 00000000..2b492d84 --- /dev/null +++ b/.claude/skills/reviewing-tsim-changes/SKILL.md @@ -0,0 +1,95 @@ +--- +name: reviewing-tsim-changes +description: Runs the review of a Tsim change before it merges, with the number of reviewers and rounds set by `tsim-dev review plan`, findings that must come with a failing test, and a merge gate that checks bot comments. Use when asked to review a branch or pull request in the Tsim repository, to get a change ready to merge, to handle Copilot or Codex review comments, or to merge a pull request. +metadata: + internal: true +--- + +# Reviewing a Tsim change + +The owner merges without reading the code. What stands in for reading is this +procedure, so do not shorten it and do not add to it: extra rounds on a small diff +have produced only nits here, and missing rounds on a large one have let bugs out. + +## 1. Find out what the change owes + +```bash +uv run tsim-dev review plan # tier, reasons, required steps +uv run tsim-dev review status # the same, checked; exit 0 ready, 1 to do, 3 waiting +``` + +| Tier | When | Reviews | +|---|---|---| +| 0 | no change in `src/tsim` | none | +| 1 | other changes in `src/tsim` | one, in a fresh context | +| 2 | sampling code (`statevector/`, `noise/`, `compile/`, `sampler.py`, kernels) or more than 300 changed source lines | two, from different model families, then rounds until one has no findings; at most 3 rounds | + +Benchmarks do not depend on the tier: `plan` asks for `tsim-dev bench --ref ` +when the diff touches the compile or sampling path or the dependencies, and for +`bench --width` when it can change the active width. A benchmark that finds a +regression still counts as run; `status` notes the regression, and it has to be +confirmed, profiled and explained in the pull request (see the +`shipping-tsim-changes` skill). + +Notes printed by `plan`: `SPLIT` (over 600 source lines: split into stacked pull +requests before reviewing), `HUMAN` (tell the user which files they should read), +`TESTS` (explain removed tests). + +## 2. Run a review round + +Start the reviewer as a subagent with a fresh context (`tsim-reviewer` in +`.claude/agents/`). For the second family in tier 2, use the model the user names, +or ask which one is available. Give the reviewer: + +- the command for the diff (`git diff ...HEAD`), and for a later round + only the diff since the last reviewed commit; +- the task: the issue or the user's request, in their words; +- the findings already known, so that it does not repeat them. + +Do not give it the pull request description, your summary, or your opinion of the +change. Reviewers that read the author's account find fewer bugs. + +## 3. Accept only findings with a failing test + +A finding counts when the reviewer gives a test or a short script that fails on the +branch, with the expected and the observed result. For each one: + +1. Run the reproducer yourself. If it does not fail, drop the finding. +2. Add it to the test suite, fix the code, and show the test pass. +3. For a performance finding, the evidence is `tsim-dev bench`. + +Do not act on unconfirmed findings, style remarks or suggested abstractions. Do not +weaken or delete a test to make a finding go away. + +Then record the round, with the number of confirmed findings: + +```bash +uv run tsim-dev review record --model opus-5.5 --findings 2 +``` + +If round 3 of a tier 2 change still has findings, stop. Tell the user that the +change does not converge and propose a split or another design. + +## 4. Bot comments on the pull request + +Copilot and Codex review automatically within about five minutes of a push; of +their comments here, about half pointed at real bugs and almost none were wrong. +`review status` lists every thread that has no reply. + +- Real: fix it, with a test where one is possible, and reply with the commit. +- Not real or out of scope: reply with the reason in one sentence. For something + real but out of scope, open an issue and link it. +- Never merge with unanswered threads, and wait for reviewers that `status` reports + as pending. + +## 5. Merge + +Only when the user asked for the merge, and only as + +```bash +uv run tsim-dev review status && gh pr merge --squash +``` + +from the pull request's branch. A hook blocks `gh pr merge` in any other form. In +your report, paste the output of `review status`, and list anything it marked as a +note (files for a person to read, removed tests). diff --git a/.claude/skills/shipping-tsim-changes/SKILL.md b/.claude/skills/shipping-tsim-changes/SKILL.md index 5781a6fc..fe45cc21 100644 --- a/.claude/skills/shipping-tsim-changes/SKILL.md +++ b/.claude/skills/shipping-tsim-changes/SKILL.md @@ -73,8 +73,9 @@ reformatting. Ask whether a simpler change would do. - Do not push, open a pull request, force-push or merge unless the user asked for that step in this conversation. Some branches are not meant to be public yet. When updating an existing PR, pushing to its branch still needs the user's go-ahead. -- For a review, give a fresh subagent the diff and the task, not your conclusions, and - ask for correctness findings only. Fix what it finds, then run step 3 again. +- Review and merge follow the `reviewing-tsim-changes` skill: + `uv run tsim-dev review plan` says how many reviews the change needs, and + `uv run tsim-dev review status` says when it is ready. ## 5. Report diff --git a/AGENTS.md b/AGENTS.md index 19f54801..8cc036e9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,6 +16,8 @@ uv run tsim-dev check # the gate before review: pre-commit, fu uv run tsim-dev test --changed # tests of the changed files; or give paths uv run tsim-dev bench --ref origin/main # quick A/B benchmark, ~4 min; exit 1 if slower uv run tsim-dev bench --width --ref origin/main # active widths only, nothing sampled +uv run tsim-dev review status # what the change still owes before merge +uv run tsim-dev fuzz # 10 min differential fuzz against a dense simulator uv run tsim-dev inspect FILE.stim # qubits, T count, active width, stats; no sampling uv run tsim-dev profile cultivation_d5 # where compile and sampling time goes (py-spy) uv run tsim-dev run -- CMD # any command under memory, CPU and time caps @@ -31,6 +33,7 @@ just doc-build # docs build and Mintlify checks (needs are not published to users. - `shipping-tsim-changes`: branch, verify, commit, review; what evidence a change needs. +- `reviewing-tsim-changes`: review tiers, findings with failing tests, bot comments, merge. - `benchmarking-tsim`: A/B benchmarks, reading verdicts, the tsim-bench corpus. - `profiling-tsim`: sizing a circuit, reading profiles, choosing what to optimize. - `testing-tsim-correctness`: oracles and statistical tests for sampling changes. @@ -137,3 +140,16 @@ Agents share this machine with people and other agents. thread count and the explanation of any regression. - `tsim-dev check` must pass before review. Report the commands you ran and their results, and say what you did not verify. +- Merge only when `tsim-dev review status` exits 0: every review thread, including Copilot's + and Codex's, has a fix or a reply. + +## Code review rules + +For any reviewer of a pull request here, human or automatic. + +- Report defects that change behavior: wrong samples, a broken invariant from the list above, + crashes on valid input, memory or time that grows where it did not, an unintended change + to the public API, a test that passes without the change. +- Give the input, the expected result and the observed result. A finding without a way to + reproduce it is not acted on. +- Do not report style, naming, formatting or typing; pre-commit enforces those. diff --git a/dev/tsim_dev/_fuzz.py b/dev/tsim_dev/_fuzz.py new file mode 100644 index 00000000..0e72a485 --- /dev/null +++ b/dev/tsim_dev/_fuzz.py @@ -0,0 +1,100 @@ +"""Differential fuzz worker: random small circuits against a dense simulator. + +Run from the repository root with the checkout's Python; it prints `RESULT `. +Each circuit is sampled with `active_statevector` and with `VecSampler`, an +independent dense state-vector simulator, and the two sets of measurement records +are compared outcome by outcome. A mismatch is sampled again with more shots and +other seeds before it counts, so that a statistical fluke does not fail the run. +""" + +from __future__ import annotations + +import argparse +import json +import random +import time +from pathlib import Path + +# NOTE: the dense simulator has no PAULI_CHANNEL gates, so they are left out. +NOISE = ("p_x_error", "p_z_error", "p_y_error", "p_depolarize1", "p_depolarize2") +ROTATIONS = ("p_r_x", "p_r_y", "p_r_z", "p_u3") + + +def random_case(seed: int) -> dict: + """Draw generator settings: small circuits with varied gates, noise and depth.""" + rng = random.Random(seed) + settings: dict[str, float] = { + "qubits": rng.randint(1, 5), + "depth": rng.randint(5, 90), + "p_t": rng.choice([0, 1, 3]), + "seed": seed, + } + for name in rng.sample(NOISE, rng.randint(0, 3)): + settings[name] = rng.choice([0.3, 1.0]) + for name in rng.sample(ROTATIONS, rng.randint(0, 2)): + settings[name] = rng.choice([0.5, 1.0]) + return settings + + +def mismatch(stim_circuit, shots: int, seed: int) -> str | None: + """Return a description if Tsim and the dense simulator disagree, else None.""" + from test.helpers.statevector import assert_counts_match + + from tsim.circuit import Circuit + from tsim.external.vec_sim.vec_sampler import VecSampler + + expected, _, _ = VecSampler(stim_circuit).sample(shots) + sampler = Circuit.from_stim_program(stim_circuit).compile_sampler(seed=seed) + got = sampler.sample(shots) + try: + assert_counts_match(got, expected) + except AssertionError: + return "measurement distribution differs from the dense simulator" + packed = Circuit.from_stim_program(stim_circuit).compile_sampler(seed=seed) + import numpy as np + + unpacked = np.unpackbits( + packed.sample(shots, bit_packed=True), axis=1, bitorder="little" + )[:, : got.shape[1]] + if not np.array_equal(unpacked.astype(bool), got.astype(bool)): + return "bit-packed samples differ from unpacked samples with the same seed" + return None + + +def main() -> None: + """Fuzz for the given time and report circuits, failures and their files.""" + parser = argparse.ArgumentParser() + parser.add_argument("--seconds", type=float, default=600) + parser.add_argument("--start", type=int, default=0) + parser.add_argument("--shots", type=int, default=2000) + parser.add_argument("--out", required=True) + args = parser.parse_args() + + from test.helpers.gen import gen_stim_circuit + + out = Path(args.out) + out.mkdir(parents=True, exist_ok=True) + end = time.time() + args.seconds + seed, failures = args.start, [] + while time.time() < end: + settings = random_case(seed) + stim_circuit = gen_stim_circuit(**settings) + try: + problem = mismatch(stim_circuit, args.shots, seed) + if problem: + # NOTE: confirm with ten times the shots and a fresh seed. + problem = mismatch(stim_circuit, 10 * args.shots, seed + 1_000_003) + except Exception as error: + problem = f"{type(error).__name__}: {error}"[:300] + if problem: + path = out / f"fail-{seed}.stim" + path.write_text(str(stim_circuit)) + failures.append({"seed": seed, "problem": problem, "circuit": str(path)}) + print(f"FAILURE seed={seed}: {problem} ({path})", flush=True) + seed += 1 + result = {"circuits": seed - args.start, "start": args.start, "failures": failures} + print("RESULT " + json.dumps(result)) + + +if __name__ == "__main__": + main() diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py index 640bf4ab..80127d23 100644 --- a/dev/tsim_dev/bench.py +++ b/dev/tsim_dev/bench.py @@ -18,7 +18,7 @@ from dataclasses import dataclass from pathlib import Path -from . import common, corpus, width +from . import common, corpus, review, width from .common import FAILED, INFRA, OK, InfraError @@ -493,6 +493,20 @@ def run(args) -> int: if wider or narrower: summary += f", active width: {wider} wider, {narrower} narrower" print(f"{summary}. Details: {out}") + # NOTE: only a comparison of two checkouts over a whole suite, with the recorded + # postselection, counts as evidence for a review (not --side setting comparisons). + whole_suite = not (args.circuits or args.only or args.max_width is not None) + if args.ref and whole_suite and not args.no_postselect: + # NOTE: a regression does not fail the evidence: it has to be explained. + review.write_evidence( + "bench", + not errors, + ref=sides[1].label, + suite=args.suite, + slower=slower, + wider=wider, + kept_differs=differs, + ) if errors: return INFRA return FAILED if slower or differs or wider else OK diff --git a/dev/tsim_dev/checks.py b/dev/tsim_dev/checks.py index ccfd9d3f..7124a168 100644 --- a/dev/tsim_dev/checks.py +++ b/dev/tsim_dev/checks.py @@ -7,7 +7,7 @@ import time from pathlib import Path -from . import common +from . import common, review from .common import FAILED, OK, InfraError @@ -158,4 +158,6 @@ def check(args) -> int: print( f"{verdict} tests: {last.strip('= ')} ({time.time() - start:.0f} s). Log: {log}" ) + if not args.no_tests: + review.write_evidence("check", not failed) return FAILED if failed else OK diff --git a/dev/tsim_dev/cli.py b/dev/tsim_dev/cli.py index 408cec38..a5b5dff7 100644 --- a/dev/tsim_dev/cli.py +++ b/dev/tsim_dev/cli.py @@ -9,7 +9,7 @@ import argparse import sys -from . import bench, checks, jobs, profile +from . import bench, checks, jobs, profile, review def _caps(parser, mem: str, timeout: int, cpus: float | None = None) -> None: @@ -166,6 +166,29 @@ def build_parser() -> argparse.ArgumentParser: _caps(p, "8G", 600) p.set_defaults(func=profile.profile) + p = sub.add_parser("fuzz", help="differential fuzz against a dense simulator") + p.add_argument("--seconds", type=float, default=600, help="default 600") + p.add_argument("--start", type=int, default=0, help="first seed (default 0)") + p.add_argument("--mem", default="4G", help="memory cap (default 4G)") + p.set_defaults(func=review.fuzz) + + rev = sub.add_parser("review", help="what a change owes before it merges") + rev_sub = rev.add_subparsers(dest="review_command", required=True) + base_help = "diff base (default: the pull request's base, else origin/main)" + p = rev_sub.add_parser("plan", help="tier of the diff and the required steps") + p.add_argument("--base", help=base_help) + p.set_defaults(func=review.plan) + p = rev_sub.add_parser("status", help="check the requirements; exit 0 when ready") + p.add_argument("--base", help=base_help) + p.set_defaults(func=review.status) + p = rev_sub.add_parser("record", help="record a finished review round") + p.add_argument("--model", required=True, help="the reviewing model") + p.add_argument( + "--findings", type=int, required=True, help="confirmed findings of this round" + ) + p.add_argument("--note", default="") + p.set_defaults(func=review.record) + p = sub.add_parser( "run", help="run `-- command ...` under memory, CPU and time caps" ) diff --git a/dev/tsim_dev/review.py b/dev/tsim_dev/review.py new file mode 100644 index 00000000..8b4fe3f0 --- /dev/null +++ b/dev/tsim_dev/review.py @@ -0,0 +1,535 @@ +"""`tsim-dev review` and `tsim-dev fuzz`: what a change owes before it merges. + +The tier comes from the paths and the size of the diff, not from the author: + +- tier 0 (docs, tests, tooling): the `check` gate. +- tier 1 (other changes in `src/tsim`): tier 0 and one review in a fresh context. +- tier 2 (sampling semantics, or a large diff): tier 1, a second review by another + model, a short differential fuzz run, and a last review round with no findings. + +Independently of the tier, a diff on the compile or sampling path (or to the +dependencies) needs a benchmark against the base, and one that can change the active +width needs `bench --width`. A benchmark that finds a regression still counts as run; +the regression has to be explained in the pull request, not removed at any cost. + +`review status` reads the evidence that the other commands leave in `.tsim-dev/` +and the pull request's review threads, and prints what is still missing. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from dataclasses import dataclass, field +from pathlib import Path + +from . import common +from .common import FAILED, INFRA, OK, RUNNING, InfraError + +# Paths whose changes can alter samples: tier 2. +SAMPLING = ( + "src/tsim/statevector/", + "src/tsim/noise/", + "src/tsim/compile/", + "src/tsim/sampler.py", + "src/tsim/core/graph.py", + "src/tsim/utils/kernels.py", + "src/tsim/utils/rng.py", +) +# Paths whose changes can alter compile or sampling cost: a benchmark is needed. +PERFORMANCE = ( + "src/tsim/circuit.py", + "src/tsim/core/", + "src/tsim/compile/", + "src/tsim/noise/channels.py", + "src/tsim/statevector/", + "src/tsim/sampler.py", + "src/tsim/utils/kernels.py", + "src/tsim/utils/linalg.py", + "src/tsim/utils/clifford.py", + "src/tsim/utils/rng.py", + "pyproject.toml", + "uv.lock", +) +# Paths whose changes can alter the active width: `bench --width` is needed. +WIDTH = ( + "src/tsim/circuit.py", + "src/tsim/core/", + "src/tsim/compile/", + "src/tsim/statevector/pbc.py", + "src/tsim/statevector/plan.py", + "src/tsim/statevector/program.py", + "src/tsim/statevector/schedule.py", +) +# Paths a person should read: they change what the gates themselves enforce, or +# what users may rely on. +HUMAN = ( + ".github/workflows/", + ".claude/settings.json", + ".claude/hooks/", + "dev/tsim_dev/review.py", + "src/tsim/__init__.py", + "test/unit/test_public_api.py", +) +LARGE = 300 # changed source lines that make a diff tier 2 +SPLIT = 600 # changed source lines above which reviews have missed bugs +MAX_ROUNDS = 3 + + +@dataclass +class Plan: + """The tier of a diff and the reasons for it.""" + + tier: int + base: str + source_lines: int + reasons: list[str] = field(default_factory=list) + notes: list[str] = field(default_factory=list) + bench: bool = False # touches the compile or sampling path + width: bool = False # can change the active width + + +def classify(stat: list[tuple[int, int, str]], base: str = "") -> Plan: + """Classify a diff given as (added, deleted, path) rows.""" + source = [row for row in stat if row[2].startswith("src/tsim/")] + source = [row for row in source if not row[2].startswith("src/tsim/external/")] + lines = sum(added + deleted for added, deleted, _ in source) + plan = Plan(0, base, lines) + if source: + plan.tier = 1 + plan.reasons.append(f"{len(source)} files in src/tsim, {lines} changed lines") + sampling = sorted({p for _, _, p in source if p.startswith(SAMPLING)}) + if sampling: + plan.tier = 2 + plan.reasons.append("can change samples: " + ", ".join(sampling[:4])) + if lines > LARGE: + plan.tier = 2 + plan.reasons.append(f"more than {LARGE} changed source lines") + if not source: + plan.reasons.append("no changes in src/tsim") + perf = sorted({p for _, _, p in stat if p.startswith(PERFORMANCE)}) + if perf: + plan.bench = True + plan.reasons.append("can change performance: " + ", ".join(perf[:4])) + plan.width = any(p.startswith(WIDTH) for _, _, p in stat) + if lines > SPLIT: + plan.notes.append( + f"SPLIT: {lines} changed source lines. Diffs above {SPLIT} have let " + "bugs through review here; split it into stacked pull requests." + ) + human = sorted({p for _, _, p in stat if p.startswith(HUMAN)}) + if human: + plan.notes.append("HUMAN: a person should read " + ", ".join(human)) + removed = sum(d for _, d, p in stat if p.startswith("test/")) + added = sum(a for a, _, p in stat if p.startswith("test/")) + if removed > 20 and removed > added: + plan.notes.append( + f"TESTS: {removed} test lines removed, {added} added. Say why in the " + "pull request; tests are not to be weakened to make a change pass." + ) + return plan + + +def pull_request() -> dict | None: + """Return the open pull request of the current branch, if there is one.""" + done = subprocess.run( + ["gh", "pr", "view", "--json", "number,baseRefName,headRefOid,state,url"], + cwd=common.root(), + capture_output=True, + text=True, + check=False, + ) + if done.returncode: + return None + pr = json.loads(done.stdout) + return pr if pr.get("state") == "OPEN" else None + + +def resolve_base(explicit: str | None, pr: dict | None) -> str: + """Pick the base to diff against: the option, the pull request's base, or main.""" + if explicit: + return explicit + if pr: + return f"origin/{pr['baseRefName']}" + return "origin/main" + + +def make_plan(base: str) -> Plan: + """Classify the diff between the merge base with `base` and HEAD.""" + root = common.root() + merge_base = common.git("merge-base", "HEAD", base, cwd=root) + rows = [] + for line in common.git( + "diff", "--numstat", merge_base, "HEAD", cwd=root + ).splitlines(): + added, deleted, path = line.split("\t", 2) + rows.append( + ( + int(added) if added.isdigit() else 0, + int(deleted) if deleted.isdigit() else 0, + path, + ) + ) + return classify(rows, base) + + +def head() -> str: + """Return the full commit id of HEAD.""" + return common.git("rev-parse", "HEAD", cwd=common.root()) + + +def is_dirty() -> bool: + """Return whether tracked files have uncommitted changes.""" + return bool( + common.git("status", "--porcelain", "--untracked-files=no", cwd=common.root()) + ) + + +def evidence_path(kind: str) -> Path: + """Return the file that records the last run of a gate.""" + return common.out_dir("evidence") / f"{kind}.json" + + +def write_evidence(kind: str, passed: bool, **extra) -> None: + """Record that a gate ran on the current commit.""" + record = {"head": head(), "dirty": is_dirty(), "passed": passed, **extra} + evidence_path(kind).write_text(json.dumps(record)) + + +def evidence_record(kind: str) -> dict: + """Return the last recorded run of a gate, or an empty dict.""" + path = evidence_path(kind) + return json.loads(path.read_text()) if path.exists() else {} + + +def regressions(record: dict) -> str: + """Describe the regressions a benchmark run found, or return "".""" + found = [ + f"{record[key]} {label}" + for key, label in ( + ("slower", "slower"), + ("wider", "wider"), + ("kept_differs", "with a different kept fraction"), + ) + if record.get(key) + ] + return ", ".join(found) + + +def read_evidence(kind: str) -> tuple[bool, str]: + """Return whether a gate passed on exactly this commit, and a description.""" + path = evidence_path(kind) + if not path.exists(): + return False, "not run" + record = json.loads(path.read_text()) + if record["head"] != head() or record["dirty"] or is_dirty(): + return False, "ran on other code than the current commit; run it again" + return bool(record["passed"]), "passed" if record["passed"] else "failed" + + +def reviews_path() -> Path: + """Return the review log of the current branch.""" + branch = common.git("rev-parse", "--abbrev-ref", "HEAD", cwd=common.root()) + return common.out_dir("reviews") / (branch.replace("/", "__") + ".jsonl") + + +def read_reviews() -> list[dict]: + """Return the recorded review rounds of the current branch.""" + path = reviews_path() + if not path.exists(): + return [] + return [json.loads(line) for line in path.read_text().splitlines() if line.strip()] + + +def family(model: str) -> str: + """Reduce a model name to its family, so two Claude models count as one.""" + name = model.lower() + for key, value in ( + ("claude", "anthropic"), + ("opus", "anthropic"), + ("sonnet", "anthropic"), + ("fable", "anthropic"), + ("haiku", "anthropic"), + ("gpt", "openai"), + ("codex", "openai"), + ("astra", "openai"), + ("gemini", "google"), + ): + if key in name: + return value + return name + + +def review_gaps(tier: int, reviews: list[dict]) -> list[str]: + """Return what the recorded reviews still lack for this tier.""" + if tier == 0: + return [] + if not reviews: + return ["no review recorded; see the reviewing-tsim-changes skill"] + gaps = [] + if tier == 2: + if len({family(r["model"]) for r in reviews}) < 2: + gaps.append("tier 2 needs a second review by a model from another family") + last = reviews[-1] + if last["findings"]: + if len(reviews) >= MAX_ROUNDS: + gaps.append( + f"round {len(reviews)} still had findings. Stop reviewing: " + "rethink the approach or split the change, and tell the user" + ) + else: + gaps.append( + "the last round had findings; after the fixes, run a round on " + "the new diff only, until one has none" + ) + return gaps + + +def open_threads(pr: dict) -> tuple[list[str], list[str]]: + """Return unanswered review threads and reviewers that have not reported yet. + + A thread counts as answered when it is resolved or has a reply. + """ + query = """query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){ + pullRequest(number:$number){ + reviewRequests(first:20){nodes{requestedReviewer{__typename ... on Bot{login} ... on User{login}}}} + reviewThreads(first:100){nodes{isResolved path line + comments(first:30){nodes{author{login} body url}}}}}}}""" + owner, name = ( + common.git("remote", "get-url", "origin", cwd=common.root()) + .removesuffix(".git") + .replace(":", "/") + .split("/")[-2:] + ) + done = subprocess.run( + [ + "gh", + "api", + "graphql", + "-f", + f"query={query}", + "-F", + f"owner={owner}", + "-F", + f"name={name}", + "-F", + f"number={pr['number']}", + ], + capture_output=True, + text=True, + check=False, + ) + if done.returncode: + raise InfraError(f"gh api graphql: {done.stderr.strip()[-200:]}") + data = json.loads(done.stdout)["data"]["repository"]["pullRequest"] + threads = [] + for thread in data["reviewThreads"]["nodes"]: + comments = thread["comments"]["nodes"] + if thread["isResolved"] or len(comments) != 1: + continue + first = comments[0] + text = " ".join(first["body"].split()) + text = text.split("")[-1].strip(" *")[:90] + author = (first["author"] or {}).get("login", "?") + threads.append( + f"{thread['path']}:{thread['line']} [{author}] {text}\n {first['url']}" + ) + pending = [ + (node["requestedReviewer"] or {}).get("login", "?") + for node in data["reviewRequests"]["nodes"] + if (node["requestedReviewer"] or {}).get("__typename") == "Bot" + ] + return threads, pending + + +def ci_state(pr: dict) -> tuple[str, list[str]]: + """Return "pass", "fail" or "pending" for the pull request's checks.""" + done = subprocess.run( + ["gh", "pr", "checks", str(pr["number"]), "--json", "name,bucket"], + cwd=common.root(), + capture_output=True, + text=True, + check=False, + ) + try: + checks = json.loads(done.stdout) + except ValueError: + return "pending", ["no checks reported yet"] + failed = [c["name"] for c in checks if c["bucket"] in ("fail", "cancel")] + if failed: + return "fail", failed + waiting = [c["name"] for c in checks if c["bucket"] == "pending"] + return ("pending", waiting) if waiting else ("pass", []) + + +def print_plan(plan: Plan) -> None: + """Print the tier, why, and what it requires.""" + print(f"tier {plan.tier} against {plan.base}: " + "; ".join(plan.reasons)) + for note in plan.notes: + print(note) + steps = ["uv run tsim-dev check"] + if plan.bench: + steps.append( + f"uv run tsim-dev bench --ref {plan.base}; explain any regression in the PR" + ) + if plan.width: + steps.append(f"uv run tsim-dev bench --width --ref {plan.base}") + if plan.tier >= 1: + steps.append("one review in a fresh context (reviewing-tsim-changes skill)") + if plan.tier >= 2: + steps.append("a second review by a model from another family") + steps.append("uv run tsim-dev fuzz") + steps.append( + f"a last review round with no findings (at most {MAX_ROUNDS} rounds)" + ) + steps.append("every review thread on the pull request answered") + print("required:") + for step in steps: + print(f" - {step}") + + +def plan(args) -> int: + """Print the tier of the current branch and what it owes.""" + try: + print_plan(make_plan(resolve_base(args.base, pull_request()))) + except InfraError as error: + common.die(str(error)) + return OK + + +def record(args) -> int: + """Record one finished review round for the current branch.""" + reviews = read_reviews() + entry = { + "round": len(reviews) + 1, + "model": args.model, + "findings": args.findings, + "head": head(), + "note": args.note, + } + with open(reviews_path(), "a") as file: + file.write(json.dumps(entry) + "\n") + print( + f"recorded round {entry['round']}: {args.model}, {args.findings} confirmed findings" + ) + return OK + + +def status(args) -> int: + """Check every requirement of the tier and print what is missing.""" + try: + pr = pull_request() + current = make_plan(resolve_base(args.base, pr)) + print( + f"tier {current.tier} against {current.base}: " + "; ".join(current.reasons) + ) + missing: list[str] = [] + waiting: list[str] = [] + + def line(ok: bool, text: str, pending: bool = False) -> None: + mark = "ok " if ok else ("wait" if pending else "TODO") + print(f" [{mark}] {text}") + if not ok: + (waiting if pending else missing).append(text) + + for note in current.notes: + print(f" [note] {note}") + checked, detail = read_evidence("check") + ci, names = ci_state(pr) if pr else ("none", []) + if ci == "pass" and not checked: + line(True, "check: CI passed on the pull request") + else: + line(checked, f"check: {detail} (uv run tsim-dev check)") + if pr and ci != "pass": + line(False, f"CI {ci}: {', '.join(names[:4])}", pending=ci == "pending") + for kind, needed, command in ( + ("bench", current.bench, f"uv run tsim-dev bench --ref {current.base}"), + ( + "width", + current.width, + f"uv run tsim-dev bench --width --ref {current.base}", + ), + ): + if not needed: + continue + ran, detail = read_evidence(kind) + line(ran, f"{kind}: {detail} ({command})") + found = regressions(evidence_record(kind)) if ran else "" + if found: + print( + f" [note] {kind} found regressions ({found}): confirm, profile, " + "and explain them in the pull request" + ) + if current.tier >= 2: + fuzzed, detail = read_evidence("fuzz") + line(fuzzed, f"fuzz: {detail} (uv run tsim-dev fuzz)") + reviews = read_reviews() + summary = ", ".join(f"{r['model']}: {r['findings']}" for r in reviews) or "none" + gaps = review_gaps(current.tier, reviews) + line( + not gaps, f"reviews ({summary})" + "".join(f"\n {g}" for g in gaps) + ) + if pr: + threads, pending = open_threads(pr) + line( + not threads, f"{len(threads)} unanswered review threads on {pr['url']}" + ) + for thread in threads[:12]: + print(f" - {thread}") + if pending: + line(False, "waiting for " + ", ".join(pending), pending=True) + else: + print(" [note] no open pull request: review threads and CI not checked") + except InfraError as error: + common.die(str(error)) + if missing: + print(f"NOT READY: {len(missing)} items to do") + return FAILED + if waiting: + print("WAIT: nothing to do yet, but checks or reviewers are still running") + return RUNNING + print("READY: every requirement of this tier is met") + return OK + + +def fuzz(args) -> int: + """Run the differential fuzzer for a fixed time and record the result.""" + out = common.out_dir("fuzz") + worker = Path(__file__).resolve().parent / "_fuzz.py" + cmd = [sys.executable, str(worker), "--seconds", str(args.seconds)] + cmd += ["--start", str(args.start), "--out", str(out)] + env = common.child_env(1) + env["PYTHONPATH"] = str(common.root()) + done = subprocess.run( + common.capped(cmd, mem=args.mem, timeout=int(args.seconds) + 300), + cwd=common.root(), + env=env, + capture_output=True, + text=True, + check=False, + ) + result = next( + ( + json.loads(x[7:]) + for x in done.stdout.splitlines() + if x.startswith("RESULT ") + ), + None, + ) + if result is None: + reason = common.explain_exit(done.returncode, args.mem, None) + print(f"ERROR: fuzz did not finish ({reason}): {done.stderr.strip()[-300:]}") + return INFRA + failures = result["failures"] + for failure in failures[:10]: + print( + f"FAILURE seed={failure['seed']}: {failure['problem']}\n {failure['circuit']}" + ) + write_evidence("fuzz", not failures, circuits=result["circuits"]) + verdict = "FAIL" if failures else "PASS" + print( + f"{verdict} fuzz: {result['circuits']} circuits from seed {result['start']} in " + f"{args.seconds:g} s, {len(failures)} failures" + ) + return FAILED if failures else OK diff --git a/dev/tsim_dev/width.py b/dev/tsim_dev/width.py index 8cf63df9..57331390 100644 --- a/dev/tsim_dev/width.py +++ b/dev/tsim_dev/width.py @@ -18,7 +18,7 @@ from collections.abc import Iterator from dataclasses import dataclass -from . import common, corpus +from . import common, corpus, review from .common import FAILED, INFRA, OK NAME = 44 @@ -208,6 +208,17 @@ def run(args, cases, sides) -> int: f"{counts.get(v, 0)} {v}" for v in ("wider", "narrower", "same", "unknown") ) print(f"{summary}. Details: {out}") + # NOTE: as for shot benchmarks, only a comparison of two checkouts over a whole + # selection counts as evidence for a review, and a wider width must be explained. + whole = not (args.circuits or args.only or args.max_width is not None) + if args.ref and whole: + review.write_evidence( + "width", + not errors, + ref=sides[1].label, + suite=args.suite, + wider=counts.get("wider", 0), + ) if errors: return INFRA return FAILED if counts.get("wider") else OK diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index af788595..5af01973 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -7,7 +7,7 @@ import pytest import yaml -from tsim_dev import bench, checks, cli, common, corpus, width +from tsim_dev import bench, checks, cli, common, corpus, review, width ROOT = Path(__file__).resolve().parents[2] HOOK = ROOT / ".claude" / "hooks" / "guard_bash.py" @@ -389,3 +389,96 @@ def fake_measure(side, case, args, mode="bench"): widths.update(new=[6, 6, 7], ref=[6, 6, 6]) assert "changed between runs" in bench.run_case(case, sides, args)["error"] + + +@pytest.mark.parametrize( + ("stat", "tier"), + [ + ([(10, 2, "docs/index.mdx"), (40, 0, "test/unit/test_x.py")], 0), + ([(200, 0, "dev/tsim_dev/bench.py")], 0), + ([(30, 5, "src/tsim/circuit.py")], 1), + ([(3, 1, "src/tsim/statevector/plan.py")], 2), + ([(2, 2, "src/tsim/sampler.py")], 2), + ([(250, 60, "src/tsim/circuit.py")], 2), + ([(900, 0, "src/tsim/external/vec_sim/vec_sampler.py")], 0), + ], +) +def test_the_review_tier_follows_paths_and_size(stat, tier): + assert review.classify(stat).tier == tier + + +def test_review_plans_flag_large_diffs_gate_files_and_removed_tests(): + plan = review.classify( + [ + (700, 0, "src/tsim/circuit.py"), + (1, 1, ".github/workflows/ci.yml"), + (2, 80, "test/unit/test_circuit.py"), + ] + ) + assert [note.split(":")[0] for note in plan.notes] == ["SPLIT", "HUMAN", "TESTS"] + assert review.classify([(5, 5, "src/tsim/circuit.py")]).notes == [] + + +def _rounds(*rounds): + return [{"model": model, "findings": findings} for model, findings in rounds] + + +def test_review_requirements_grow_with_the_tier(): + assert review.review_gaps(0, []) == [] + assert review.review_gaps(1, []) != [] + assert review.review_gaps(1, _rounds(("opus-5.5", 3))) == [] + # Two Claude models are one family; tier 2 needs another one. + assert review.review_gaps(2, _rounds(("opus-5.5", 0), ("claude-fable", 0))) != [] + assert review.review_gaps(2, _rounds(("opus-5.5", 2), ("gpt-6-astra", 0))) == [] + + +@pytest.mark.parametrize( + ("path", "bench", "width"), + [ + ("src/tsim/statevector/schedule.py", True, True), + ("src/tsim/statevector/shots.py", True, False), + ("src/tsim/sampler.py", True, False), + ("uv.lock", True, False), + ("src/tsim/utils/diagram.py", False, False), + ("src/tsim/noise/dem.py", False, False), + ("docs/index.mdx", False, False), + ], +) +def test_benchmarks_are_required_by_path_not_by_tier(path, bench, width): + plan = review.classify([(10, 2, path)]) + assert (plan.bench, plan.width) == (bench, width) + + +def test_a_benchmark_with_a_regression_counts_as_run_and_is_noted(): + record = {"passed": True, "slower": 2, "wider": 0, "kept_differs": 1} + assert review.regressions(record) == "2 slower, 1 with a different kept fraction" + assert review.regressions({"passed": True, "slower": 0}) == "" + + +def test_tier_two_reviews_stop_after_three_rounds_with_findings(): + two = review.review_gaps(2, _rounds(("opus-5.5", 2), ("gpt-6-astra", 1))) + assert len(two) == 1 and "new diff only" in two[0] + three = review.review_gaps( + 2, _rounds(("opus-5.5", 2), ("gpt-6-astra", 1), ("opus-5.5", 1)) + ) + assert len(three) == 1 and "Stop reviewing" in three[0] + + +@pytest.mark.parametrize( + ("command", "blocked"), + [ + ("gh pr merge 281 --squash", True), + ("uv run tsim-dev review status; gh pr merge 281", True), + ("uv run tsim-dev review status && gh pr merge 281 --squash", False), + ("gh pr view 281", False), + ], +) +def test_the_bash_hook_lets_merges_through_the_review_gate_only(command, blocked): + done = subprocess.run( # noqa: S603 + [sys.executable, str(HOOK)], + input=json.dumps({"tool_name": "Bash", "tool_input": {"command": command}}), + capture_output=True, + text=True, + check=False, + ) + assert done.returncode == (2 if blocked else 0)