From 5b8c62d2f0d50b7b0057d064c93a68e7091f282d Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 28 Sep 2026 14:15:38 -0400 Subject: [PATCH 1/3] fix(xtest): test benchmark improvements on the faster tail IMPROVED was decided by `ci_high < 1/threshold and p_adjusted > 1 - alpha`, reading the *slower* tail's Benjamini-Hochberg adjusted p-value backwards. BH never lowers a p-value, so adjusting the upper tail and then requiring the result to be large makes that clause easier to satisfy the more cells a run has: a multiplicity correction that manufactures improvements instead of suppressing false ones. The two tails are not exact complements under the discrete signed-rank null either, so even unadjusted the reversal was not the test it claimed to be. `PairedComparison` now carries `p_value_faster` from `wilcoxon(alternative="less")` alongside the existing upper tail, and `apply_multiplicity_control` BH-adjusts both tails independently within each of the same two families (gated, rest) into `p_adjusted_faster`. IMPROVED reads `ci_high < 1/threshold and p_faster < alpha`, the exact mirror of the REGRESSION clause. Controls and censored keys still enter no family and fall back to their own raw tails. Both new fields are emitted in the JSON artifact; it stays "schema": 1, since this is additive evidence only. Regression coverage: the two control-only tests are kept, and a censored control-only case is added -- a run holding nothing but an A/A cell whose RSS is pinned at the measurement floor must still report `nothing_measured`. The existing RSS-floor tests pair the control with a real cell, so they cannot see that failure: inverting `analyze`'s `control_keys.add` and its censoring `continue` leaves the censored key looking like a candidate comparison and the run reports a PASS headline having measured nothing. The partial-round test now also asserts both arms' vectors stayed the same length, which `n_rounds` alone cannot show. New `TestFasterTail` covers both tails on `compare`, the adjusted faster tail deciding IMPROVED, BH applying to it within its family, controls and censored keys entering neither family, and a hand-built pair where BH lifts a 0.94 slower tail past 1 - alpha and the old rule would have called it IMPROVED on weak lower-tail evidence. Deadline handling, complete-round recording and the configured minimum-round refusal were already correct on main and are unchanged. xtest/perf/README.md documents the two tails, the mirrored IMPROVED rule, the new JSON fields, and the ordering `analyze` depends on. The invariant list stays at 8. --- spec/DSPX-4372-t1.md | 356 +++++++++++++++++++++++++++++++++++++ xtest/perf/README.md | 46 ++++- xtest/perf/report.py | 5 + xtest/perf/stats.py | 71 +++++++- xtest/test_bench_runner.py | 33 ++++ xtest/test_bench_stats.py | 140 +++++++++++++++ 6 files changed, 639 insertions(+), 12 deletions(-) create mode 100644 spec/DSPX-4372-t1.md diff --git a/spec/DSPX-4372-t1.md b/spec/DSPX-4372-t1.md new file mode 100644 index 000000000..0a5093530 --- /dev/null +++ b/spec/DSPX-4372-t1.md @@ -0,0 +1,356 @@ +--- +ticket: DSPX-4372 +title: Six-stage delivery of multi-arm SDK benchmarks +status: draft +branches: [opentdf/tests:DSPX-4372-02-karm-core] +prs: [621] +created: 2026-09-28 +updated: 2026-09-28 +--- + +# DSPX-4372-t1 — Six-stage delivery of multi-arm SDK benchmarks + +## Objective + +Split [PR #621](https://github.com/opentdf/tests/pull/621) into independently +reviewable changes that extend the existing two-arm benchmark to two through +four named SDK builds. Developers must be able to prepare and run the same +experiment locally and in CI through shared Python implementations. + +This plan extends the [original benchmark specification](DSPX-4372.md). +Preserve its measurement isolation, paired rounds, comparability checks, +noise controls, and raw evidence. Deliver a simple result table first; report +presentation can follow after the measurement and conclusions are validated. + +## Starting point + +The review covered commit `7bd7506988c7ba912393c90959838c429268f598`. +[PR #620](https://github.com/opentdf/tests/pull/620), the branch-name flattening +prerequisite, merged on 2026-09-28. Rebase onto current main before extracting +the stages, retaining fixes already present there. + +At review time, all 192 benchmark unit tests passed. The PR benchmark job was +skipped, so those checks did not establish that the new workflow path worked. +Three additional simulated cases exposed gaps: + +| Finding | Observed behavior | Required correction | +| --- | --- | --- | +| Control-only run with censored RSS | `nothing_measured=False` and a PASS headline without any candidate measurement | Register every control comparison before applying censoring; preserve this invariant in the multi-arm conversion | +| Four arms with an uncertain candidate | A winner is declared after checking only the first two candidates in point-estimate order | Require supporting comparisons against every other candidate, or report pairwise conclusions without an overall winner | +| Biased A/A control with a narrow interval | `trustworthy=False` still permits a named winner | A failed applicable control must suppress winner claims | + +The first issue reverses existing classification behavior. Restore the +control-only tests removed by the PR and extend them with the censored case. +Carry these review cases into the stage acceptance tests below. + +## Ownership and contracts + +| Component | Owns | +| --- | --- | +| `xtest/perf` | Benchmark options, reference and arm ordering, budget defaults, measurement, statistical analysis, structured outcomes, and reports | +| `otdf-sdk-mgr` | Artifact resolution, installation, build settings, and a manifest recording exactly what was installed | +| `otdf-local` | Service configuration, startup, provisioning, readiness, environment export, logs, and shutdown | +| GitHub workflow | Dispatch inputs, runner matrix, toolchain setup, caches, calls to the local commands, artifact upload, and cleanup | + +Extract benchmark option validation from pytest fixtures into a pure module +under `xtest/perf`. Both the local preparation entry point and pytest must use +it. `otdf-sdk-mgr` should accept generic ordered artifact requests; it should +not own benchmark thresholds or statistical policy. + +Use a versioned resolved installation manifest to connect the components. +For each SDK request, preserve its requested alias, repository, immutable +commit, installed tag/path, installation method, and relevant build settings. +Record the platform and provisioning CLI pins separately from measured arms. +Preserve request order and the mapping from multiple aliases to one artifact. +Installed paths may differ between machines; artifact identities must not. + +The benchmark preparation result should combine that installation information +with validated benchmark settings and an explicit preparation status. The +renderer displays statuses; it must not decide whether pytest succeeds. +Do not use `report.same_commit()` as an exit-policy dependency. + +The existing scenario schema and `install scenario` manifest are useful +starting points. They are not already a complete implementation: the SDK +scenario install loop currently calls `install_release`, and arbitrary source +refs need explicit support. Extend or reuse that machinery without making a +general scenario-system rewrite a prerequisite for this work. Final CLI +spellings are to be selected in stage 2; the interfaces below describe +required behavior, not commands that already exist. + +## Delivery sequence + +| Stage | Deliverable | Dependencies | +| --- | --- | --- | +| 1 | Independent two-arm statistical fixes and regression coverage | Current main | +| 2 | Shared local preparation and installation contract | Stage 1 baseline | +| 3 | Multi-arm measurement and reference gates with simple output | Stages 1–2 | +| 4 | Candidate-to-candidate conclusions | Stage 3 | +| 5 | Thin CI integration using the local path | Stages 2–4 | +| 6 | Report presentation and publishing improvements | Stages 3–4; stage 5 for CI artifact links | + +Prepare one focused PR per stage. Stage 6 can be developed independently once +the result schema is stable and must not block the core release. Keep the +existing nightly two-arm path operational between merges. + +## Stage 1 — Independent two-arm statistical fixes + +### Scope + +Extract fixes that are useful without additional arms: + +- Carry explicit faster-tail p-values through analysis and artifacts, with + their appropriate multiplicity adjustment. Do not infer improvement by + reversing an adjusted slower-tail probability. +- Preserve current-main deadline handling, complete-round recording, and + configured minimum-round enforcement. Include changes here only where the + rebased branch still requires them. +- Establish control-only regression coverage, including censored RSS. + Classify controls before metric-specific censoring. +- Keep the current two-arm CLI and report shape, apart from additive evidence + needed for the statistical fix. + +Primary files: `xtest/perf/stats.py`, `xtest/perf/runner.py`, their tests, and +minimal serializer changes. Exclude multi-arm selection, new workflow inputs, +winner logic, and visual report changes. + +### Acceptance + +- [ ] A planted meaningful slowdown gates; a trivial change does not. +- [ ] A planted improvement uses the faster-tail test and never fails the job. +- [ ] Controls and censored metrics do not enter candidate correction families. +- [ ] A control-only run counts as nothing measured, including when RSS is at + the measurement floor; the session fails unless explicitly run without gating. +- [ ] Deadline expiry never commits a partial round or grants a verdict below + the configured minimum. +- [ ] Existing two-arm invocations and functional tests retain their behavior. + +## Stage 2 — Shared local preparation + +### Scope + +Provide a locally executable preparation path for the existing two-arm case. +Keep CI on its existing path until stage 5. + +1. Create the shared benchmark configuration parser and validator. Normalize + the existing pytest baseline/candidate aliases into one ordered request. + Validate SDK names, conflicting options, duplicate requests, finite positive + budgets, thresholds, and `max_rounds >= min_rounds` before setup begins. +2. Extend `otdf-sdk-mgr` to resolve ordered artifact requests, install the + resolved identities, and emit the installation manifest. Reuse existing + release and source-build implementations. Resolve mutable refs once per + plan and install those commits without resolving the names again. +3. Reject conflicting installed tags/paths before one build can overwrite + another. Preserve aliases when multiple requests identify one commit. +4. Represent two distinct aliases for the same commit as an explicit neutral + preparation outcome, produced before builds or service startup. Unknown + refs and failed resolutions remain errors. This must work identically + locally and in CI. +5. Use `otdf-local` to prepare only the benchmark's required backend services. + Start from `up --services docker,platform` and add any missing configuration + parity through its settings and CLI. Include required features, provisioning, + readiness, environment export, log paths, and cleanup. +6. Pin the provisioning `otdfctl` independently of the benchmark arms. A + release-only Go comparison must not fall back implicitly to another CLI. +7. Feed installed provenance and preparation status to pytest directly. Local + users must not need to construct `BENCH_VERSION_INFO` or + `BENCH_REQUESTED_REFS` themselves. + +Limit the service work to what the benchmark needs. Do not migrate the entire +functional matrix or expand the scenario runtime as part of this stage. + +### Acceptance + +- [ ] A documented local sequence resolves, installs, starts services, exports + the environment, runs a two-arm comparison, and shuts down the environment. +- [ ] Local preparation requires no copied shell from workflow files and no + GitHub-specific environment variables. +- [ ] Branches containing slashes, releases, SHAs, and supported PR refs retain + their requested identity and map to the expected installed build. +- [ ] Invalid options and ref collisions fail before installation or startup. +- [ ] Same-commit preparation is neutral; missing or failed builds are errors. +- [ ] A plan can be reused without a moving branch silently changing its SHA. +- [ ] Release-only Go arms use the explicitly selected provisioning CLI. +- [ ] Unit/CLI tests cover validation, manifest round trips, resolution failures, + and installation identity; a live local smoke run verifies service parity. + +## Stage 3 — Multi-arm measurement and reference gates + +### Scope + +Generalize the local measurement path to two through four named builds of one +SDK. The first arm is the reference, and every candidate is compared with it. + +- Introduce explicit arm identities, labels, reference identity, and per-arm + sample vectors in `runner.py` and the benchmark fixtures. +- Preserve randomized within-round execution. Record a round only after all + arms complete successfully. +- Run K copies of the reference for the A/A control and assess the worst + applicable control contrast. Preserve stage 1's control classification. +- Apply comparability checks across every arm and share reference-produced + ciphertexts for decrypt measurements. +- Scale the default budget by `K / 2` in the shared configuration module. + Honor explicit budgets unchanged. Treat the four-arm ceiling as a deliberate + supported benchmark limit, independent of the Action's installation slots. +- Support `--bench-refs`; preserve existing pytest baseline/candidate aliases + as adapters to the same validated configuration. +- Reject a partially duplicated multi-arm request before installation. If all + distinct requested aliases resolve to one commit, use the explicit neutral + preparation status from stage 2. +- Version the JSON schema for per-arm samples and reference contrasts. Provide + a simple table containing arm identities, metrics, intervals, and verdicts. + +Exclude candidate ranking, new report visualizations, and CI orchestration. +Retain convenience baseline/candidate labels where useful, but document that +changed sample and contrast structures require schema-aware consumers. + +### Acceptance + +- [ ] Two-, three-, and four-arm tests exercise each arm once per completed + round and preserve seeded execution order. +- [ ] A regression in any candidate's gated metric is detected against the + configured reference; CPU remains informational. +- [ ] All arms read the same ciphertext and use compatible operation settings. +- [ ] Censored control-only runs still count as nothing measured. +- [ ] Budget expiry, warm-up failures, partial rounds, and cleanup are tested + with more than two arms. +- [ ] JSON identifies every arm and preserves every completed sample with an + explicit schema version and reference. +- [ ] Existing two-arm commands continue to work. +- [ ] A live local multi-arm smoke run produces actual comparison cells and + usable artifacts; an all-skipped run does not satisfy acceptance. +- [ ] `xtest/perf/README.md` documents the new local invocation and contracts. + +## Stage 4 — Candidate-to-candidate conclusions + +### Scope + +Add every non-reference pairwise comparison to the analysis and simple report. +Keep candidate comparisons informational: they cannot fail the regression gate. + +- Define FASTER, SLOWER, TIED, and INCONCLUSIVE for the practical effect band. + Review the directional tests and multiplicity treatment explicitly. +- Keep gated reference comparisons, candidate comparisons, and other ungated + metrics in their intended separate correction families. +- Report individual pairwise conclusions first. Point-estimate order is not + evidence that every adjacent candidate is distinguishable. +- If an overall winner is emitted, require trustworthy supporting comparisons + against every other candidate. A tie, an unusable comparison, or an + unresolved competitor prevents a unique winner claim. +- Suppress winner claims when the applicable A/A control trips, even if its + interval is narrow. Preserve raw measurements and explain the limitation. +- Keep winner eligibility in the analysis/result model so JSON, terminal, and + Markdown cannot disagree about the conclusion. + +### Acceptance + +- [ ] Reversing comparison direction preserves the underlying conclusion. +- [ ] Adding informational comparisons does not alter the reference gate's + correction family. +- [ ] A clear two-candidate difference is reported without failing the job + solely because one candidate loses the comparison. +- [ ] A tied pair has no unique winner. +- [ ] With three candidates, a resolved first pair and an inconclusive third + competitor do not produce an overall winner. +- [ ] A tripped control suppresses winners even when `underpowered=False`. +- [ ] No control, censored data, and insufficient precision produce honest + unresolved conclusions. +- [ ] The README describes the pairwise verdicts and winner requirements. + +## Stage 5 — Thin CI integration + +### Scope + +Replace benchmark-specific workflow implementation with calls to the local +preparation, installation, service, and measurement paths delivered above. + +- Retain dispatch inputs, focus-SDK matrix selection, toolchain setup, caches, + upload actions, and cleanup in YAML. +- Run shared validation before creating expensive benchmark jobs. Emit the + matrix from validated input through a small adapter; do not reimplement ref + parsing, arm limits, defaults, or same-commit handling in shell. +- Resolve benchmark arms during preparation before platform startup. Consume + the same installed manifest and configuration that local pytest consumes. +- Replace the benchmark job's use of the fixed-slot setup action with the + manager's installation path. Other jobs can retain the action until a + separate migration. +- Remove the newly proposed deprecated workflow inputs `bench-baseline-ref` + and `bench-candidate-ref`; they did not exist before PR #621. Expose one + `bench-refs` form and retain the already-supported pytest aliases. +- Export service environment through `otdf-local`; ensure logs and completed + results remain available when measurement fails. Run cleanup on success and + failure. +- Keep measurements serial, with one SDK per runner and no ordinary PR perf + gate. Preserve nightly two-arm defaults. +- Choose and document a job timeout that includes installation, service setup, + measurement, and artifact upload. Validate any advertised budget ceiling + against that timeout; do not imply arbitrary budgets fit a fixed backstop. + +### Acceptance + +- [ ] The workflow contains no duplicate benchmark parsing or statistical policy. +- [ ] The same prepared experiment selects the same artifacts and settings + locally and in CI. Timings need not match across machines. +- [ ] Invalid requests fail before expensive setup; same-commit requests report + the same neutral outcome as local preparation. +- [ ] Dispatch smoke runs cover two, three, and four arms, with comparison + cells confirmed to execute and artifacts available. +- [ ] The default nightly path is verified for Go, Java, and JS. +- [ ] Failed installation/startup and a benchmark failure exercise cleanup and + retain the available diagnostic evidence. +- [ ] Existing functional jobs retain their behavior. + +## Stage 6 — Report presentation and publishing + +### Scope + +Extract the large presentation expansion from PR #621 into this final stage: +provenance links, effect charts, Braille traces, round diagnostics, improved +summaries, budget guidance, and links to uploaded artifacts. + +Keep serialization, conclusions, and rendering distinct. The renderer consumes +the structured outcome from earlier stages and never changes gate status, +winner eligibility, or statistical families. Provenance comes from the installed +manifest in both environments. + +Keep the simple table and JSON usable independently of visual enhancements. +Generate the Markdown artifact locally as well as in CI. If artifact links +require publication after upload, implement that rendering/substitution in a +locally testable function; YAML supplies only the returned artifact URL. + +### Acceptance + +- [ ] Local and CI reports identify requested refs and installed commits. +- [ ] PASS, regression, no measurement, same commit, untrustworthy control, + partial coverage, ties, and unresolved competitors have distinct wording. +- [ ] A partial run lists skipped cells and does not imply complete coverage. +- [ ] JSON, terminal, and Markdown agree on outcomes and winner eligibility. +- [ ] Large effects, long arm names, Markdown-sensitive characters, missing + provenance, and unavailable intervals render legibly. +- [ ] The report works without GitHub environment variables or an artifact URL; + CI publication never leaves an unresolved URL placeholder in the summary. +- [ ] Existing analysis fixtures produce identical gate decisions before and + after presentation changes. + +## Validation and merge discipline + +For every stage, include its tests and documentation in the same PR. Run lint, +format, and type checks from each touched Python package using `uv run`, plus +the relevant package tests. Follow the package `AGENTS.md` requirements. + +Use offline simulated measurements for statistical and deadline invariants. +Use CLI tests with controlled resolution/installation dependencies for the +preparation contract. Use live smoke runs to establish that installed builds, +service setup, and measurement work together. Record completed rounds, +comparison counts, and skip reasons when assessing smoke-run success. + +Each PR description should state its independent user-visible result, +dependencies, compatibility effects, and validation evidence. Keep extraction +work based on current main so already-fixed two-arm behavior is not lost. +Do not carry report embellishments or unrelated workflow refactoring into the +core stages merely because they were present in the original branch. + +## Out of scope + +Historical performance storage, cross-SDK speed rankings, new payload-size +experiments, a general workflow migration, a general scenario-runtime rewrite, +and performance gating on ordinary pull requests remain separate work. diff --git a/xtest/perf/README.md b/xtest/perf/README.md index 0c9b57014..b0b95eeaf 100644 --- a/xtest/perf/README.md +++ b/xtest/perf/README.md @@ -41,6 +41,14 @@ The JSON is the useful one. It holds each cell's full per-round vectors for both arms, so a surprising verdict can be re-analysed offline instead of by re-running a 30-minute job to look at the same numbers again. +Each `metrics` entry carries both directions of the test: `p_value` / +`p_adjusted` are the one-sided "candidate is slower" pair that the regression +gate reads, and `p_value_faster` / `p_adjusted_faster` are the matching +"candidate is faster" pair behind IMPROVED. The `_adjusted` forms are the +Benjamini–Hochberg values within that key's correction family and are `null` +for controls and censored keys, which enter no family. These are additive +fields; the artifact is still `"schema": 1`. + ### The table ```markdown @@ -54,7 +62,9 @@ a 30-minute job to look at the same numbers again. longer. Below 1.0 means faster. - **95% CI** — the bootstrap interval on that ratio. Its *width* is how precisely this run could measure; a wide interval means a noisy runner, not a big change. -- **p (BH)** — one-sided p-value, Benjamini–Hochberg adjusted across the run. +- **p (BH)** — the one-sided "candidate is slower" p-value, Benjamini–Hochberg + adjusted across the run. The opposite direction has its own adjusted p-value; + it is in the JSON rather than the table. - **n** — paired rounds actually measured (20–60; the loop stops early once the interval is narrow enough). @@ -70,7 +80,11 @@ enough to be ignored within a week. This fails the job. one. "We looked and found nothing" only counts when we could have found something. -**IMPROVED** — the same test in the other direction. Never fails anything. +**IMPROVED** — the same test mirrored: the CI *upper* bound below `1/threshold` +(0.87x by default) *and* the adjusted "candidate is faster" p < 0.05. That +second clause is its own lower-tail test with its own BH adjustment, not a +reversal of the slowdown one — see [the two tails](#the-two-tails). Never fails +anything. **inconclusive** — the run could not decide. Common reasons, all shown in the note beside the verdict: @@ -262,13 +276,33 @@ BH-adjusted p is below alpha. Clause 1 alone fires on real-but-trivial effects measured precisely; clause 2 alone fires on noise roughly alpha of the time per cell, and a run has enough cells that "roughly alpha" becomes "most nights". +#### The two tails + +Every comparison carries two one-sided p-values: `p_value` for "candidate is +slower", which REGRESSION reads, and `p_value_faster` for "candidate is +faster", which IMPROVED reads. Both come from `wilcoxon(...)` directly, and +both get their own BH adjustment inside the same family. + +The obvious shortcut — derive one direction from the other, `p > 1 - alpha` +instead of a lower-tail test — is wrong twice over. The tails are not exact +complements under the discrete signed-rank null, and, far worse, **BH never +lowers a p-value**. Adjusting the upper tail and then asking whether the result +is *large* makes that clause easier to satisfy the more cells a run has, which +is a multiplicity correction running backwards: adding cells would manufacture +improvements rather than suppress false ones. +`test_adjusted_slower_tail_is_not_read_as_evidence_of_improvement` pins this. + #### Separate BH families Gated keys are corrected as their own family. Ungated metrics get a family of their own so they still carry a reportable verdict. Adjusting the gated metrics against metrics nobody gates on would only make a real regression harder to confirm. Controls and censored keys are excluded from correction entirely — an -A/A cell is not a hypothesis about the candidate. +A/A cell is not a hypothesis about the candidate. Both tails are adjusted +within the same families, so a key's two adjusted p-values always describe the +same set of hypotheses. A key outside every family has `p_adjusted` and +`p_adjusted_faster` both unset; a control falls back to its own raw tails, +since a floor that cannot resolve itself is no floor at all. #### One A/A control per SDK, running first @@ -352,6 +386,12 @@ report is visibly quiet rather than indistinguishable from a clean one. If explicit request for a measurement, and answering it with a green tick and an empty table is the one outcome nobody inspects. +A control-only run counts as nothing measured too, and `analyze()` has to +register each control key *before* it censors that cell's floored RSS — the +`continue` must come after `control_keys.add(key)`. Reversed, the censored RSS +key stops looking like part of a control, `has_candidate_comparisons` goes +true, and a run holding only A/A cells reports a PASS headline. + The bench job installs `go` on every runner even when it is not the SDK under measurement, because `otdfctl` provisions the attributes and KAS registry that every cell needs and `conftest.py` loads it at import time. `OTDFCTL_HEADS` must diff --git a/xtest/perf/report.py b/xtest/perf/report.py index a100044ae..f5e4e8d65 100644 --- a/xtest/perf/report.py +++ b/xtest/perf/report.py @@ -144,6 +144,11 @@ def _comparison_dict(c: stats.PairedComparison) -> dict[str, object]: "ci_high": _jsonable(c.ci_high), "p_value": _jsonable(c.p_value), "p_adjusted": _jsonable(c.p_adjusted), + # Both directions are carried explicitly. The schema stays at 1: these + # are additive fields, and a consumer that only knows the slower tail + # reads exactly what it read before. + "p_value_faster": _jsonable(c.p_value_faster), + "p_adjusted_faster": _jsonable(c.p_adjusted_faster), "verdict": str(c.verdict), "note": c.note, } diff --git a/xtest/perf/stats.py b/xtest/perf/stats.py index f7aea9660..afff4a1ed 100644 --- a/xtest/perf/stats.py +++ b/xtest/perf/stats.py @@ -38,6 +38,14 @@ Together they answer the only question worth gating on: is the slowdown both real and large enough to care about? + +The other direction +------------------- +IMPROVED mirrors the rule with the inequalities flipped: the CI *upper* bound +below ``1/threshold`` and the lower-tail p-value below ``alpha``. That lower +tail is computed and adjusted in its own right rather than inferred by reading +a large upper-tail p-value as evidence of a speedup -- see +:func:`_one_sided_ps` and :func:`apply_multiplicity_control`. """ from __future__ import annotations @@ -89,9 +97,15 @@ class PairedComparison: ratio: float ci_low: float ci_high: float + #: One-sided p-value for "candidate is slower". Retains the original field + #: name for artifact/API compatibility; the opposite direction is explicit. p_value: float + #: One-sided p-value for "candidate is faster". + p_value_faster: float #: Set by :func:`apply_multiplicity_control` once every cell is known. p_adjusted: float | None = None + #: BH-adjusted form of :attr:`p_value_faster`. + p_adjusted_faster: float | None = None verdict: Verdict = Verdict.INCONCLUSIVE note: str = "" @@ -174,18 +188,27 @@ def _bootstrap_ci( return float(res.confidence_interval.low), float(res.confidence_interval.high) -def _one_sided_p(d: np.ndarray) -> float: - """One-sided Wilcoxon signed-rank p-value for "candidate is slower". +def _one_sided_ps(d: np.ndarray) -> tuple[float, float]: + """Wilcoxon signed-rank p-values for slower and faster, respectively. Signed-rank rather than a t-test because latency distributions are skewed and occasionally have a stray outlier round; we do not want a single stalled invocation to drive the verdict. + + Both tails are computed here rather than one being derived from the + other. They are not complements of each other under the discrete + signed-rank null, and the adjusted forms are even less so: BH pushes + large p-values toward 1, so reading an adjusted upper-tail value as + evidence for the lower tail makes the improvement test *easier* the more + cells a run has. """ if np.all(d == 0): # No difference whatsoever. Wilcoxon rejects an all-zero input. - return 1.0 + return 1.0, 1.0 # scipy's stubs type the result as an opaque tuple-like; index and cast. - return cast(float, _scipy_stats.wilcoxon(d, alternative="greater")[1]) + slower = cast(float, _scipy_stats.wilcoxon(d, alternative="greater")[1]) + faster = cast(float, _scipy_stats.wilcoxon(d, alternative="less")[1]) + return slower, faster def compare( @@ -216,12 +239,14 @@ def compare( ci_low=math.nan, ci_high=math.nan, p_value=math.nan, + p_value_faster=math.nan, note=f"only {n} usable rounds; need at least {MIN_USABLE_ROUNDS}", ) lo_log, hi_log = _bootstrap_ci( d, confidence=confidence, seed=seed, n_resamples=n_resamples ) + p_slower, p_faster = _one_sided_ps(d) return PairedComparison( n_rounds=n, baseline_median=b_med, @@ -229,7 +254,8 @@ def compare( ratio=math.exp(float(np.median(d))), ci_low=math.exp(lo_log), ci_high=math.exp(hi_log), - p_value=_one_sided_p(d), + p_value=p_slower, + p_value_faster=p_faster, ) @@ -450,6 +476,7 @@ def apply_multiplicity_control( gated_family = [k for k in adjustable if gated is None or k in gated] rest = [k for k in adjustable if k not in set(gated_family)] p_adj: dict[str, float] = {} + p_adj_faster: dict[str, float] = {} for family in (gated_family, rest): p_adj.update( zip( @@ -458,6 +485,17 @@ def apply_multiplicity_control( strict=True, ) ) + # The opposite direction needs its own lower-tail p-value and its own + # adjustment. Reading an adjusted upper-tail value as `p > 1-alpha` + # is invalid: BH controls small p-values and generally pushes large + # ones toward 1, making that backwards test easier rather than safer. + p_adj_faster.update( + zip( + family, + benjamini_hochberg([comparisons[k].p_value_faster for k in family]), + strict=True, + ) + ) result = GateResult( noise=noise, @@ -467,12 +505,14 @@ def apply_multiplicity_control( for key in keys: c = comparisons[key] pa = p_adj.get(key) + pa_faster = p_adj_faster.get(key) if key in censored: verdict, note = Verdict.INCONCLUSIVE, censored[key] else: verdict, note = _verdict_for( c, pa, + pa_faster, threshold=threshold, alpha=alpha, noise=noise_by_control.get(controls.get(key, ""), uncontrolled), @@ -486,7 +526,9 @@ def apply_multiplicity_control( ci_low=c.ci_low, ci_high=c.ci_high, p_value=c.p_value, + p_value_faster=c.p_value_faster, p_adjusted=pa, + p_adjusted_faster=pa_faster, verdict=verdict, note=note or c.note, ) @@ -505,6 +547,7 @@ def apply_multiplicity_control( def _verdict_for( c: PairedComparison, p_adjusted: float | None, + p_adjusted_faster: float | None, *, threshold: float, alpha: float, @@ -514,13 +557,23 @@ def _verdict_for( if c.n_rounds < MIN_USABLE_ROUNDS or not math.isfinite(c.ci_low): return Verdict.INCONCLUSIVE, c.note or "no usable interval" - p = c.p_value if is_control else p_adjusted - if p is None or not math.isfinite(p): + # A control is not part of any correction family -- it is not a hypothesis + # about the candidate -- so it reads its own unadjusted tails. + p_slower = c.p_value if is_control else p_adjusted + p_faster = c.p_value_faster if is_control else p_adjusted_faster + if p_slower is None or p_faster is None: + return Verdict.INCONCLUSIVE, "no p-value" + if not (math.isfinite(p_slower) and math.isfinite(p_faster)): return Verdict.INCONCLUSIVE, "no p-value" - if c.ci_low > threshold and p < alpha: + if c.ci_low > threshold and p_slower < alpha: return Verdict.REGRESSION, "" - if c.ci_high < 1 / threshold and p > 1 - alpha: + # The mirror image of the clause above, not a reversal of it. Testing + # `p_slower > 1 - alpha` on an *adjusted* upper-tail p-value would be + # invalid: BH controls small p-values and generally pushes large ones + # toward 1, so that backwards test gets easier as a run grows rather than + # safer. The lower tail is measured and adjusted in its own right. + if c.ci_high < 1 / threshold and p_faster < alpha: return Verdict.IMPROVED, "" # Not a regression. But "we looked and found nothing" only counts as PASS diff --git a/xtest/test_bench_runner.py b/xtest/test_bench_runner.py index ee0e7dd9b..13d4c3b7d 100644 --- a/xtest/test_bench_runner.py +++ b/xtest/test_bench_runner.py @@ -318,6 +318,14 @@ def scripted_run( assert result.n_rounds == 5 assert len(calls) == 11, "round six started but only one arm completed" assert timeouts[-1] == pytest.approx(2.5), "timeout is remaining budget" + # `n_rounds` reads one arm only, so it cannot see the failure mode + # this test exists for: the orphaned sixth-round reading landing in + # one arm's vector and silently un-pairing every round after it. + assert all( + len(vector) == 5 + for arm_samples in result.samples.values() + for vector in arm_samples.values() + ), "the discarded round left one arm ahead of the other" class TestBenchConfigValidation: @@ -434,6 +442,31 @@ def test_control_plus_candidate_counts_as_measured(self): assert not gate.nothing_measured + def test_censored_control_only_run_still_measured_nothing(self): + # `analyze` records every metric of a control cell as a control key + # *before* it censors the floored RSS one. Reverse those two steps -- + # let the `continue` skip the `control_keys.add` -- and the censored + # RSS key stops looking like part of a control to the run-level + # safeguard. A control-only run then claims to have measured a + # candidate and reports a PASS headline having measured nothing. + # + # The other RSS-floor tests cannot catch this: they pair the control + # with a real cell, so `has_candidate_comparisons` is true either way. + cfg = config(max_rounds=40) + floor = 4 * BASELINE_RSS + control, _ = run( + 1.0, cfg=cfg, cell_id="aa", control=True, sdk="go", rss_floor=floor + ) + gate = analyze([control], cfg) + + rss = gate.comparisons["aa/rss"] + assert rss.verdict is stats.Verdict.INCONCLUSIVE + assert "floor" in rss.note, "precondition: the control's RSS is censored" + assert gate.nothing_measured + assert "NOTHING MEASURED" in gate.summary + assert not gate.regressions + assert not gate.improvements + def test_a_regression_in_an_ungated_metric_does_not_fail(self): cfg = config(max_rounds=40, gated_metrics=("wall",)) control, _ = run(1.0, cfg=cfg, cell_id="aa", control=True) diff --git a/xtest/test_bench_stats.py b/xtest/test_bench_stats.py index 71a94d9ae..1ff29a7c1 100644 --- a/xtest/test_bench_stats.py +++ b/xtest/test_bench_stats.py @@ -234,6 +234,146 @@ def test_detects_regression_across_realistic_noise(self): ) +class TestFasterTail: + """IMPROVED is its own one-sided test, not a reversed slower-tail one. + + The rule these replaced read ``p_adjusted > 1 - alpha`` off the *upper* + tail. BH never lowers a p-value, so adjusting the upper tail made that + clause easier to satisfy the more cells a run had -- a multiplicity + correction working backwards. Improvements now carry their own lower-tail + p-value and their own adjustment. + """ + + def test_compare_reports_both_tails(self): + rng = np.random.default_rng(40) + b, c = synth(rng, 0.70, n=60) + r = stats.compare(b, c, seed=40, n_resamples=RESAMPLES) + assert r.p_value_faster < 0.05, "a 30% speedup is evidence of a speedup" + assert r.p_value > 0.95, "and no evidence at all of a slowdown" + + def test_identical_inputs_have_no_evidence_in_either_direction(self): + v = [1.0, 2.0, 3.0, 4.0, 5.0, 6.0] + r = stats.compare(v, v, seed=0, n_resamples=RESAMPLES) + assert r.p_value == 1.0 + assert r.p_value_faster == 1.0 + + def test_too_few_rounds_leaves_both_tails_unset(self): + rng = np.random.default_rng(43) + b, c = synth(rng, 0.5, n=3) + r = stats.compare(b, c, seed=43, n_resamples=RESAMPLES) + assert math.isnan(r.p_value) + assert math.isnan(r.p_value_faster) + + def test_improvement_is_decided_by_the_adjusted_faster_tail(self): + rng = np.random.default_rng(41) + b, c = synth(rng, 0.70, n=60) + g = gate_one( + stats.compare(b, c, seed=41, n_resamples=RESAMPLES), control=quiet_control() + ) + cell = g.comparisons["cell"] + assert cell.verdict is Verdict.IMPROVED + assert cell.p_adjusted_faster is not None and cell.p_adjusted_faster < 0.05 + assert g.improvements == ["cell"] + assert not g.should_fail, "an improvement must never fail the job" + assert g.regressions == [] + + def test_faster_tail_is_bh_adjusted_within_its_family(self): + cells: dict[str, stats.PairedComparison] = {} + for i in range(6): + rng = np.random.default_rng(4000 + i) + b, c = synth(rng, 0.90, n=30) + cells[f"cell{i}"] = stats.compare(b, c, seed=i, n_resamples=RESAMPLES) + cells["control"] = quiet_control() + g = stats.apply_multiplicity_control( + cells, controls=all_under_one_control(cells) + ) + + family = [k for k in cells if k != "control"] + raw = [cells[k].p_value_faster for k in family] + adjusted = [g.comparisons[k].p_adjusted_faster for k in family] + assert all(a is not None for a in adjusted) + assert adjusted == pytest.approx(stats.benjamini_hochberg(raw)) + assert any( + a > p for a, p in zip(adjusted, raw, strict=True) if a is not None + ), "precondition: the adjustment actually moved something" + + def test_adjusted_slower_tail_is_not_read_as_evidence_of_improvement(self): + # Hand-built so the two rules disagree on purpose. The raw evidence + # for a speedup here is weak (p_faster = 0.30); the slower tail sits + # at 0.94, just under 1 - alpha, and BH lifts it to 0.99 against the + # second cell. The rule this replaced would have called that IMPROVED + # *because of* the correction. + weak = stats.PairedComparison( + n_rounds=30, + baseline_median=1.0, + candidate_median=0.8, + ratio=0.80, + # Comfortably below 1/1.15, so the interval clause is satisfied + # and only the p-value clause decides the verdict. + ci_low=0.70, + ci_high=0.84, + p_value=0.94, + p_value_faster=0.30, + ) + other = stats.PairedComparison( + n_rounds=30, + baseline_median=1.0, + candidate_median=1.0, + ratio=1.00, + ci_low=0.95, + ci_high=1.05, + p_value=0.99, + p_value_faster=0.02, + ) + cells = {"weak": weak, "other": other, "control": quiet_control()} + g = stats.apply_multiplicity_control( + cells, controls=all_under_one_control(cells) + ) + + adjusted = g.comparisons["weak"].p_adjusted + assert adjusted is not None and adjusted > 0.95, ( + "precondition: BH lifted the slower tail past 1 - alpha" + ) + assert g.comparisons["weak"].p_adjusted_faster == pytest.approx(0.30) + assert g.comparisons["weak"].verdict is Verdict.PASS + assert g.improvements == [] + + def test_controls_and_censored_keys_enter_neither_family(self): + rng = np.random.default_rng(42) + b, c = synth(rng, 0.70, n=60) + improving = stats.compare(b, c, seed=42, n_resamples=RESAMPLES) + cells = {"wall": improving, "rss": improving, "control": quiet_control()} + g = stats.apply_multiplicity_control( + cells, + gated={"wall", "rss"}, + controls=all_under_one_control(cells), + control_keys={"control"}, + censored={"rss": "peak rss reaches the measurement floor"}, + ) + + for key in ("control", "rss"): + assert g.comparisons[key].p_adjusted is None + assert g.comparisons[key].p_adjusted_faster is None + # `wall` is now the whole gated family, so its adjustment is a no-op: + # neither the control nor the censored twin diluted it. + assert g.comparisons["wall"].p_adjusted_faster == pytest.approx( + improving.p_value_faster + ) + assert g.improvements == ["wall"] + assert g.comparisons["rss"].verdict is Verdict.INCONCLUSIVE + + def test_a_control_reads_its_own_unadjusted_tails(self): + # A control is in no correction family at all, so both of its tails + # have to come straight off the comparison or it can never resolve. + control = quiet_control() + g = stats.apply_multiplicity_control( + {"control": control}, controls={"control": "control"} + ) + assert g.comparisons["control"].p_adjusted is None + assert g.comparisons["control"].p_adjusted_faster is None + assert g.comparisons["control"].verdict is Verdict.PASS + + class TestNoiseFloor: def test_clean_control_is_trusted(self): n = stats.assess_noise_floor(quiet_control(), threshold=1.15) From 826a800bf74c189ab7b99e5ccd44465053d3522a Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 29 Sep 2026 08:27:32 -0400 Subject: [PATCH 2/3] docs(xtest): figures for the benchmark's three-way decision rule Why the old IMPROVED rule was wrong is hard to see from the diff -- it reads like an algebraic identity. Adds three figures to perf/README.md that make the failure legible, plus the generator behind them: - fig1, three regions: the verdict margins (1.15 / 0.87) against the point null at 1.0, where both Wilcoxon tests actually sit. The CI clause carries the margin; across 1200 simulated cells the raw p-clause never once bound. - fig2, the correction runs backwards: BH never lowers a p-value, so `p_adjusted > 1 - alpha` gets easier as a run grows. Mean false IMPROVED cells per run go 0.43 / 2.71 / 6.28 at 4 / 12 / 24 cells under the old rule, against a flat ~0.06 under the new one. - fig3, positivity: the log transform fixes scale but not skew. One-sided stalls drive the slower tail's size to 0.815 and the faster tail's to 0.000 while the median ratio moves only 1.00 -> 1.07 -- which is why requiring both clauses is load-bearing, not belt-and-braces. Figure data is frozen as module constants rather than simulated at render time, so a scipy release that shifted a tie correction cannot silently rewrite three committed SVGs into an unreviewable diff. `--verify` re-runs the Monte Carlo and reports a mismatch for a human to judge; `--check` byte-compares for drift. Documentation only: nothing in perf/docs/ is imported by the harness, no test collects it, and the emitted report artifacts are untouched. --- xtest/perf/README.md | 92 ++++ xtest/perf/docs/__init__.py | 8 + xtest/perf/docs/fig1-three-regions.svg | 50 ++ xtest/perf/docs/fig2-fdr-backwards.svg | 40 ++ xtest/perf/docs/fig3-positivity.svg | 87 ++++ xtest/perf/docs/make_figures.py | 634 +++++++++++++++++++++++++ xtest/perf/docs/svgkit.py | 328 +++++++++++++ xtest/perf/docs/verify.py | 278 +++++++++++ 8 files changed, 1517 insertions(+) create mode 100644 xtest/perf/docs/__init__.py create mode 100644 xtest/perf/docs/fig1-three-regions.svg create mode 100644 xtest/perf/docs/fig2-fdr-backwards.svg create mode 100644 xtest/perf/docs/fig3-positivity.svg create mode 100644 xtest/perf/docs/make_figures.py create mode 100644 xtest/perf/docs/svgkit.py create mode 100644 xtest/perf/docs/verify.py diff --git a/xtest/perf/README.md b/xtest/perf/README.md index b0b95eeaf..a7b52ed8f 100644 --- a/xtest/perf/README.md +++ b/xtest/perf/README.md @@ -70,6 +70,11 @@ fields; the artifact is still `"schema": 1`. ### The verdicts +![Three verdicts, two tests of the same point null: a log-scaled ratio axis +banded into IMPROVED below 0.87, no verdict, and REGRESSION above 1.15, with a +rule at 1.00 marking where both Wilcoxon tests actually +test.](docs/fig1-three-regions.svg) + **REGRESSION** — the CI lower bound exceeds the threshold (default **1.15x**, i.e. 15% slower) *and* the adjusted p < 0.05. Both clauses are required, and neither is redundant: the threshold alone would fire on a reproducible 0.5% @@ -210,6 +215,7 @@ things; the noise floor will tell you whether you succeeded. | `runner.py` | The paired round loop, the stopping rule, the budget, `analyze()` | | `stats.py` | Pure functions: log-ratios, bootstrap CI, Wilcoxon, BH, the decision rule | | `report.py` | Session recorder, JSON artifact, step-summary markdown | +| `docs/` | The figures in this README, and the Monte Carlo behind them. Documentation only; imported by nothing in the harness | | `../fixtures/bench.py` | The pytest glue: arm selection, payloads, ciphertexts, budget | | `../test_benchmarks.py` | One test per cell. **Records; never asserts** | | `../conftest.py` | `--bench*` options, cell parametrization, the session-finish gate | @@ -253,6 +259,39 @@ correlate their noise. slowdown and a 2x speedup are equal and opposite) and additive, which is what the median and the bootstrap want. Everything is exponentiated back for reporting. +![Two panels. On the left, the spread of the raw paired difference grows with +the effect while the log-ratio's stays flat. On the right, stalling the +candidate arm only drives the slower tail's rejection rate to 0.815 and the +faster tail's to zero while the median ratio barely moves; stalling both arms +leaves the slower tail at nominal.](docs/fig3-positivity.svg) + +The log fixes a *scale* problem. Timings are strictly positive, so the raw +paired difference inherits the size of whatever effect is present — its spread +grows from 0.142 to 0.412 as the true ratio goes 1x to 4x, while the log-ratio's +stays flat at 0.141. Thresholds and interval widths are therefore comparable +across payload sizes and SDKs only because of the log. + +It does not fix a *skew* problem. Signed-rank needs `d` symmetric under the +null, and a positive right-tailed variable produces one-sided contamination — +a stalled round makes an arm slower, never faster. Stalls landing on one arm +break the symmetry directionally: at a 15% stall rate the slower tail rejects +36.2% of the time and the faster tail collapses to 0.001, while the median +ratio moves only 2.6%. + +Read what that is and is not. Stalling one arm changes that arm's +distribution, so the true median ratio leaves 1 and the point null the +p-values test is genuinely false — 36.2% is *power against a 2.6% effect*, +not a size failure. Running the control confirms it: stall both arms at the +same rate and the true ratio stays 1 under contamination just as heavy, and +the slower tail holds between 0.039 and 0.057 across every rate simulated. +Signed-rank is not being invalidated here; it is answering the question it was +asked, and that question is the wrong one for a gate. + +Which is the second reason [both clauses](#both-clauses-of-the-decision-rule) +are required. A 2.6% shift is real and nowhere near the 15% margin, and only +the CI clause knows the difference — it is what keeps a stall-contaminated +cell from being read as a regression. + #### Stopping on precision, never on significance > This is the single easiest thing here to "optimize" into invalidity. @@ -276,6 +315,25 @@ BH-adjusted p is below alpha. Clause 1 alone fires on real-but-trivial effects measured precisely; clause 2 alone fires on noise roughly alpha of the time per cell, and a run has enough cells that "roughly alpha" becomes "most nights". +Note which clause carries which claim. The p-values test the *point* null +(`ratio = 1`); the margin is carried by the interval alone. Across 1200 cells +simulated from the harness's log-normal noise model the CI clause passed while +the raw p-clause failed zero times, so *on measurements shaped like these* +clause 2 contributes exactly one thing clause 1 does not: the multiplicity +adjustment. + +That is an observation about those distributions, not an implication — +`verify.ci_without_significance()` constructs a cell where the CI clause +passes and the raw p-clause does not. Signed-rank ranks differences by +magnitude, so 22 rounds clustered just above the margin against 8 swinging far +below it give a bootstrap CI of [1.211, 1.223] at a slower-tail p of 0.343. +Nothing in the harness produces that shape, but the gate should not be +documented as if it could not. + +The conjunction is load-bearing in a second way regardless — it is also what +shields the gate from the signed-rank point null being the wrong null on +[skewed positive data](#log-ratios). + #### The two tails Every comparison carries two one-sided p-values: `p_value` for "candidate is @@ -292,6 +350,35 @@ is a multiplicity correction running backwards: adding cells would manufacture improvements rather than suppress false ones. `test_adjusted_slower_tail_is_not_read_as_evidence_of_improvement` pins this. +![Grouped columns over runs of 4, 12 and 24 cells, counting acceptances of the +improvement p-clause under a pure null. The old rule's count rises from 0.43 to +6.28 per run; the new rule's stays flat near +0.06.](docs/fig2-fdr-backwards.svg) + +The scale of it, under a pure null where every acceptance is false by +construction: the old rule accepted on 0.43 cells per 4-cell run and 6.28 per +24-cell run — about a quarter of everything measured — while the new rule holds +near 0.06 whatever the run size. At one cell the two rules agree to within +0.002, which is what identifies this as a multiplicity artifact rather than a +discreteness one. + +Those are counts for the *p-clause on its own*, not for completed IMPROVED +verdicts. On this null the accompanying CI clause (`ci_high < 1/1.15`) passed +0 of 2000 cells, so neither rule would have published a false IMPROVED — the +conjunction is what kept the broken clause off the output. Isolating the +clause is deliberate: it is the half whose behaviour under multiplicity is at +issue, and scoring whole verdicts would report zero for both rules and +distinguish nothing. A clause that gets *more* permissive as the run grows is +worth fixing while the other clause is still masking it. + +Discreteness is the smaller of the two problems and is easy to over-credit. +scipy uses the exact signed-rank null only when `n <= 50` *and* there are no +ties, so the two tails sum to 1 + the point mass there, and to exactly 1 +otherwise: the excess is about +0.010 at n = 20, +0.004 at n = 30, and zero at +n >= 51. Ties from a floored RSS push a cell onto the normal approximation and +*remove* the discrepancy rather than worsening it, so this lives only in the +clean wall/cpu cells. + #### Separate BH families Gated keys are corrected as their own family. Ungated metrics get a family of @@ -443,6 +530,11 @@ two builds doing different amounts of work) is invisible in the output. 6. Never run the measured command from a process holding memory. 7. Never run the benchmark in parallel with anything, including itself. 8. Never let a run that measured nothing report success. +9. Never infer one tail of a test from the other, and never read an *adjusted* + p-value in the direction it was not adjusted for. +10. Never hand-edit the figures. Change `docs/make_figures.py` and regenerate; + if a number moves, `--verify` is what tells you whether the figure or the + world changed. Every one of these fails *silently* and *plausibly* when broken: the numbers still look like numbers. That is why they are written down. diff --git a/xtest/perf/docs/__init__.py b/xtest/perf/docs/__init__.py new file mode 100644 index 000000000..1296dd9bd --- /dev/null +++ b/xtest/perf/docs/__init__.py @@ -0,0 +1,8 @@ +"""Documentation tooling for the benchmark's statistical figures. + +Nothing here is imported by the benchmark runtime. ``perf.stats``, +``perf.runner``, and ``perf.report`` do not depend on this package, and no test +collects it -- the figures are prose, not measurement. + +Run ``python -m perf.docs.make_figures`` to regenerate the committed SVGs. +""" diff --git a/xtest/perf/docs/fig1-three-regions.svg b/xtest/perf/docs/fig1-three-regions.svg new file mode 100644 index 000000000..3e872bb65 --- /dev/null +++ b/xtest/perf/docs/fig1-three-regions.svg @@ -0,0 +1,50 @@ + +Three verdicts, two tests of the same point null +A log-scaled ratio axis banded into IMPROVED below 0.87, no verdict, and REGRESSION above 1.15. A vertical rule at 1.00 marks where both Wilcoxon tests actually test. Four example cells are drawn as confidence intervals, one per region. + + +Three verdicts, two tests of the same point null +Where the gate's clauses actually sit on the ratio axis. + + + +IMPROVED +no verdict +REGRESSION + + + +both p-values test H0: ratio = 1 + + + + + +1.30 [1.28, 1.35] + + + + +1.09 [1.05, 1.11] + + + + +0.98 [0.96, 1.01] + + + + +0.81 [0.79, 0.83] + +0.70 +0.87 = 1/1.15 +1.00 +1.15 +1.30 +1.45 +ratio of medians, candidate / baseline (log scale) +The margins at 0.87 and 1.15 are carried by the confidence interval alone. +On 1200 cells simulated from the harness's noise model the CI clause passed while the raw p-clause failed 0 times. +It is not implied, though: a constructed cell reaches [1.21, 1.22] at p = 0.34. + diff --git a/xtest/perf/docs/fig2-fdr-backwards.svg b/xtest/perf/docs/fig2-fdr-backwards.svg new file mode 100644 index 000000000..be1822275 --- /dev/null +++ b/xtest/perf/docs/fig2-fdr-backwards.svg @@ -0,0 +1,40 @@ + +A multiplicity correction running backwards +Grouped columns over runs of 4, 12 and 24 cells, counting cells that accept the improvement p-clause under a pure null. The old rule's count rises from 0.43 to 6.28 per run; the new rule's stays flat near 0.06. The CI clause of the same verdict passed zero of 2000 null cells, so these are clause acceptances rather than completed IMPROVED verdicts. + + +A multiplicity correction running backwards +Cells per run accepting the improvement p-clause, everything drawn from a pure null. + +old: p_adj_slower > 0.95 + +new: p_adj_faster < 0.05 + +0 + +2 + +4 + +6 + + +0.43 + +0.05 + +2.71 + +0.07 + +6.28 + +0.06 +4 cells +12 cells +24 cells +cells in the run +BH never lowers a p-value, so a test that asks for a large adjusted p gets easier as a run grows. +These count clause 2 alone. The CI clause passed 0 of 2000 null cells, so neither +rule completed a verdict here -- the conjunction is what hid the broken clause. + diff --git a/xtest/perf/docs/fig3-positivity.svg b/xtest/perf/docs/fig3-positivity.svg new file mode 100644 index 000000000..e1798ddbb --- /dev/null +++ b/xtest/perf/docs/fig3-positivity.svg @@ -0,0 +1,87 @@ + +A positive, right-tailed measurement +Two panels. On the left, the spread of the raw paired difference grows with the effect while the log-ratio's stays flat at 0.141. On the right, stalling the candidate arm only drives the slower tail's rejection rate from 0.047 to 0.815 and the faster tail's to zero, while the median ratio moves only from 1.000 to 1.068. A text row shows the same stall rates applied to both arms, where the true ratio stays 1 and the slower tail holds between 0.039 and 0.057 -- so the curves measure sensitivity to asymmetric contamination, not test size. + + +A positive, right-tailed measurement +Taking logs makes the spread independent of the effect. It does not make the two tails behave alike. +Scale: the log absorbs it + +0 + +0.15 + +0.30 + +0.45 + + + + + + + + + + + +0.412 +0.141 - flat +1.0 +1.3 +2.0 +4.0 +true ratio (candidate / baseline) + +sd of raw difference + +sd of log-ratio +Skew: it does not + +0 + +0.2 + +0.4 + +0.6 + +0.8 + + +alpha = 0.05 + + + + + + + + + + +0.815 +0.000 +0% +5% +15% +30% +median ratio +1.000 +1.008 +1.026 +1.068 +both arms stalled +0.048 +0.051 +0.057 +0.039 +share of rounds stalled, on the candidate arm only + +slower tail + +faster tail +Signed-rank needs the paired difference symmetric under H0. Stalling one arm breaks that directionally -- and moves the true median, so the +plotted rates are power against a 2.6% effect, not size. Stall both arms and the true ratio stays 1: the slower tail holds at nominal. +Either way it is the interval, not the p-value, that refuses to call 1.026 a regression. + diff --git a/xtest/perf/docs/make_figures.py b/xtest/perf/docs/make_figures.py new file mode 100644 index 000000000..565719ca4 --- /dev/null +++ b/xtest/perf/docs/make_figures.py @@ -0,0 +1,634 @@ +"""Render the three figures that explain the benchmark's decision rule. + +Run ``python -m perf.docs.make_figures`` from ``xtest/`` to rewrite the +committed SVGs, ``--check`` to fail on drift, and ``--verify`` to re-run the +Monte Carlo behind the frozen constants below. + +Every number here came out of :mod:`perf.docs.verify`. They are frozen rather +than computed at render time so that the committed SVGs are a pure function of +literals and geometry: a scipy upgrade that shifted a tie correction would +otherwise rewrite three binary-looking files in a diff nobody can read. +``--verify`` is how the freezing stays honest. +""" + +from __future__ import annotations + +import argparse +from collections.abc import Callable, Sequence +from pathlib import Path + +from perf.docs import svgkit as k +from perf.docs.svgkit import Band, Canvas, Panel, Scale, series + +# --- Figure 1 --------------------------------------------------------------- + +#: Default gating margins, from ``perf.stats.DEFAULT_THRESHOLD``. +THRESHOLD = 1.15 +IMPROVE_MARGIN = 1 / THRESHOLD # 0.8696 + +#: ``(ratio, ci_low, ci_high, right-hand label, mark color)`` for four cells +#: planted one per region. From ``verify.simulate_example_cells()``. +FIG1_CELLS: tuple[tuple[float, float, float, str, str], ...] = ( + (1.304, 1.276, 1.346, "1.30 [1.28, 1.35]", series(1)), + (1.087, 1.054, 1.110, "1.09 [1.05, 1.11]", "var(--muted)"), + (0.983, 0.956, 1.005, "0.98 [0.96, 1.01]", "var(--muted)"), + (0.808, 0.793, 0.825, "0.81 [0.79, 0.83]", series(2)), +) + +#: Cells where the CI clause passed but the unadjusted p-clause did not, out of +#: 1200. From ``verify.simulate_binding_clause()``. +FIG1_BINDING_DISAGREEMENTS = 0 +FIG1_BINDING_TRIALS = 1200 + +#: ``(ci_low, ci_high, p_slower)`` for a constructed cell that passes the CI +#: clause on a raw p of 0.343. From ``verify.ci_without_significance()``: the +#: zero above is a fact about the harness's noise model, not an implication. +FIG1_COUNTEREXAMPLE = (1.211, 1.223, 0.343) + +# --- Figure 2 --------------------------------------------------------------- + +#: From ``verify.simulate_improvement_p_clause((4, 12, 24))``, rounded to 2dp. +#: These count acceptances of the improvement *p-clause*, not whole IMPROVED +#: verdicts -- see ``FIG2_NULL_CI_PASSES``. +FIG2_CELLS = (4, 12, 24) +FIG2_OLD_RULE = (0.43, 2.71, 6.28) +FIG2_NEW_RULE = (0.05, 0.07, 0.06) + +#: Null cells out of 2000 whose CI clause for IMPROVED passed, from +#: ``verify.simulate_null_improvement_ci()``. Zero, so neither rule above ever +#: completed a false IMPROVED verdict on this null: the conjunction masked the +#: broken clause rather than the clause being harmless. +FIG2_NULL_CI_PASSES = 0 +FIG2_NULL_CI_TRIALS = 2000 + +# --- Figure 3 --------------------------------------------------------------- + +#: From ``verify.simulate_scale(...)`` and ``verify.simulate_stall_tails(...)``, +#: rounded to 3dp. +FIG3_RATIOS = (1.0, 1.3, 2.0, 4.0) +FIG3_SD_RAW = (0.142, 0.164, 0.224, 0.412) +FIG3_SD_LOG = (0.142, 0.141, 0.141, 0.141) + +FIG3_STALL_P = (0.0, 0.05, 0.15, 0.30) +#: Stalls on the candidate arm only. The true median ratio moves with them, so +#: these are rejection rates under a (small) real effect, not test size. +FIG3_SLOWER_TAIL = (0.047, 0.107, 0.362, 0.815) +FIG3_FASTER_TAIL = (0.049, 0.017, 0.001, 0.000) +FIG3_MEDIAN_RATIO = (1.000, 1.008, 1.026, 1.068) +#: The same stall rate on both arms: contamination just as heavy, true ratio +#: still 1. This row is the one that measures size, and it stays at nominal. +FIG3_SLOWER_BOTH_ARMS = (0.048, 0.051, 0.057, 0.039) + +ALPHA = 0.05 + + +def fig1_three_regions() -> str: + """The decision space: three verdicts, but both tests sit at the point null.""" + w, h = 900.0, 378.0 + panel = Panel(x=64, y=92, w=626, h=160) + scale = Scale(0.70, 1.45, panel.x, panel.right, log=True) + c = Canvas() + + c.text( + 64, + 30, + "Three verdicts, two tests of the same point null", + size=15, + weight="bold", + ) + c.text( + 64, + 52, + "Where the gate's clauses actually sit on the ratio axis.", + cls="muted", + ) + + x_improve = scale.at(IMPROVE_MARGIN) + x_regress = scale.at(THRESHOLD) + x_null = scale.at(1.0) + + # Region washes. "No verdict" is grey on purpose: the absence of a verdict + # is not a fourth category competing with the other two. + c.rect(panel.x, panel.y, x_improve - panel.x, panel.h, fill=series(2), opacity=0.12) + c.rect( + x_improve, + panel.y, + x_regress - x_improve, + panel.h, + fill="var(--grid)", + opacity=0.38, + ) + c.rect( + x_regress, + panel.y, + panel.right - x_regress, + panel.h, + fill=series(1), + opacity=0.12, + ) + + c.text( + (panel.x + x_improve) / 2, + panel.y + 16, + "IMPROVED", + cls="muted", + size=11, + anchor="middle", + weight="bold", + ) + # Offset into the left half of the middle band, clear of the point-null + # rule and of its callout directly above. + c.text( + (x_improve + x_null) / 2, + panel.y + 16, + "no verdict", + cls="muted", + size=11, + anchor="middle", + ) + c.text( + (x_regress + panel.right) / 2, + panel.y + 16, + "REGRESSION", + cls="muted", + size=11, + anchor="middle", + weight="bold", + ) + + # The margins, and then the point null drawn over them. + c.rule(x_improve, panel.y, x_improve, panel.bottom, cls="rule") + c.rule(x_regress, panel.y, x_regress, panel.bottom, cls="rule") + c.stroke(x_null, panel.y, x_null, panel.bottom, stroke=series(0), width=2) + c.text(x_null, 76, "both p-values test H0: ratio = 1", size=11, anchor="middle") + c.stroke(x_null, 80, x_null, panel.y, stroke=series(0), width=1) + + for i, (ratio, lo, hi, label, color) in enumerate(FIG1_CELLS): + y = panel.y + 44 + i * 30 + c.stroke(scale.at(lo), y, scale.at(hi), y, stroke=color, width=2) + for bound in (lo, hi): + c.stroke( + scale.at(bound), y - 5, scale.at(bound), y + 5, stroke=color, width=2 + ) + c.dot(scale.at(ratio), y, fill=color) + c.text(panel.right + 14, y + 4, label, size=11) + + c.rule(panel.x, panel.bottom, panel.right, panel.bottom, cls="axis") + # No 0.80 tick: its label collides with the wider "0.87 = 1/1.15" one, and + # the margin is the number worth reading here. + for tick, text in ( + (0.70, "0.70"), + (IMPROVE_MARGIN, "0.87 = 1/1.15"), + (1.00, "1.00"), + (1.15, "1.15"), + (1.30, "1.30"), + (1.45, "1.45"), + ): + c.text( + scale.at(tick), + panel.bottom + 20, + text, + cls="muted", + size=11, + anchor="middle", + ) + c.text( + (panel.x + panel.right) / 2, + panel.bottom + 42, + "ratio of medians, candidate / baseline (log scale)", + cls="muted", + size=11, + anchor="middle", + ) + + c.text( + 64, + 322, + "The margins at 0.87 and 1.15 are carried by the confidence interval alone.", + cls="muted", + size=11, + ) + c.text( + 64, + 340, + f"On {FIG1_BINDING_TRIALS} cells simulated from the harness's noise model " + f"the CI clause passed while the raw p-clause failed " + f"{FIG1_BINDING_DISAGREEMENTS} times.", + cls="muted", + size=11, + ) + # The zero above invites "so clause 1 implies clause 2", which is false. + ci_low, ci_high, p_slower = FIG1_COUNTEREXAMPLE + c.text( + 64, + 358, + f"It is not implied, though: a constructed cell reaches " + f"[{ci_low:.2f}, {ci_high:.2f}] at p = {p_slower:.2f}.", + cls="muted", + size=11, + ) + + return k.document( + w, + h, + c.render(), + title="Three verdicts, two tests of the same point null", + desc=( + "A log-scaled ratio axis banded into IMPROVED below 0.87, no verdict, " + "and REGRESSION above 1.15. A vertical rule at 1.00 marks where both " + "Wilcoxon tests actually test. Four example cells are drawn as " + "confidence intervals, one per region." + ), + ) + + +def fig2_correction_runs_backwards() -> str: + """False improvement p-clause acceptances per run, old rule against new.""" + w, h = 660.0, 428.0 + panel = Panel(x=74, y=96, w=536, h=204) + y = Scale(0, 7, panel.bottom, panel.y) + band = Band(len(FIG2_CELLS), panel.x, panel.right, pad=0.40) + c = Canvas() + + c.text( + 64, 30, "A multiplicity correction running backwards", size=15, weight="bold" + ) + c.text( + 64, + 52, + "Cells per run accepting the improvement p-clause, everything drawn " + "from a pure null.", + cls="muted", + ) + # Slots are assigned in declaration order, not by sentiment. Do not "fix" + # this to paint the broken rule in a warning color: status hues are + # reserved, and recoloring by verdict would make the two panels of figure 3 + # disagree with this one. + k.legend( + c, 64, 76, [(0, "old: p_adj_slower > 0.95"), (1, "new: p_adj_faster < 0.05")] + ) + + k.y_axis(c, panel, y, [0, 2, 4, 6], ["0", "2", "4", "6"]) + c.rule(panel.x, panel.bottom, panel.right, panel.bottom, cls="axis") + + for i, (old, new) in enumerate(zip(FIG2_OLD_RULE, FIG2_NEW_RULE, strict=True)): + for slot, value in ((0, old), (1, new)): + x, width = band.slot(i, slot, 2) + if width > 30.0: + # Cap the width but keep the pair together: growing the gap + # instead would read as two separate groups. + x += (width - 30.0) / 2 + width = 30.0 + c.column(x, panel.bottom, y.at(value), width, fill=series(slot)) + c.text( + x + width / 2, y.at(value) - 8, f"{value:.2f}", size=11, anchor="middle" + ) + + k.x_ticks( + c, + panel, + [band.center(i) for i in range(len(FIG2_CELLS))], + [f"{m} cells" for m in FIG2_CELLS], + ) + c.text( + (panel.x + panel.right) / 2, + panel.bottom + 44, + "cells in the run", + cls="muted", + size=11, + anchor="middle", + ) + + c.text( + 64, + 372, + "BH never lowers a p-value, so a test that asks for a large adjusted " + "p gets easier as a run grows.", + cls="muted", + size=11, + ) + # The clause, not the verdict. Without these lines the columns read as + # false IMPROVED verdicts, a count neither rule reaches on this null. + c.text( + 64, + 390, + f"These count clause 2 alone. The CI clause passed {FIG2_NULL_CI_PASSES} " + f"of {FIG2_NULL_CI_TRIALS} null cells, so neither", + cls="muted", + size=11, + ) + c.text( + 64, + 408, + "rule completed a verdict here -- the conjunction is what hid the " + "broken clause.", + cls="muted", + size=11, + ) + + return k.document( + w, + h, + c.render(), + title="A multiplicity correction running backwards", + desc=( + "Grouped columns over runs of 4, 12 and 24 cells, counting cells " + "that accept the improvement p-clause under a pure null. The old " + "rule's count rises from 0.43 to 6.28 per run; the new rule's " + "stays flat near 0.06. The CI clause of the same verdict passed " + "zero of 2000 null cells, so these are clause acceptances rather " + "than completed IMPROVED verdicts." + ), + ) + + +def _line_series( + c: Canvas, band: Band, y: Scale, values: Sequence[float], slot: int +) -> None: + points = [(band.center(i), y.at(v)) for i, v in enumerate(values)] + c.polyline(points, stroke=series(slot)) + for px, py in points: + c.dot(px, py, fill=series(slot)) + + +def fig3_positivity() -> str: + """Two panels: the log transform fixes the scale problem, not the skew one.""" + w, h = 1000.0, 488.0 + left = Panel(x=74, y=96, w=366, h=190) + right = Panel(x=596, y=96, w=354, h=190) + c = Canvas() + + c.text(64, 30, "A positive, right-tailed measurement", size=15, weight="bold") + c.text( + 64, + 52, + "Taking logs makes the spread independent of the effect. It does not " + "make the two tails behave alike.", + cls="muted", + ) + + # Left panel: spread against the size of a real effect. + c.text(left.x, 80, "Scale: the log absorbs it", size=12, weight="bold") + y_left = Scale(0, 0.45, left.bottom, left.y) + band_left = Band(len(FIG3_RATIOS), left.x, left.right, pad=0.30) + k.y_axis(c, left, y_left, [0, 0.15, 0.30, 0.45], ["0", "0.15", "0.30", "0.45"]) + c.rule(left.x, left.bottom, left.right, left.bottom, cls="axis") + _line_series(c, band_left, y_left, FIG3_SD_RAW, 0) + _line_series(c, band_left, y_left, FIG3_SD_LOG, 1) + last_left = band_left.center(len(FIG3_RATIOS) - 1) + c.text( + last_left, y_left.at(FIG3_SD_RAW[-1]) - 14, "0.412", size=11, anchor="middle" + ) + c.text( + last_left, + y_left.at(FIG3_SD_LOG[-1]) + 22, + "0.141 - flat", + size=11, + anchor="middle", + ) + k.x_ticks( + c, + left, + [band_left.center(i) for i in range(len(FIG3_RATIOS))], + [f"{r:.1f}" for r in FIG3_RATIOS], + ) + # Both axis titles sit at bottom + 82 so the two panels align, even though + # only the right one needs the extra rows its strips occupy. + c.text( + (left.x + left.right) / 2, + left.bottom + 82, + "true ratio (candidate / baseline)", + cls="muted", + size=11, + anchor="middle", + ) + k.legend(c, left.x, 398, [(0, "sd of raw difference"), (1, "sd of log-ratio")]) + + # Right panel: its own y-axis, because two measures never share one. + c.text(right.x, 80, "Skew: it does not", size=12, weight="bold") + y_right = Scale(0, 0.85, right.bottom, right.y) + band_right = Band(len(FIG3_STALL_P), right.x, right.right, pad=0.30) + k.y_axis( + c, right, y_right, [0, 0.2, 0.4, 0.6, 0.8], ["0", "0.2", "0.4", "0.6", "0.8"] + ) + c.rule(right.x, right.bottom, right.right, right.bottom, cls="axis") + y_alpha = y_right.at(ALPHA) + c.rule(right.x, y_alpha, right.right, y_alpha, cls="ref") + # Left end, not right: the faster tail lands on zero at the right edge and + # its direct label would sit on top of this one. + c.text(right.x + 4, y_alpha - 8, "alpha = 0.05", cls="muted", size=11) + _line_series(c, band_right, y_right, FIG3_SLOWER_TAIL, 0) + _line_series(c, band_right, y_right, FIG3_FASTER_TAIL, 1) + last_right = band_right.center(len(FIG3_STALL_P) - 1) + c.text( + last_right, + y_right.at(FIG3_SLOWER_TAIL[-1]) - 14, + "0.815", + size=11, + anchor="middle", + ) + c.text( + last_right, + y_right.at(FIG3_FASTER_TAIL[-1]) - 14, + "0.000", + size=11, + anchor="middle", + ) + k.x_ticks( + c, + right, + [band_right.center(i) for i in range(len(FIG3_STALL_P))], + [f"{p:.0%}" for p in FIG3_STALL_P], + ) + + # The median ratio and the both-arms control are further quantities on + # other measures. They go in labelled text rows, not on more y-axes. + # The control row is what stops the plotted curves reading as test size: + # stalling one arm moves the median off 1, so the null it would be the + # size of is not the null being simulated. + for row, label, values, fmt in ( + (42, "median ratio", FIG3_MEDIAN_RATIO, "{:.3f}"), + (60, "both arms stalled", FIG3_SLOWER_BOTH_ARMS, "{:.3f}"), + ): + c.text( + right.x - 12, + right.bottom + row, + label, + cls="muted", + size=11, + anchor="end", + ) + for i, value in enumerate(values): + c.text( + band_right.center(i), + right.bottom + row, + fmt.format(value), + cls="muted", + size=11, + anchor="middle", + ) + c.text( + (right.x + right.right) / 2, + right.bottom + 82, + "share of rounds stalled, on the candidate arm only", + cls="muted", + size=11, + anchor="middle", + ) + k.legend(c, right.x, 398, [(0, "slower tail"), (1, "faster tail")]) + + c.text( + 64, + 432, + "Signed-rank needs the paired difference symmetric under H0. Stalling " + "one arm breaks that directionally -- and moves the true median, so the", + cls="muted", + size=11, + ) + c.text( + 64, + 450, + "plotted rates are power against a 2.6% effect, not size. Stall both " + "arms and the true ratio stays 1: the slower tail holds at nominal.", + cls="muted", + size=11, + ) + c.text( + 64, + 468, + "Either way it is the interval, not the p-value, that refuses to call " + "1.026 a regression.", + cls="muted", + size=11, + ) + + return k.document( + w, + h, + c.render(), + title="A positive, right-tailed measurement", + desc=( + "Two panels. On the left, the spread of the raw paired difference " + "grows with the effect while the log-ratio's stays flat at 0.141. " + "On the right, stalling the candidate arm only drives the slower " + "tail's rejection rate from 0.047 to 0.815 and the faster tail's " + "to zero, while the median ratio moves only from 1.000 to 1.068. " + "A text row shows the same stall rates applied to both arms, " + "where the true ratio stays 1 and the slower tail holds between " + "0.039 and 0.057 -- so the curves measure sensitivity to " + "asymmetric contamination, not test size." + ), + ) + + +FIGURES: tuple[tuple[str, Callable[[], str]], ...] = ( + ("fig1-three-regions.svg", fig1_three_regions), + ("fig2-fdr-backwards.svg", fig2_correction_runs_backwards), + ("fig3-positivity.svg", fig3_positivity), +) + + +def _verify() -> int: + """Re-run the Monte Carlo and compare it to the constants above.""" + from perf.docs import verify + + failures: list[str] = [] + + def check(name: str, got: object, want: object) -> None: + if got != want: + failures.append(f"{name}: simulated {got}, figure says {want}") + + old, new = verify.simulate_improvement_p_clause(FIG2_CELLS) + check("fig2 old rule", tuple(round(v, 2) for v in old), FIG2_OLD_RULE) + check("fig2 new rule", tuple(round(v, 2) for v in new), FIG2_NEW_RULE) + check( + "fig2 null CI clause", + verify.simulate_null_improvement_ci(trials=FIG2_NULL_CI_TRIALS), + FIG2_NULL_CI_PASSES, + ) + + raw, log = verify.simulate_scale(FIG3_RATIOS) + check("fig3 sd raw", tuple(round(v, 3) for v in raw), FIG3_SD_RAW) + check("fig3 sd log", tuple(round(v, 3) for v in log), FIG3_SD_LOG) + + slower, faster, both, medians = verify.simulate_stall_tails(FIG3_STALL_P) + check("fig3 slower tail", tuple(round(v, 3) for v in slower), FIG3_SLOWER_TAIL) + check("fig3 faster tail", tuple(round(v, 3) for v in faster), FIG3_FASTER_TAIL) + check( + "fig3 both arms stalled", + tuple(round(v, 3) for v in both), + FIG3_SLOWER_BOTH_ARMS, + ) + check("fig3 median ratio", tuple(round(v, 3) for v in medians), FIG3_MEDIAN_RATIO) + + check( + "fig1 binding clause", + verify.simulate_binding_clause(), + FIG1_BINDING_DISAGREEMENTS, + ) + check( + "fig1 counterexample", + verify.ci_without_significance(), + FIG1_COUNTEREXAMPLE, + ) + check( + "fig1 example cells", + verify.simulate_example_cells(), + tuple(cell[:3] for cell in FIG1_CELLS), + ) + + for line in failures: + print(f"MISMATCH {line}") + if failures: + print(f"\n{len(failures)} constant(s) no longer match the simulation.") + return 1 + print("All figure constants match the simulation.") + return 0 + + +def main(argv: Sequence[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--out", + type=Path, + default=Path(__file__).parent, + help="directory to write the SVGs into", + ) + parser.add_argument( + "--check", + action="store_true", + help="compare against the committed files instead of writing", + ) + parser.add_argument( + "--verify", + action="store_true", + help="re-run the Monte Carlo behind the frozen constants", + ) + args = parser.parse_args(argv) + + if args.verify: + return _verify() + + stale: list[str] = [] + for name, build in FIGURES: + svg = build() + path = args.out / name + if args.check: + if not path.exists() or path.read_text(encoding="utf-8") != svg: + stale.append(name) + else: + path.write_text(svg, encoding="utf-8") + print(f"wrote {path}") + + if args.check: + for name in stale: + print(f"STALE {name}") + if stale: + print("\nRun: python -m perf.docs.make_figures") + return 1 + print("All figures are up to date.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/xtest/perf/docs/svgkit.py b/xtest/perf/docs/svgkit.py new file mode 100644 index 000000000..40e0ac7a0 --- /dev/null +++ b/xtest/perf/docs/svgkit.py @@ -0,0 +1,328 @@ +"""Just enough SVG to draw three figures, and deliberately no more. + +What is missing is the point. There is no nice-number tick algorithm, no domain +inference, and no text-measurement engine: tick arrays are literals in the +figure code and gutters are generous. A charting library has to guess what the +caller meant; three hand-laid figures do not, and every guess we skip is a +class of bug we do not have to debug in a file nobody looks at twice a year. + +Two rules are enforced by the shapes here rather than by discipline: + +- :meth:`Canvas.text` takes a *class* (``ink`` or ``muted``), not a color. Text + therefore cannot wear a series color, because the signature gives you no way + to ask for one. +- Every coordinate goes through :func:`n`. A 1e-16 wobble in a scale + computation cannot change a committed byte, so regenerating the figures on a + different CPU produces no diff. + +Colors are emitted as ``var(--s1)`` and friends, resolved by the stylesheet +:func:`document` writes. That is what lets one geometry pass serve both themes. +""" + +from __future__ import annotations + +import math +from dataclasses import dataclass +from typing import Literal + +#: Text roles. Deliberately not a color -- see the module docstring. +InkClass = Literal["ink", "muted"] + +#: Line roles, resolved by the stylesheet. +LineClass = Literal["grid", "axis", "ref", "rule"] + +_FONT = 'ui-sans-serif, -apple-system, "Segoe UI", Roboto, Helvetica, sans-serif' + + +def n(v: float) -> str: + """Format a coordinate to a stable two decimals, with ``-0`` normalized. + + Float formatting is the whole reason the committed SVGs do not churn. + """ + s = f"{v:.2f}" + if s.startswith("-") and float(s) == 0.0: + return s[1:] + return s + + +def esc(s: str) -> str: + """XML-escape text content. + + Not theoretical: figure 2's legend reads ``p_adj_slower > 0.95``. + """ + return s.replace("&", "&").replace("<", "<").replace(">", ">") + + +@dataclass(frozen=True, slots=True) +class Theme: + """One resolved set of colors. Two instances exist: :data:`LIGHT`, :data:`DARK`.""" + + surface: str + ink: str + muted: str + grid: str + #: Categorical slots 1-3, in fixed order. Never cycled, never reordered: + #: a figure's fourth series does not exist, it becomes a second panel. + series: tuple[str, str, str] + + +#: Validated under ``--pairs all``: worst CVD dE 9.2, worst normal-vision dE 24.0. +#: ``#1baf7a`` sits at 2.74:1 on this surface, so anything drawn in it must also +#: carry a visible direct label. +LIGHT = Theme( + surface="#fcfcfb", + ink="#0b0b0b", + muted="#52514e", + grid="#dedcd4", + series=("#2a78d6", "#eb6834", "#1baf7a"), +) + +#: The same three hues re-stepped for the dark surface, not a second palette. +#: Validated under ``--pairs all``: every check passes, contrast included. +DARK = Theme( + surface="#1a1a19", + ink="#ffffff", + muted="#c3c2b7", + grid="#3b3a37", + series=("#3987e5", "#d95926", "#199e70"), +) + + +def series(slot: int) -> str: + """Return the CSS variable for categorical slot ``slot`` (0-based).""" + if not 0 <= slot <= 2: + raise ValueError(f"categorical slot must be 0..2, got {slot}") + return f"var(--s{slot + 1})" + + +@dataclass(frozen=True, slots=True) +class Scale: + """Maps a data value onto a pixel coordinate, linearly or in log space.""" + + lo: float + hi: float + px0: float + px1: float + log: bool = False + + def at(self, v: float) -> float: + if self.log: + span = math.log(self.hi) - math.log(self.lo) + t = (math.log(v) - math.log(self.lo)) / span + else: + t = (v - self.lo) / (self.hi - self.lo) + return self.px0 + t * (self.px1 - self.px0) + + +@dataclass(frozen=True, slots=True) +class Panel: + """A plot area. A figure with two of these has two y-axes and no dual axis.""" + + x: float + y: float + w: float + h: float + + @property + def right(self) -> float: + return self.x + self.w + + @property + def bottom(self) -> float: + return self.y + self.h + + +@dataclass(frozen=True, slots=True) +class Band: + """Equal-width categorical slots across a pixel range.""" + + count: int + px0: float + px1: float + #: Fraction of each step left empty, so adjacent groups do not touch. + pad: float = 0.34 + + @property + def step(self) -> float: + return (self.px1 - self.px0) / self.count + + def center(self, i: int) -> float: + return self.px0 + self.step * (i + 0.5) + + def slot(self, i: int, k: int, of: int, *, gap: float = 2.0) -> tuple[float, float]: + """Return ``(x, width)`` for column ``k`` of ``of`` inside group ``i``. + + ``gap`` is the surface gap between adjacent columns, which is what keeps + two touching fills from reading as one shape. + """ + inner = self.step * (1.0 - self.pad) + width = (inner - gap * (of - 1)) / of + left = self.center(i) - inner / 2.0 + return left + k * (width + gap), width + + +class Canvas: + """Accumulates SVG fragments. The whole mark vocabulary is its methods.""" + + def __init__(self) -> None: + self.parts: list[str] = [] + + def render(self) -> str: + return "\n".join(self.parts) + + def rect( + self, + x: float, + y: float, + w: float, + h: float, + *, + fill: str, + rx: float = 0.0, + opacity: float = 1.0, + ) -> None: + radius = f' rx="{n(rx)}"' if rx else "" + alpha = f' opacity="{n(opacity)}"' if opacity != 1.0 else "" + self.parts.append( + f'' + ) + + def column(self, x: float, y_base: float, y_top: float, w: float, *, fill: str): + """A column rounded at the data end only. + + ``rect rx=4`` would round the baseline too, which reads as a floating + lozenge rather than a quantity standing on an axis. + """ + h = y_base - y_top + if h < 1.5: + # Keep a near-zero quantity visible; the direct label carries the + # real value. A bar that renders as nothing reads as missing data. + h = 1.5 + y_top = y_base - h + r = min(4.0, w / 2.0, h) + self.parts.append( + f'' + ) + + def polyline(self, points: list[tuple[float, float]], *, stroke: str) -> None: + pts = " ".join(f"{n(px)},{n(py)}" for px, py in points) + self.parts.append( + f'' + ) + + def dot(self, cx: float, cy: float, *, fill: str, r: float = 4.5) -> None: + """A marker with a 2px surface ring, so overlapping marks stay countable.""" + self.parts.append( + f'' + ) + + def rule( + self, x1: float, y1: float, x2: float, y2: float, *, cls: LineClass + ) -> None: + self.parts.append( + f'' + ) + + def stroke( + self, x1: float, y1: float, x2: float, y2: float, *, stroke: str, width: float + ) -> None: + """A colored 2px-class line. Used for series marks, never for text.""" + self.parts.append( + f'' + ) + + def text( + self, + x: float, + y: float, + s: str, + *, + cls: InkClass = "ink", + size: float = 12.0, + anchor: str = "start", + weight: str = "normal", + ) -> None: + bold = ' font-weight="600"' if weight == "bold" else "" + self.parts.append( + f'{esc(s)}' + ) + + +def y_axis( + c: Canvas, + panel: Panel, + scale: Scale, + ticks: list[float], + labels: list[str], +) -> None: + """Horizontal gridlines with left-hand labels. Grid stays recessive.""" + for value, label in zip(ticks, labels, strict=True): + y = scale.at(value) + c.rule(panel.x, y, panel.right, y, cls="grid") + c.text(panel.x - 10, y + 4, label, cls="muted", size=11, anchor="end") + + +def x_ticks(c: Canvas, panel: Panel, positions: list[float], labels: list[str]) -> None: + """Category labels under a panel, with no tick marks -- the gap is enough.""" + for x, label in zip(positions, labels, strict=True): + c.text(x, panel.bottom + 20, label, cls="muted", size=11, anchor="middle") + + +def legend(c: Canvas, x: float, y: float, entries: list[tuple[int, str]]) -> None: + """A legend row. Present whenever a panel carries two or more series.""" + cursor = x + for slot, label in entries: + c.rect(cursor, y - 8, 10, 10, fill=series(slot), rx=2) + c.text(cursor + 16, y, label, cls="muted", size=11) + cursor += 26 + 6.4 * len(label) + + +def _vars(t: Theme) -> str: + return ( + f"--surface:{t.surface};--ink:{t.ink};--muted:{t.muted};--grid:{t.grid};" + f"--s1:{t.series[0]};--s2:{t.series[1]};--s3:{t.series[2]}" + ) + + +def document(width: float, height: float, body: str, *, title: str, desc: str) -> str: + """Wrap a canvas in a themed, self-contained SVG document. + + The first thing drawn is an opaque surface covering the whole viewBox. On + GitHub this file is served as an image in an isolated document, so its + ``prefers-color-scheme`` follows the *operating system* rather than the + GitHub theme toggle, and Safari ignores the query outright. Painting the + surface means the worst case is a light card on a dark page -- bright, but + never a figure rendered in ink that matches its background. + """ + style = ( + f"svg{{{_vars(LIGHT)}}}" + f"@media (prefers-color-scheme:dark){{svg{{{_vars(DARK)}}}}}" + f"text{{font-family:{_FONT};}}" + ".ink{fill:var(--ink);}" + ".muted{fill:var(--muted);}" + ".grid{stroke:var(--grid);stroke-width:1;}" + ".axis{stroke:var(--muted);stroke-width:1;}" + ".ref{stroke:var(--muted);stroke-width:1;stroke-dasharray:4 3;}" + ".rule{stroke:var(--muted);stroke-width:1;}" + ) + return ( + f'\n' + f'{esc(title)}\n' + f'{esc(desc)}\n' + f"\n" + f'\n' + f"{body}\n" + f"\n" + ) diff --git a/xtest/perf/docs/verify.py b/xtest/perf/docs/verify.py new file mode 100644 index 000000000..1fb21b0ac --- /dev/null +++ b/xtest/perf/docs/verify.py @@ -0,0 +1,278 @@ +"""The Monte Carlo behind the figures, kept off the rendering path. + +:mod:`perf.docs.make_figures` draws from frozen constants, not from these +functions. That split is deliberate. If the figures re-simulated at render +time, a scipy release that changed a tie correction would silently rewrite +three committed SVGs and produce a diff nobody can review. Instead +``make_figures --verify`` runs these and *reports* a mismatch, and a human +decides whether the figure or the world changed. + +Every model here matches the one described beside its constant. +""" + +from __future__ import annotations + +from typing import cast + +import numpy as np +from scipy import stats as _scipy_stats + +from perf import stats + +#: Fixed so the reported numbers are reproducible, not so they are flattering. +SEED_FALSE_IMPROVED = 31 +SEED_NULL_CI = 23 +SEED_SCALE = 11 +SEED_STALLS = 11 +SEED_BINDING = 5 +SEED_CELLS = 17 + +ALPHA = 0.05 +THRESHOLD = 1.15 +TRIALS = 300 + + +def _wilcoxon_tails(d: np.ndarray) -> tuple[float, float]: + """Both one-sided p-values, as :func:`perf.stats._one_sided_ps` computes them.""" + slower = cast(float, _scipy_stats.wilcoxon(d, alternative="greater")[1]) + faster = cast(float, _scipy_stats.wilcoxon(d, alternative="less")[1]) + return slower, faster + + +def _bh(p: list[float]) -> np.ndarray: + return np.asarray( + _scipy_stats.false_discovery_control(np.asarray(p, dtype=float), method="bh") + ) + + +def _null_cell(rng: np.random.Generator, n: int = 30) -> tuple[float, float]: + """One A/A cell: two arms drawn from the same distribution.""" + b = np.exp(rng.normal(0, 0.06, n)) + c = np.exp(rng.normal(0, 0.06, n)) + return _wilcoxon_tails(np.log(c) - np.log(b)) + + +def simulate_improvement_p_clause( + cells: tuple[int, ...], *, trials: int = TRIALS +) -> tuple[tuple[float, ...], tuple[float, ...]]: + """Mean false acceptances of the improvement *p-clause* per run. + + Counts clause 2 of the IMPROVED rule only -- ``p_adjusted_slower > + 1 - alpha`` under the old formulation, ``p_adjusted_faster < alpha`` under + the new one -- not the full verdict, which also requires ``ci_high < + 1 / threshold``. Isolating the clause is the point: it is the clause whose + behaviour under multiplicity is being compared, and pairing it with a CI + condition that never fires on this null would report zero for both rules + and measure nothing. + + :func:`simulate_null_improvement_ci` measures the CI clause on the same + null, which is what says how much of the gate the difference here reaches. + + Everything is a pure null, so every acceptance is false by construction. + """ + rng = np.random.default_rng(SEED_FALSE_IMPROVED) + old: list[float] = [] + new: list[float] = [] + for m in cells: + o = 0 + w = 0 + for _ in range(trials): + tails = [_null_cell(rng) for _ in range(m)] + p_slower = _bh([t[0] for t in tails]) + p_faster = _bh([t[1] for t in tails]) + o += int(np.sum(p_slower > 1 - ALPHA)) + w += int(np.sum(p_faster < ALPHA)) + old.append(o / trials) + new.append(w / trials) + return tuple(old), tuple(new) + + +def simulate_null_improvement_ci( + *, trials: int = 2000, threshold: float = THRESHOLD +) -> int: + """Null cells whose CI clause for IMPROVED passes: ``ci_high < 1/threshold``. + + The answer is zero. The whole interval would have to sit below 0.87 -- + asserting a speedup of at least 13% -- on data with no effect at all, and + at this dispersion it never comes close. So the figure counts p-clause + acceptances rather than whole verdicts, and the broken rule it illustrates + never published a false IMPROVED on its own. + """ + rng = np.random.default_rng(SEED_NULL_CI) + passes = 0 + for _ in range(trials): + b = np.exp(rng.normal(0, 0.06, 30)) + c = np.exp(rng.normal(0, 0.06, 30)) + if stats.compare(b, c, seed=2, n_resamples=999).ci_high < 1 / threshold: + passes += 1 + return passes + + +def simulate_scale( + ratios: tuple[float, ...], *, n: int = 30, trials: int = 3000 +) -> tuple[tuple[float, ...], tuple[float, ...]]: + """Spread of the paired difference on the raw scale and the log scale. + + The raw difference inherits the effect's size because the measurements are + positive; the log-ratio does not. That is what the log transform buys. + """ + rng = np.random.default_rng(SEED_SCALE) + raw: list[float] = [] + log: list[float] = [] + for r in ratios: + rs: list[np.ndarray] = [] + ls: list[np.ndarray] = [] + for _ in range(trials): + b = np.exp(rng.normal(0, 0.10, n) - 0.10**2 / 2) + c = r * np.exp(rng.normal(0, 0.10, n) - 0.10**2 / 2) + rs.append(c - b) + ls.append(np.log(c) - np.log(b)) + raw.append(float(np.concatenate(rs).std())) + log.append(float(np.concatenate(ls).std())) + return tuple(raw), tuple(log) + + +def _stalled( + rng: np.random.Generator, n: int, mean: float, sigma: float, stall_p: float +) -> np.ndarray: + """A positive, right-tailed timing vector with one-sided stall contamination.""" + x = mean * np.exp(rng.normal(0, sigma, n) - sigma**2 / 2) + return x * np.where(rng.random(n) < stall_p, 2.5, 1.0) + + +def _stall_rejection( + rng: np.random.Generator, + stall_p: float, + *, + both_arms: bool, + n: int, + trials: int, +) -> tuple[float, float, float]: + """Rejection rate of each tail, and the median ratio, at one stall rate. + + Returns ``(slower_rate, faster_rate, median_ratio)``. + """ + pg: list[float] = [] + pl: list[float] = [] + meds: list[float] = [] + for _ in range(trials): + b = _stalled(rng, n, 1.0, 0.08, stall_p if both_arms else 0.0) + c = _stalled(rng, n, 1.0, 0.08, stall_p) + d = np.log(c) - np.log(b) + g, f = _wilcoxon_tails(d) + pg.append(g) + pl.append(f) + meds.append(float(np.exp(np.median(d)))) + return ( + float(np.mean(np.asarray(pg) < ALPHA)), + float(np.mean(np.asarray(pl) < ALPHA)), + float(np.median(meds)), + ) + + +def simulate_stall_tails( + stall_ps: tuple[float, ...], *, n: int = 30, trials: int = 1500 +) -> tuple[tuple[float, ...], tuple[float, ...], tuple[float, ...], tuple[float, ...]]: + """Tail rejection rates and median ratio, under one-sided and two-sided stalls. + + Stalling *one* arm is not a null experiment: it shifts the candidate's + distribution, so the true median ratio leaves 1 and the rejections it + produces are power against a small real effect, not test size. Stalling + *both* arms at the same rate keeps the true ratio at 1 while contaminating + just as heavily, and that is the run that measures size. + + Reading them together is what separates the two explanations. Returns + ``(slower_one_arm, faster_one_arm, slower_both_arms, median_ratio)``, the + median ratio being the one from the one-arm run. + + Each run gets its own generator, seeded off :data:`SEED_STALLS`, so adding + or dropping a comparison here cannot shift the others' numbers. + """ + one_arm = np.random.default_rng(SEED_STALLS) + both = np.random.default_rng(SEED_STALLS + 1) + slower: list[float] = [] + faster: list[float] = [] + slower_both: list[float] = [] + medians: list[float] = [] + for sp in stall_ps: + g, f, med = _stall_rejection(one_arm, sp, both_arms=False, n=n, trials=trials) + gb, _, _ = _stall_rejection(both, sp, both_arms=True, n=n, trials=trials) + slower.append(g) + faster.append(f) + slower_both.append(gb) + medians.append(med) + return tuple(slower), tuple(faster), tuple(slower_both), tuple(medians) + + +def simulate_binding_clause( + ratios: tuple[float, ...] = (1.0, 1.10, 1.18, 1.30), + *, + threshold: float = THRESHOLD, + trials: int = TRIALS, +) -> int: + """Count cells where the CI clause passes but the raw p-clause does not. + + The answer is zero over these log-normal draws, so on the harness's own + noise model the p-clause contributes only its multiplicity adjustment. + + That is an empirical statement about this generative model, not an + implication -- :func:`ci_without_significance` constructs a cell where the + CI clause passes and the raw p-clause does not. + """ + rng = np.random.default_rng(SEED_BINDING) + disagreements = 0 + for r in ratios: + for _ in range(trials): + b = np.exp(rng.normal(0, 0.06, 30)) + c = r * np.exp(rng.normal(0, 0.06, 30)) + cmp_ = stats.compare(b, c, seed=2, n_resamples=999) + if cmp_.ci_low > threshold and not cmp_.p_value < ALPHA: + disagreements += 1 + return disagreements + + +def ci_without_significance() -> tuple[float, float, float]: + """A cell passing the CI clause while the raw slower-tail p-clause fails. + + Signed-rank scores each difference by the *rank of its magnitude*, so a + minority of large negative rounds can outweigh a majority of small + positive ones. Here 22 rounds sit tightly just above the margin and 8 swing + far below it: the median -- and so the bootstrap interval around it -- stays + above ``threshold``, while the signed-rank statistic is nearly balanced. + + Nothing in the harness generates this shape; it exists to bound what + :func:`simulate_binding_clause` is allowed to claim. Returns + ``(ci_low, ci_high, p_slower)``. + """ + d = np.concatenate([np.linspace(0.19, 0.21, 22), np.linspace(-2.0, -1.0, 8)]) + lo, hi = _scipy_stats.bootstrap( + (d,), + np.median, + confidence_level=0.95, + method="percentile", + n_resamples=9999, + rng=np.random.default_rng(0), + ).confidence_interval + return ( + round(float(np.exp(lo)), 3), + round(float(np.exp(hi)), 3), + round(float(_wilcoxon_tails(d)[0]), 3), + ) + + +def simulate_example_cells() -> tuple[tuple[float, float, float], ...]: + """Four representative cells, as ``(ratio, ci_low, ci_high)``. + + Planted at a clear slowdown, a real-but-under-margin slowdown, pure noise, + and a clear speedup -- one per region of figure 1. + """ + rng = np.random.default_rng(SEED_CELLS) + out: list[tuple[float, float, float]] = [] + for true_ratio in (1.31, 1.08, 1.00, 0.81): + b = np.exp(rng.normal(0, 0.05, 40)) + c = true_ratio * np.exp(rng.normal(0, 0.05, 40)) + cmp_ = stats.compare(b, c, seed=4, n_resamples=4999) + out.append( + (round(cmp_.ratio, 3), round(cmp_.ci_low, 3), round(cmp_.ci_high, 3)) + ) + return tuple(out) From e4c7c8b0ddcad5200cfce4e30272872cb5bca180 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 29 Sep 2026 09:05:11 -0400 Subject: [PATCH 3/3] refactor(xtest): tidy the figure Monte Carlo after the review fixes No simulated number moves: `make_figures --verify` still matches every frozen constant and `--check` finds the three committed SVGs byte-identical, so this touches only how the simulations are written. - `_pair()` replaces four open-coded log-normal arm pairs. - `SEED_FALSE_IMPROVED` was left stale by the rename to `simulate_improvement_p_clause`; it is now `SEED_P_CLAUSE`. - `verify.THRESHOLD` was a third copy of a gating constant, in the module whose whole job is to agree with the gate. It imports `stats.DEFAULT_THRESHOLD`. - `_stall_rejection`'s `both_arms` flag switched between two genuinely different experiments, and left the control call discarding two of three return values. Split into `simulate_stall_tails` (back to its original 3-tuple) and `simulate_stall_size`, the helper now taking explicit per-arm stall rates. - Dropped a dead format column in figure 3's text-row loop. --- xtest/perf/docs/make_figures.py | 18 +++--- xtest/perf/docs/verify.py | 109 ++++++++++++++++++-------------- 2 files changed, 72 insertions(+), 55 deletions(-) diff --git a/xtest/perf/docs/make_figures.py b/xtest/perf/docs/make_figures.py index 565719ca4..234abfa66 100644 --- a/xtest/perf/docs/make_figures.py +++ b/xtest/perf/docs/make_figures.py @@ -63,8 +63,8 @@ # --- Figure 3 --------------------------------------------------------------- -#: From ``verify.simulate_scale(...)`` and ``verify.simulate_stall_tails(...)``, -#: rounded to 3dp. +#: From ``verify.simulate_scale(...)``, ``verify.simulate_stall_tails(...)`` and +#: ``verify.simulate_stall_size(...)``, rounded to 3dp. FIG3_RATIOS = (1.0, 1.3, 2.0, 4.0) FIG3_SD_RAW = (0.142, 0.164, 0.224, 0.412) FIG3_SD_LOG = (0.142, 0.141, 0.141, 0.141) @@ -446,9 +446,9 @@ def fig3_positivity() -> str: # The control row is what stops the plotted curves reading as test size: # stalling one arm moves the median off 1, so the null it would be the # size of is not the null being simulated. - for row, label, values, fmt in ( - (42, "median ratio", FIG3_MEDIAN_RATIO, "{:.3f}"), - (60, "both arms stalled", FIG3_SLOWER_BOTH_ARMS, "{:.3f}"), + for row, label, values in ( + (42, "median ratio", FIG3_MEDIAN_RATIO), + (60, "both arms stalled", FIG3_SLOWER_BOTH_ARMS), ): c.text( right.x - 12, @@ -462,7 +462,7 @@ def fig3_positivity() -> str: c.text( band_right.center(i), right.bottom + row, - fmt.format(value), + f"{value:.3f}", cls="muted", size=11, anchor="middle", @@ -551,15 +551,15 @@ def check(name: str, got: object, want: object) -> None: check("fig3 sd raw", tuple(round(v, 3) for v in raw), FIG3_SD_RAW) check("fig3 sd log", tuple(round(v, 3) for v in log), FIG3_SD_LOG) - slower, faster, both, medians = verify.simulate_stall_tails(FIG3_STALL_P) + slower, faster, medians = verify.simulate_stall_tails(FIG3_STALL_P) check("fig3 slower tail", tuple(round(v, 3) for v in slower), FIG3_SLOWER_TAIL) check("fig3 faster tail", tuple(round(v, 3) for v in faster), FIG3_FASTER_TAIL) + check("fig3 median ratio", tuple(round(v, 3) for v in medians), FIG3_MEDIAN_RATIO) check( "fig3 both arms stalled", - tuple(round(v, 3) for v in both), + tuple(round(v, 3) for v in verify.simulate_stall_size(FIG3_STALL_P)), FIG3_SLOWER_BOTH_ARMS, ) - check("fig3 median ratio", tuple(round(v, 3) for v in medians), FIG3_MEDIAN_RATIO) check( "fig1 binding clause", diff --git a/xtest/perf/docs/verify.py b/xtest/perf/docs/verify.py index 1fb21b0ac..84fecc0ab 100644 --- a/xtest/perf/docs/verify.py +++ b/xtest/perf/docs/verify.py @@ -18,9 +18,10 @@ from scipy import stats as _scipy_stats from perf import stats +from perf.stats import DEFAULT_THRESHOLD #: Fixed so the reported numbers are reproducible, not so they are flattering. -SEED_FALSE_IMPROVED = 31 +SEED_P_CLAUSE = 31 SEED_NULL_CI = 23 SEED_SCALE = 11 SEED_STALLS = 11 @@ -28,7 +29,6 @@ SEED_CELLS = 17 ALPHA = 0.05 -THRESHOLD = 1.15 TRIALS = 300 @@ -45,10 +45,25 @@ def _bh(p: list[float]) -> np.ndarray: ) +def _pair( + rng: np.random.Generator, + ratio: float = 1.0, + *, + n: int = 30, + sigma: float = 0.06, +) -> tuple[np.ndarray, np.ndarray]: + """Two log-normal arms of one synthetic cell, the candidate scaled by ``ratio``. + + ``ratio = 1`` is the A/A case: both arms from the same distribution. + """ + b = np.exp(rng.normal(0, sigma, n)) + c = ratio * np.exp(rng.normal(0, sigma, n)) + return b, c + + def _null_cell(rng: np.random.Generator, n: int = 30) -> tuple[float, float]: - """One A/A cell: two arms drawn from the same distribution.""" - b = np.exp(rng.normal(0, 0.06, n)) - c = np.exp(rng.normal(0, 0.06, n)) + """Both tails of one A/A cell.""" + b, c = _pair(rng, n=n) return _wilcoxon_tails(np.log(c) - np.log(b)) @@ -70,7 +85,7 @@ def simulate_improvement_p_clause( Everything is a pure null, so every acceptance is false by construction. """ - rng = np.random.default_rng(SEED_FALSE_IMPROVED) + rng = np.random.default_rng(SEED_P_CLAUSE) old: list[float] = [] new: list[float] = [] for m in cells: @@ -88,7 +103,7 @@ def simulate_improvement_p_clause( def simulate_null_improvement_ci( - *, trials: int = 2000, threshold: float = THRESHOLD + *, trials: int = 2000, threshold: float = DEFAULT_THRESHOLD ) -> int: """Null cells whose CI clause for IMPROVED passes: ``ci_high < 1/threshold``. @@ -101,8 +116,7 @@ def simulate_null_improvement_ci( rng = np.random.default_rng(SEED_NULL_CI) passes = 0 for _ in range(trials): - b = np.exp(rng.normal(0, 0.06, 30)) - c = np.exp(rng.normal(0, 0.06, 30)) + b, c = _pair(rng) if stats.compare(b, c, seed=2, n_resamples=999).ci_high < 1 / threshold: passes += 1 return passes @@ -142,13 +156,13 @@ def _stalled( def _stall_rejection( rng: np.random.Generator, - stall_p: float, + baseline_p: float, + candidate_p: float, *, - both_arms: bool, n: int, trials: int, ) -> tuple[float, float, float]: - """Rejection rate of each tail, and the median ratio, at one stall rate. + """Rejection rate of each tail, and the median ratio, at one pair of stall rates. Returns ``(slower_rate, faster_rate, median_ratio)``. """ @@ -156,8 +170,8 @@ def _stall_rejection( pl: list[float] = [] meds: list[float] = [] for _ in range(trials): - b = _stalled(rng, n, 1.0, 0.08, stall_p if both_arms else 0.0) - c = _stalled(rng, n, 1.0, 0.08, stall_p) + b = _stalled(rng, n, 1.0, 0.08, baseline_p) + c = _stalled(rng, n, 1.0, 0.08, candidate_p) d = np.log(c) - np.log(b) g, f = _wilcoxon_tails(d) pg.append(g) @@ -172,42 +186,48 @@ def _stall_rejection( def simulate_stall_tails( stall_ps: tuple[float, ...], *, n: int = 30, trials: int = 1500 -) -> tuple[tuple[float, ...], tuple[float, ...], tuple[float, ...], tuple[float, ...]]: - """Tail rejection rates and median ratio, under one-sided and two-sided stalls. +) -> tuple[tuple[float, ...], tuple[float, ...], tuple[float, ...]]: + """Tail rejection rates and median ratio with only the candidate arm stalling. + + This is *not* a null experiment, and the numbers are not test size: stalls + on one arm shift that arm's distribution, so the true median ratio leaves 1 + and the rejections are power against a small real effect. It is the + asymmetry that the figure is about -- the two tails stop matching each + other while the median barely moves. + + :func:`simulate_stall_size` is the companion null, and reading the two + together is what separates the two explanations. Returns + ``(slower_rate, faster_rate, median_ratio)``. + """ + rng = np.random.default_rng(SEED_STALLS) + runs = [_stall_rejection(rng, 0.0, sp, n=n, trials=trials) for sp in stall_ps] + slower, faster, medians = zip(*runs, strict=True) + return slower, faster, medians - Stalling *one* arm is not a null experiment: it shifts the candidate's - distribution, so the true median ratio leaves 1 and the rejections it - produces are power against a small real effect, not test size. Stalling - *both* arms at the same rate keeps the true ratio at 1 while contaminating - just as heavily, and that is the run that measures size. - Reading them together is what separates the two explanations. Returns - ``(slower_one_arm, faster_one_arm, slower_both_arms, median_ratio)``, the - median ratio being the one from the one-arm run. +def simulate_stall_size( + stall_ps: tuple[float, ...], *, n: int = 30, trials: int = 1500 +) -> tuple[float, ...]: + """Slower-tail size with *both* arms stalling at the same rate. - Each run gets its own generator, seeded off :data:`SEED_STALLS`, so adding - or dropping a comparison here cannot shift the others' numbers. + Contamination just as heavy as in :func:`simulate_stall_tails`, but + symmetric, so the true ratio stays 1 and this genuinely is the null. The + rate holds near alpha throughout, which is what says the inflation next + door is asymmetry rather than signed-rank failing. + + Its own generator, seeded off :data:`SEED_STALLS`, so adding or dropping + this control cannot shift the numbers in the other run. """ - one_arm = np.random.default_rng(SEED_STALLS) - both = np.random.default_rng(SEED_STALLS + 1) - slower: list[float] = [] - faster: list[float] = [] - slower_both: list[float] = [] - medians: list[float] = [] - for sp in stall_ps: - g, f, med = _stall_rejection(one_arm, sp, both_arms=False, n=n, trials=trials) - gb, _, _ = _stall_rejection(both, sp, both_arms=True, n=n, trials=trials) - slower.append(g) - faster.append(f) - slower_both.append(gb) - medians.append(med) - return tuple(slower), tuple(faster), tuple(slower_both), tuple(medians) + rng = np.random.default_rng(SEED_STALLS + 1) + return tuple( + _stall_rejection(rng, sp, sp, n=n, trials=trials)[0] for sp in stall_ps + ) def simulate_binding_clause( ratios: tuple[float, ...] = (1.0, 1.10, 1.18, 1.30), *, - threshold: float = THRESHOLD, + threshold: float = DEFAULT_THRESHOLD, trials: int = TRIALS, ) -> int: """Count cells where the CI clause passes but the raw p-clause does not. @@ -223,9 +243,7 @@ def simulate_binding_clause( disagreements = 0 for r in ratios: for _ in range(trials): - b = np.exp(rng.normal(0, 0.06, 30)) - c = r * np.exp(rng.normal(0, 0.06, 30)) - cmp_ = stats.compare(b, c, seed=2, n_resamples=999) + cmp_ = stats.compare(*_pair(rng, r), seed=2, n_resamples=999) if cmp_.ci_low > threshold and not cmp_.p_value < ALPHA: disagreements += 1 return disagreements @@ -269,8 +287,7 @@ def simulate_example_cells() -> tuple[tuple[float, float, float], ...]: rng = np.random.default_rng(SEED_CELLS) out: list[tuple[float, float, float]] = [] for true_ratio in (1.31, 1.08, 1.00, 0.81): - b = np.exp(rng.normal(0, 0.05, 40)) - c = true_ratio * np.exp(rng.normal(0, 0.05, 40)) + b, c = _pair(rng, true_ratio, n=40, sigma=0.05) cmp_ = stats.compare(b, c, seed=4, n_resamples=4999) out.append( (round(cmp_.ratio, 3), round(cmp_.ci_low, 3), round(cmp_.ci_high, 3))