From 6b9c5dde179303bff7f7998f9a5f634fa648ab4f Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 06:19:35 -0700 Subject: [PATCH 01/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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/12] 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 2a38216e3874d1e86d5d8625d522fd6c4bc82283 Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 09:34:07 -0700 Subject: [PATCH 11/12] feat(dev): stream `bench --width` results and give the run a deadline Both sides now compile at once: the circuits are cut into batches of 8, small files first, and `--jobs` workers take the next batch when free. A row prints as soon as a circuit is done on both sides, with an [n/N] count, so the first rows appear within a second and one slow compile delays only its batch. `--timeout` is the deadline for the whole width run (default 1800 s; 600 s per measurement for shot benchmarks as before): no batch starts after it, and the circuits not compiled are reported as errors. Whole corpus against main: 100 of 190 rows in 9 s, 180 in 64 s, all in 512 s (the 15-to-1 d25 circuit alone compiles for 240 s); all widths the same. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/benchmarking-tsim/SKILL.md | 10 +- dev/tsim_dev/bench.py | 2 + dev/tsim_dev/cli.py | 12 +- dev/tsim_dev/width.py | 163 +++++++++++++++------- test/unit/test_dev_cli.py | 59 ++++++-- 5 files changed, 184 insertions(+), 62 deletions(-) diff --git a/.claude/skills/benchmarking-tsim/SKILL.md b/.claude/skills/benchmarking-tsim/SKILL.md index 5351e41d..86a8601e 100644 --- a/.claude/skills/benchmarking-tsim/SKILL.md +++ b/.claude/skills/benchmarking-tsim/SKILL.md @@ -53,9 +53,13 @@ uv run tsim-dev bench --width --ref origin/main # whole corpus, compile onl 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 +Both sides compile every circuit once, at the same time, in `--jobs` workers (default +4). A row is printed as soon as a circuit is done on both sides, small circuits first, +with an `[n/N]` count, so the first rows appear within seconds; the row shows the width +next to the one recorded in tsim-bench. The verdict is `wider`, `narrower`, `same` or +`unknown`; any `wider` exits 1. `--case-timeout` (300 s) stops one slow compile; +`--timeout` (1800 s here) is the deadline for the whole run, after which the circuits +not yet compiled are reported as errors. 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. diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py index 640bf4ab..ccee84b2 100644 --- a/dev/tsim_dev/bench.py +++ b/dev/tsim_dev/bench.py @@ -427,6 +427,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.timeout is None: + args.timeout = 1800 if args.width else 600 if args.width: return width.run(args, cases, sides) has_ref = len(sides) == 2 diff --git a/dev/tsim_dev/cli.py b/dev/tsim_dev/cli.py index 408cec38..b72ec339 100644 --- a/dev/tsim_dev/cli.py +++ b/dev/tsim_dev/cli.py @@ -137,7 +137,17 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument("--batch-size", type=int, default=0) p.add_argument("--bit-packed", action="store_true") _circuit_options(p) - _caps(p, "8G", 600) + parser_caps = p.add_argument_group("caps") + parser_caps.add_argument( + "--mem", default="8G", help="memory cap per process (default 8G)" + ) + parser_caps.add_argument( + "--timeout", + type=int, + default=None, + help="seconds per measurement (default 600); with --width, for the whole run " + "(default 1800)", + ) p.set_defaults(func=bench.run) p = sub.add_parser( diff --git a/dev/tsim_dev/width.py b/dev/tsim_dev/width.py index 8cf63df9..38a5732c 100644 --- a/dev/tsim_dev/width.py +++ b/dev/tsim_dev/width.py @@ -1,9 +1,11 @@ """`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. +Both sides compile the selected circuits at once, in batches taken by `--jobs` +worker processes (tsim-bench's worker in `width` mode), and each circuit's row is +printed as soon as it has a result on every side. If a worker dies (memory cap) or one circuit compiles for +longer than `--case-timeout`, that circuit is recorded as an error and a new worker +continues with the next one. When the run passes `--timeout`, the circuits not yet +compiled are recorded as errors. """ from __future__ import annotations @@ -11,11 +13,12 @@ import concurrent.futures import json import os +import queue import select import statistics import subprocess import time -from collections.abc import Iterator +from collections.abc import Callable, Iterator from dataclasses import dataclass from . import common, corpus @@ -38,46 +41,52 @@ def _items(cases) -> list[tuple[str, str, str]]: @dataclass class Exit: - """How a worker ended: its exit code and stderr, or a stop for taking too long.""" + """How a worker ended: its exit code and stderr, or why it was stopped.""" code: int = 0 stderr: str = "" - timed_out: bool = False + timed_out: bool = False # one circuit compiled for longer than --case-timeout + past_deadline: bool = False # the whole run passed --timeout -def _stream(cmd, env, case_timeout: int, outcome: Exit) -> Iterator[str]: +def _stream( + cmd, env, case_timeout: int, deadline: float, 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. + The worker is stopped if no line arrives for `case_timeout` seconds (one circuit + compiles for longer than that), or when the run's `deadline` (monotonic) passes. """ 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 + quiet_until = time.monotonic() + case_timeout while True: ready, _, _ = select.select([process.stdout], [], [], 1.0) + now = time.monotonic() if ready: line = process.stdout.readline() if not line: break - deadline = time.monotonic() + case_timeout + quiet_until = now + case_timeout yield line.rstrip("\n") - elif time.monotonic() > deadline: + elif now > deadline or now > quiet_until: process.kill() process.wait() - outcome.timed_out = True + outcome.past_deadline = now > deadline + outcome.timed_out = not outcome.past_deadline return outcome.code = process.wait() outcome.stderr = process.stderr.read() -def measure(side, cases, args) -> dict[str, dict]: - """Compile every case on one side and return its result by case key.""" +def measure( + side, cases, args, deadline: float, report: Callable[[str, dict], None] +) -> None: + """Compile every case on one side and call `report(key, result)` for each one.""" 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))] @@ -87,18 +96,29 @@ 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, outcome = None, Exit() + started, outcome, done = None, Exit(), set() capped = common.capped(cmd, mem=args.mem) - for line in _stream(capped, env, args.case_timeout, outcome): + for line in _stream(capped, env, args.case_timeout, deadline, 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] + key = result.pop("name") + done.add(key) + report(key, result) + missing = [item for item in pending if item[0] not in done] if not missing: - break + return # NOTE: the worker died or was stopped while compiling `started`. + if outcome.past_deadline: + for key, _, _ in missing: + report( + key, + { + "error": f"not compiled: the run passed --timeout {args.timeout} s" + }, + ) + return if outcome.timed_out: reason = f"compile took over {args.case_timeout} s" else: @@ -106,23 +126,62 @@ def measure(side, cases, args) -> dict[str, dict]: 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}) - break - results[started] = error + for key, _, _ in missing: + report(key, dict(error)) + return + report(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] = {} +BATCH = 8 # circuits per worker process; a free worker takes the next batch + + +def measure_all(sides, cases, args, deadline: float) -> Iterator[tuple[int, str, dict]]: + """Compile the cases on every side at once and yield `(side, key, result)` as they come. + + The cases are cut into batches of `BATCH`, small files first, interleaved over the + sides, and `args.jobs` workers take the next batch whenever they are free. So the + first rows appear within seconds, one slow compile delays only its own batch, and + a batch that would start after `deadline` is reported as not compiled at once. + """ + ordered = sorted( + cases, key=lambda case: case.path.stat().st_size if case.path.exists() else 0 + ) + batches: queue.Queue = queue.Queue() + for start in range(0, len(ordered), BATCH): + for index in range(len(sides)): + batches.put((index, ordered[start : start + BATCH])) + results: queue.Queue = queue.Queue() + + def worker() -> None: + while True: + try: + index, batch = batches.get_nowait() + except queue.Empty: + return + + def report(key, result, index=index): + results.put((index, key, result)) + + if time.monotonic() > deadline: + error = f"not compiled: the run passed --timeout {args.timeout} s" + for key, _, _ in _items(batch): + report(key, {"error": error}) + continue + measure(sides[index], batch, args, deadline, report) + + jobs = max(1, min(args.jobs, batches.qsize())) with concurrent.futures.ThreadPoolExecutor(jobs) as pool: - for part in pool.map(lambda chunk: measure(side, chunk, args), chunks): - results.update(part) - return results + futures = [pool.submit(worker) for _ in range(jobs)] + for _ in range(len(cases) * len(sides)): + while True: + try: + yield results.get(timeout=1.0) + break + except queue.Empty: + for future in futures: + if future.done(): + future.result() # re-raises a worker thread's exception def _shown(result: dict | None) -> str: @@ -134,6 +193,10 @@ def _shown(result: dict | None) -> str: return f">={width}" if result.get("refused") else str(width) +def _seconds(result: dict) -> str: + return f"{result['compile_s']:.2f}" if "compile_s" in result else "-" + + 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: @@ -160,27 +223,30 @@ def run(args, cases, sides) -> int: 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}" + by_key = {key: case for case, (key, _, _) in zip(cases, _items(cases), strict=True)} + total = len(by_key) + header = f"{'':>9} {'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] + print(header, flush=True) + records, counts, partial = [], {}, {} + # NOTE: rows are printed as circuits finish on every side, not in suite order. + for index, key, result in measure_all(sides, cases, args, start + args.timeout): + partial.setdefault(key, {})[index] = result + if len(partial[key]) < len(sides): + continue + case, new = by_key[key], partial[key][0] record = {"circuit": case.id or str(case.path), "new": new} - row = f"{case.name[:NAME]:<{NAME}} {_shown(new):>5}" + progress = f"[{len(records) + 1}/{total}]" + row = f"{progress:>9} {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}" + row += f" {_seconds(new):>8}" if has_ref: - ref = per_side[1][key] + ref = partial[key][1] 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" {_shown(ref):>6} {_seconds(ref):>8}" row += f" {record['verdict']}" if "error" in new: row += f" ERROR: {new['error']}" @@ -189,6 +255,7 @@ def run(args, cases, sides) -> int: records.append(record) print(row, flush=True) + elapsed = time.monotonic() - start out = common.out_dir("bench") / f"width-{common.stamp()}.json" meta = { "new": sides[0].describe(), diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index af788595..aaad584e 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -3,6 +3,7 @@ import json import subprocess import sys +import time from pathlib import Path import pytest @@ -338,32 +339,70 @@ 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, outcome): +def _fake_width_stream(calls, stop): + def fake_stream(cmd, env, case_timeout, deadline, outcome): cases = [cmd[i + 1] for i, x in enumerate(cmd) if x == "--case"] calls.append(cases) for case in cases: yield f"START {case}" - if case == "p/narrow" and len(calls) == 1: - outcome.timed_out = True + if (case, len(calls)) in stop: + setattr(outcome, stop[case, len(calls)], True) return yield "RESULT " + json.dumps({"name": case, "width": 3}) - monkeypatch.setattr(width, "_stream", fake_stream) + return fake_stream + + +def _measure(cases, args): + results = {} + width.measure( + bench.parse_side("."), cases, args, time.monotonic() + 100, results.__setitem__ + ) + return results + + +def test_width_mode_resumes_after_a_stuck_circuit(fake_bench, monkeypatch): + calls = [] + stop = {("p/narrow", 1): "timed_out"} + monkeypatch.setattr(width, "_stream", _fake_width_stream(calls, stop)) cases = bench.select(_bench_args("--width", "--suite", "corpus")) - results = width.measure(bench.parse_side("."), cases, _bench_args("--width")) + results = _measure(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_width_mode_stops_at_the_overall_deadline(fake_bench, monkeypatch): + calls = [] + stop = {("p/narrow", 1): "past_deadline"} + monkeypatch.setattr(width, "_stream", _fake_width_stream(calls, stop)) + cases = bench.select(_bench_args("--width", "--suite", "corpus")) + args = _bench_args("--width", "--timeout", "60") + results = _measure(cases, args) + assert all("--timeout 60" in r["error"] for r in results.values()) + assert calls == [["p/narrow", "p/wide"]] + + +def test_width_results_stream_from_both_sides(fake_bench, monkeypatch): + monkeypatch.setattr(width, "_stream", _fake_width_stream([], {})) + cases = bench.select(_bench_args("--width", "--suite", "corpus")) + sides = [bench.parse_side(".:threads=1"), bench.parse_side(".:threads=2")] + args = _bench_args("--width", "--jobs", "4") + got = list(width.measure_all(sides, cases, args, time.monotonic() + 100)) + assert sorted((i, k) for i, k, _ in got) == [ + (0, "p/narrow"), + (0, "p/wide"), + (1, "p/narrow"), + (1, "p/wide"), + ] + + def test_the_width_stream_stops_a_stuck_compile(): script = "import time; print('START a', flush=True); time.sleep(30)" outcome = width.Exit() - lines = list(width._stream([sys.executable, "-c", script], None, 1, outcome)) - assert lines == ["START a"] and outcome.timed_out + command = [sys.executable, "-c", script] + lines = list(width._stream(command, None, 1, time.monotonic() + 100, outcome)) + assert lines == ["START a"] and outcome.timed_out and not outcome.past_deadline def test_shot_benchmarks_compare_and_check_the_active_width(fake_bench, monkeypatch): From 44541cad9ec2afbd5941abd031a57794372000f9 Mon Sep 17 00:00:00 2001 From: Rafael Haenel Date: Tue, 10 Nov 2026 09:42:07 -0700 Subject: [PATCH 12/12] fix(dev): address Codex review of the harness - The Bash hook also blocks `pkill --full`, the long form of `pkill -f`. - Two sides that describe themselves the same way (two dirty checkouts on the same commit) get their checkout path in the label instead of being rejected or mixing their rounds; only truly identical sides are refused. - `tsim-dev check` runs pre-commit under the memory and time caps too. - `tsim-dev test --changed` keeps deleted files: a deleted source file runs the full suite, a deleted test file runs nothing. - `tsim-dev run` exits 1 when the command fails, as documented, and prints the command's own exit code. Co-Authored-By: Claude Opus 5.5 --- .claude/hooks/guard_bash.py | 2 +- dev/tsim_dev/bench.py | 25 +++++++++++++++++++++++-- dev/tsim_dev/checks.py | 20 +++++++++++++++----- dev/tsim_dev/jobs.py | 8 +++++--- test/unit/test_dev_cli.py | 32 +++++++++++++++++++++++++++++++- 5 files changed, 75 insertions(+), 12 deletions(-) diff --git a/.claude/hooks/guard_bash.py b/.claude/hooks/guard_bash.py index 8160a1c2..fc747ed0 100755 --- a/.claude/hooks/guard_bash.py +++ b/.claude/hooks/guard_bash.py @@ -17,7 +17,7 @@ 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): + if re.search(r"\bpkill\b[^|;&\n]*\s(-\w*f|--full\b)", 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." diff --git a/dev/tsim_dev/bench.py b/dev/tsim_dev/bench.py index ccee84b2..0bdfb9ea 100644 --- a/dev/tsim_dev/bench.py +++ b/dev/tsim_dev/bench.py @@ -8,6 +8,7 @@ from __future__ import annotations import argparse +import dataclasses import json import math import os @@ -62,6 +63,27 @@ def describe(self) -> str: SIDE_KEYS = ("threads", "strategy") +def distinct(sides: list[Side]) -> list[Side]: + """Make the sides' labels unique; reject two sides that are the same. + + Two checkouts can describe themselves the same way (same branch and commit, both + dirty). Their rounds are kept apart by label, so such labels get the checkout path. + """ + if len(sides) < 2 or sides[0].describe() != sides[1].describe(): + return sides + if sides[0].python == sides[1].python: + raise InfraError("the two sides are the same; change a setting of one") + return [ + dataclasses.replace(side, label=f"{side.label} ({_checkout(side.python)})") + for side in sides + ] + + +def _checkout(python: Path) -> Path: + """Return the checkout of a `.venv/bin/python`, or the executable itself.""" + return python.parents[2] if python.parent.parent.name == ".venv" else python + + def parse_side(spec: str) -> Side: """Parse `--side REF[:key=value,...]`. @@ -423,8 +445,7 @@ def run(args) -> int: 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") + sides = distinct(sides) except InfraError as error: common.die(str(error)) if args.timeout is None: diff --git a/dev/tsim_dev/checks.py b/dev/tsim_dev/checks.py index ccfd9d3f..64f24489 100644 --- a/dev/tsim_dev/checks.py +++ b/dev/tsim_dev/checks.py @@ -11,8 +11,12 @@ from .common import FAILED, OK, InfraError -def changed_files() -> list[str]: - """Return files that differ from the merge base with main, plus untracked ones.""" +def changed_files(deleted: bool = False) -> list[str]: + """Return files that differ from the merge base with main, plus untracked ones. + + Deleted files are left out unless `deleted` is set: pre-commit cannot lint them, + but deleting a source file is a reason to run tests. + """ root = common.root() base = "HEAD" for candidate in ("origin/main", "main"): @@ -25,7 +29,7 @@ def changed_files() -> list[str]: names += common.git( "ls-files", "--others", "--exclude-standard", cwd=root ).splitlines() - return sorted({name for name in names if (root / name).exists()}) + return sorted({name for name in names if deleted or (root / name).exists()}) def tests_for(files: list[str]) -> list[str] | None: @@ -36,6 +40,12 @@ def tests_for(files: list[str]) -> list[str] | None: path = Path(name) if path.suffix != ".py" and path.parts[0] not in ("skills", "dev"): continue + if not (root / path).exists(): + # NOTE: a deleted source file can break any importer; a deleted test + # file has nothing left to run. + if path.parts[:2] == ("src", "tsim"): + return None + continue if path.parts[0] == "test": if path.name.startswith("test_"): tests.add(name) @@ -97,7 +107,7 @@ 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()) + mapped = tests_for(changed_files(deleted=True)) if mapped == []: print( "no changed files map to tests; use `tsim-dev test` for the full suite" @@ -128,7 +138,7 @@ def check(args) -> int: cmd += ["--files", *files] if files else ["--all-files"] with open(log, "w") as file: code = subprocess.run( - cmd, + common.capped(cmd, mem=args.mem, timeout=args.timeout), cwd=common.root(), env=common.child_env(), stdout=file, diff --git a/dev/tsim_dev/jobs.py b/dev/tsim_dev/jobs.py index f2419129..3b27ab1b 100644 --- a/dev/tsim_dev/jobs.py +++ b/dev/tsim_dev/jobs.py @@ -27,10 +27,12 @@ def run(args) -> int: ) 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): + if code: + # NOTE: the documented codes are 0 pass and 1 failed; the command's own + # exit code is printed, not passed on. reason = common.explain_exit(code, args.mem, args.timeout) - print(f"ERROR: {reason}", flush=True) - return 137 if code == -9 else code + print(f"FAILED: {reason}", flush=True) + return FAILED if code else OK def _unit(name: str) -> str: diff --git a/test/unit/test_dev_cli.py b/test/unit/test_dev_cli.py index aaad584e..d7b28c35 100644 --- a/test/unit/test_dev_cli.py +++ b/test/unit/test_dev_cli.py @@ -8,7 +8,7 @@ import pytest import yaml -from tsim_dev import bench, checks, cli, common, corpus, width +from tsim_dev import bench, checks, cli, common, corpus, jobs, width ROOT = Path(__file__).resolve().parents[2] HOOK = ROOT / ".claude" / "hooks" / "guard_bash.py" @@ -193,6 +193,8 @@ def test_pytest_summary_keeps_the_first_failure_and_the_last_line(tmp_path): [ ({"command": "pkill -f fuzz.py"}, True), ({"command": "pkill -9 -f fuzz"}, True), + ({"command": "pkill --full fuzz.py"}, True), + ({"command": "pkill -9 --full 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), @@ -428,3 +430,31 @@ 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"] + + +def test_deleted_files_still_choose_tests(): + # A deleted source file can break its importers: run everything. + assert checks.tests_for(["src/tsim/no_longer_here.py"]) is None + # A deleted test file has nothing left to run. + assert checks.tests_for(["test/unit/test_no_longer_here.py"]) == [] + + +def test_sides_with_the_same_label_are_told_apart(tmp_path): + one = tmp_path / "a" / ".venv" / "bin" / "python" + two = tmp_path / "b" / ".venv" / "bin" / "python" + sides = bench.distinct( + [bench.Side("main@1234-dirty", one), bench.Side("main@1234-dirty", two)] + ) + assert sides[0].describe() != sides[1].describe() + assert str(tmp_path / "a") in sides[0].label + with pytest.raises(common.InfraError, match="the same"): + bench.distinct([bench.Side("x", one), bench.Side("x", one)]) + + +def test_run_maps_command_failures_to_exit_one(capsys): + args = cli.build_parser().parse_args(["run"]) + args.command = [sys.executable, "-c", "raise SystemExit(42)"] + assert jobs.run(args) == 1 + assert "exit code 42" in capsys.readouterr().out + args.command = [sys.executable, "-c", "pass"] + assert jobs.run(args) == 0