diff --git a/docs/design/README.md b/docs/design/README.md index 7431ef9..1ba929c 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -11,7 +11,7 @@ designed around it (see the repository README for the philosophy). | [`runner-schema.md`](runner-schema.md) | JSON schemas for the runner's contract: work order, environment policy, environment identity, identity policy. Aligned with `runner.md` and `benchmark-environments.md` | draft | | [`harness-adapter.md`](harness-adapter.md) | Translating native harness output into results: the context document, mapping rules, driving half, context collectors, adapter catalog | draft | | [`benchmark-environments.md`](benchmark-environments.md) | Who prepares the environment, what the runner controls on its launched process, and what benchx records, with recommendations per kind of environment | draft | -| [`comparator.md`](comparator.md) | Verdicts in history mode and run mode: the eligibility guard, dispersion-scaled verdicts, the comparison document | draft | +| [`comparator.md`](comparator.md) | Verdicts in history mode and run mode: selectors, evidence and methods, the eligibility guard, the comparison document | draft | | [`prototype-scope.md`](prototype-scope.md) | What the minimal implementation demonstrates and the decisions locked for it | agreed | | [`prototype-design.md`](prototype-design.md) | How the implementation is built: one CLI over a Python API, a Parquet store, three JSON documents | draft | diff --git a/docs/design/benchmark-result-schema.md b/docs/design/benchmark-result-schema.md index 9c019db..eef5872 100644 --- a/docs/design/benchmark-result-schema.md +++ b/docs/design/benchmark-result-schema.md @@ -502,7 +502,7 @@ Results carry facts; a **profile** is a comparator-time check that a set of resu | `environments` ([UC-04](../use-cases/UC-04-cross-env.md)) | one pinned component, subject configuration, or `environment.system` field, named by `labels` | subject revision, workload variant, host environment, protocol | exactly two label values whose reported coordinates differ; equal attempt counts per label; sides alternate by `procedure.slot` | | `cross-machine` (template example, pending a use case) | environment identity | subject tree id, workload variant, subject configuration | each side references an anchor result from the same run via `provenance.references` | -**Sides are chosen at comparison time.** The coordinate a profile varies is an argument to the comparator, not a declaration by the producer. Workload parameters are always recorded, so any parameter or set of parameters can be the varying one: `stable` for a sort-algorithm study, `fast_path` for a fast-path deficit, `{role, size}` for a two-factor design. Each distinct assignment of the chosen parameters is one side, and every other coordinate must agree. A comparison has two sides (`comparator.md` §1), so the chosen parameters must take exactly two joint assignments in the run. `role` is only the conventional name that UC-02's `--baseline`/`--variant` flags write; nothing in the schema privileges it. A comparison that was not planned when the run was made is therefore the same operation as one that was, provided the run contains the sides; what a run did not measure, the comparator cannot invent, and a planned run is what guarantees the interleaving invariant. +**Sides are chosen at comparison time.** The coordinate a profile varies is an argument to the comparator, not a declaration by the producer. Workload parameters are always recorded, so any parameter or set of parameters can be the varying one: `stable` for a sort-algorithm study, `fast_path` for a fast-path deficit. Each distinct assignment of the chosen parameters is one side, and every other coordinate must agree. A comparison has exactly two sides (invariant `two-sides`), so the chosen parameters must take exactly two joint assignments in the run. A two-factor design is therefore compared one factor at a time: varying `role` makes each `size` its own unit, while varying `{role, size}` together gives four sides, which that invariant refuses. `role` is only the conventional name that UC-02's `--baseline`/`--variant` flags write; nothing in the schema privileges it. A comparison that was not planned when the run was made is therefore the same operation as one that was, provided the run contains the sides; what a run did not measure, the comparator cannot invent, and a planned run is what guarantees the interleaving invariant. A comparison in which any side is not `clean` is **local-only** under the profiles that allow it (`single-run`, `variants`, `revisions`, `environments`): the comparator produces it, labels it so, names the sides that are not clean, and never promotes it into tracked history. The tree id is what makes each side internally consistent even though neither may have a commit. Under `revisions` this is the comparison of an uncommitted change against HEAD, or against another revision, that the one-off and Arrow local stories want (UC-03 §10). What `revisions` still requires is that every side can be identified: a side whose dirty state is `unknown` has no tree id (§4.1), so it cannot be told apart from the other side or checked for a change during the run, and it fails the invariant that each side has exactly one tree id, so its units get no verdict. diff --git a/docs/design/comparator.md b/docs/design/comparator.md index 9da057c..2e75ef7 100644 --- a/docs/design/comparator.md +++ b/docs/design/comparator.md @@ -10,7 +10,7 @@ estimate one result contributes to it (§4.1). ## 1. Role The comparator answers "did performance change?" in two modes that share one -comparison method and one output document: +framework and one output document: - **History mode** evaluates an explicit target series point against a reference formed from preceding points in the same series. The target is @@ -21,24 +21,34 @@ comparison method and one output document: variants (UC-02), or two environments (UC-04). The baseline side plays the role of the reference. -In both modes the comparator computes the effect and applies a configured -comparison method to classify the target as improved, regressed, no change -detected, or indeterminate. Every verdict carries its evidence, and insufficient -evidence is reported rather than ignored. +A mode is a selector, not a code path: it decides which results form the +evidence, and a method decides what the evidence shows. A method classifies the +target as improved, regressed, no change detected, or indeterminate, and may +claim equivalence. Every verdict carries its evidence, and insufficient evidence +is reported rather than ignored. -The comparator is a side-effect-free evaluator and runs on demand. A selector -chooses the target and reference inputs. The change detector decides when and -which series to evaluate continuously, while policy decides whether a verdict -should block or alert. +The comparator is a side-effect-free evaluator and runs on demand. The change +detector decides when and which series to evaluate continuously, while policy +decides whether a verdict should block or alert. + +**Implemented today:** run mode (`run-profile`), and the framework it runs on. +History mode and detectors are specified here and exercised by test-only +prototypes, whose tests follow in a separate change; they have no CLI and no +configuration. ## 2. Input -The logical core contract is: +Evaluation is a pipeline with one fixed schema in the middle: ```text -evaluate(target, reference, comparison method) -> comparison document +Selector -> Evidence -> Method -> Assessment -> comparison document +Series -> Detector -> Findings (a boundary and an Assessment) ``` +Selectors and methods are modular, versioned code. `Evidence` and `Assessment` +are fixed, so a new method needs no new selector, a new selector needs no new +method, and neither changes the comparison document. + ### 2.1 Store boundary All inputs come from the result store; result documents are ingested before @@ -60,37 +70,48 @@ results into one series, and the join appears as a boundary in the evidence (§2.2). Without such a mapping, the new machine starts a new series and the comparator has no reference to evaluate against. -### 2.2 History mode +### 2.2 History mode: the `preceding-window` selector The target and reference points belong to one series under one identity-policy schema and use one estimator. The target is explicit so a comparison remains -stable when newer results arrive. Reference points precede the target and are -selected by a versioned method, for example from a bounded ancestral window. +stable when newer results arrive. The selector's parameters are the target and +the window; its evidence has two sides, `reference` and `target`, plus the +ordered reference and the boundaries that shaped it (§2.6). A source with a revision graph uses the target's ancestors, not an arbitrary linearization of other branches. When ancestry is unavailable, the store may supply a native linear order. Without either, the reference cannot be formed and the verdict is indeterminate. -Reference selection honors recorded distribution resets, accepted change -points, and applicable continuity events, so measurements from an obsolete -performance regime do not inflate current noise. Every boundary that truncates -or joins history is included in the comparison evidence. +The window stops at the most recent distribution reset or accepted change point +before the target, so measurements from an obsolete performance regime do not +inflate current noise, and it crosses a continuity join. Every boundary that +truncates or joins history is included in the evidence. + +**Anchored history** uses the same selector with sides named by a label, such as +`subject` and `anchor`, where the anchor is a fixed release build measured every +night. Under the default identity policy the anchor and the subject are one +series, because the revision is the axis and nothing else differs: the selector +must tell them apart by the label, or anchor measurements enter the subject's +reference as if they were history (§8.3). -### 2.3 Run mode +### 2.3 Run mode: the `run-profile` selector A run-mode request names a `run_key`, a profile, and the coordinate the profile varies, for example the subject tree id for `revisions` or `parameters.role` for `variants`, and which value is the baseline. Sides are chosen at comparison time -(schema §5.5): each distinct value of the varying coordinate is one side, and -every other coordinate must agree. Results that agree on everything else form -one comparison unit, typically one workload variant, and each unit is evaluated -independently. +(schema §5.5): each distinct value of the varying coordinate is one side. -Within a unit, the two sides' attempts are paired by `procedure.round`, so the -evidence is a set of paired differences measured close together in time rather -than two independent histories. A result that belongs to a series still carries -its series point, but run mode reads no history outside the run. +**Units are series.** Results are grouped by their series fingerprint under the +profile's projection, whose axis is the varying coordinate, so everything else +the projection matches must agree for two results to share a unit. Each unit is +evaluated independently. Run mode reads no history outside the run. + +Sides that disagree on a matched fact therefore fall into separate series. That +is the same shape as a workload measured on one side only, which is merely +`missing`, so the selector tells them apart: several series with one workload +variant and quantity are a failed `sides-agree`, naming the facts that differ. A +disagreement fails the whole run; a variant on one side only does not. ### 2.4 Batch requests @@ -99,117 +120,264 @@ that target. It evaluates each target independently and reports an expected series with no target result as missing; it never substitutes that series' most recent result from an older run. +### 2.5 The `explicit` selector + +The caller lists the points. They are still validated to lie in one series, so +this selector cannot compare what a projection keeps apart. It covers one side +against one side across runs, such as a release against the previous release. +One point per side has no spread to estimate noise from, so a method needs +calibration history (§2.6) or a declared provisional rule. + +### 2.6 Evidence + +Evidence is a flat table, not two buckets. Each row is one attempt's estimate +with its observations attached, and each column is read by a `Key`: a JSON +Pointer, an optional normalization rule, and an ordering, serializable to JSON. + +| Column | Key | Typical relation to the side | +|---|---|---| +| `side` | the profile's axis, or the selector's own labels | | +| `round` | `/procedure/round` | crossed: a block | +| `slot` | `/procedure/slot` | nested: unique to one attempt | +| `run_key` | `/provenance/run_key` | crossed in anchored history; constant in a run | +| `attempt_key` | `/attempt_key` | nested: a replicate | +| `started_at` | `/provenance/started_at` | nested | +| `revision` | `/revision/key` | crossed in anchored history if a build is shared; otherwise nested | + +Rows are grouped into units, one per series. The framework derives a few facts +by counting, and gives no treatment special meaning beyond them: + +- **Crossed or nested.** A key is *crossed* with the side when some value of it + appears on more than one side, which makes it a block. It is *nested* when + every value appears on one side only, which makes it a replicate. It is + *absent* when no row has it. Failed attempts count: the design of the run is + what is being read, not which attempts happened to succeed. +- **Complete.** A crossed key is complete when every value appears on every side. +- **Alternation.** Whether the sides alternate by `procedure.slot`. This is judged + over the whole run, not within a unit: a slot is a position in the run's + realized order, and a unit that skipped one attempt has gaps that say nothing + about the schedule. + +History evidence adds the target, the reference in order oldest first, the +ordering used, and the boundaries. It may add **calibration history**: past +comparisons in the series, from which a method can set its threshold, as +rustc-perf does. + ## 3. Eligibility guard -Before computing a verdict, the comparator checks, in both modes, that: - -- the target has a usable estimate — otherwise that target is indeterminate, not - replaced by an older successful point; -- every reference point or baseline result satisfies the configured status and - quality rules; -- target and reference use the same estimator and identity-policy schema; and -- the evidence is enough for the configured method, or the method declares a - provisional rule that applies. - -In run mode the status rule belongs to the profile. Under `variants` it is -strict (UC-02 §10): a unit with an attempt of status `partial`, `censored` or -`error`, on either side, is indeterminate with reason `side-failed`, whatever -the other rounds show. Otherwise a variant that times out in the slow rounds is -judged on the fast ones. The other profiles drop the failed attempt's pair and -keep the verdict from the rest, and a `skipped` attempt is dropped in every -profile. The schema's §5.5 table is not touched, since this is a status rule, -not an invariant. - -In history mode it additionally checks that: - -- the target and every reference point have the same series fingerprint; -- every reference point precedes the target under the selected ancestry or - native order; and -- no revision contributes more than one eligible point unless configuration - selects one, combines them by a declared method, or evaluates them separately. - Otherwise the affected comparison is indeterminate with reason - `duplicate-points`; the comparator never picks one implicitly. - -In run mode it additionally checks the profile's invariants over the run (schema -§5.5): the two sides differ only in the varying coordinate, attempt counts per -side are equal, and sides alternate by `procedure.slot`; under `revisions`, -that each side has exactly one tree id and the two differ; under `revisions` -and `variants`, that a separately recorded benchmark has one `benchmark_tree` -across both sides and all rounds (`one-benchmark-tree`, schema §5.5). A side that is not clean does -not fail the run; it makes the comparison local-only (UC-03 §10). A run that -fails an invariant is not compared: the comparison document records the failed invariant -and contains no verdicts for the units it affects. - -Excluded points and their reasons remain visible in the output. Changes in -facts the projection ignores do not split a series by themselves, but differences between -the target and its reference are carried as caveats so that a kernel, load, or -other recorded condition is not hidden from the reader. +Checks belong to whoever needs them to be true. A failure is recorded in +`failed_invariants`, and the affected units get no verdict. + +**The framework** checks that a result has a usable estimate. Anything else is +excluded with its reason, and a target without one is indeterminate, never +replaced by an older successful point. Excluded points and their reasons remain +visible in the output. + +**The selector or profile** checks what makes the sides meaningful, and a failure +stops the whole run: + +| Invariant | Profile | Meaning | +|---|---|---| +| `sides` | all | every result can be named by the varying coordinate | +| `two-sides` | all | exactly two sides | +| `sides-agree` | all | one workload variant and quantity is one series (§2.3) | +| `one-tree-per-side` | `revisions` | each side has one tree id, and the two differ | +| `one-benchmark-tree` | `revisions`, `variants` | a separately recorded benchmark is one file on both sides and in every round (schema §5.5) | +| `sides-differ` | `environments` | the sides differ in more than their label | + +A side that is not clean does not fail the run; it makes the comparison +local-only (UC-03 §10). In history mode the selector checks that the target and +every reference point have the same series fingerprint and that every reference +point precedes the target under the selected ancestry or native order. + +The selector also owns the **status rule**, which is not an invariant: it leaves +one unit without a verdict and fails nothing else. Under `variants` it is strict +(UC-02 §10): a unit with an attempt of status `partial`, `censored` or `error`, on +either side, is indeterminate with reason `side-failed`, whatever the other rounds +show; otherwise a variant that times out in the slow rounds is judged on the fast +ones. The selector records this in the evidence (`no_verdict`), every method must +return `indeterminate` with that reason, and the framework refuses a method that +does not, so the rule holds whichever method a project configures. The other +profiles leave the remaining attempts to the method, and a `skipped` attempt is +only excluded under every profile. The schema's §5.5 table is not touched. + +**The method** declares what its statistics assume, and the framework checks +what it declared: + +| Requirement | Scope | Meaning | +|---|---|---| +| `alternating-slots` | run | the schedule alternated between the sides | +| `equal-attempts` | unit | as many attempts on each side | +| `duplicate-points` | unit | no cell of the method's layout holds two usable results; the comparator never picks one implicitly | + +A method that does not require alternation may still evaluate a run that did not +alternate. The document records the schedule, and the unit carries the caveat +`sides-did-not-alternate`. + +`duplicate-points` as a requirement sees one cell of the layout. A revision that +contributes two points to a history reference is a different case, since a +`("side",)` cell holds many results by design and `revision` is nested in the side, +so it cannot be a dimension. A history method reads the `revision` column and +returns `indeterminate` with reason `duplicate-points` unless it declares how it +combines them; the comparator never picks one implicitly. + +**The layout.** A method declares the dimensions it needs (§4.1). When a unit +cannot be laid out that way, because a dimension is absent or nested in the side, +the method does not apply: the failure is recorded as `layout`, with the reason. +Within a layout the method may still return `indeterminate` with its own reason. + +Changes in facts the projection ignores do not split a series by themselves, but +differences between the target and its reference are carried as caveats so that a +kernel, load, or other recorded condition is not hidden from the reader. An intentional comparison across an identity difference outside a single run requires the series to be materialized under an explicit projection that projects that difference out (schema §4.3). The comparator never bypasses the resulting series boundary. -## 4. Comparison method and verdict - -A comparison method has a stable name and version. It defines the reference -window or pairing, the estimator, effect measure, treatment of within-result -precision and between-point variation, minimum evidence, and decision threshold. - -For the target, the method produces: - -- **Reference estimate**: the expected value derived from the preceding eligible - points in history mode, or from the baseline side in run mode. -- **Effect**: a declared measure of change from the reference estimate. Relative - change is the default only when the reference is nonzero; the method must - define an absolute, ratio, logarithmic, or other measure and its behavior for - zero or negative values. -- **Noise**: variation expressed on the same scale as the effect. In history - mode it is estimated from the reference points; in run mode from the spread of - the paired differences across rounds. Observation-level precision within one - result and variation between points or rounds remain distinct evidence; a - method may combine them only through a declared rule. When there is not enough - evidence to estimate noise, the default verdict is indeterminate with reason - `insufficient-history`. A method may instead declare a provisional rule, such - as a project-supplied noise level for new series; how such a rule is chosen is - left to the method and project, and the output identifies the source of the - estimate and whether it is provisional. -- **Verdict**: the initial method indicates a change when |effect| exceeds *k* × - noise, with *k* a project-configured multiplier. Indicated changes are labeled - improved or regressed using the quantity's direction metadata when present, - otherwise just *changed*. When the evidence is sufficient but the threshold - is not exceeded, the verdict is *no change detected*. -- **Indeterminate**: there is no usable target or reference, ordering is unknown, - or the method lacks enough evidence and no provisional rule applies. The - reason is part of the verdict. - -A quantity declared deterministic (§4.4 of the schema) does not need a noise -estimate. Once it has an eligible reference, exact equality means no change -detected and any difference indicates a change. - -The dispersion-scaled initial method is deliberately simple and respects each -series' observed variability — a 2% change on a 0.2%-noise series is indicated, -while a 2% change on a 5%-noise series is not. Prediction intervals, -significance tests, paired confidence intervals, minimum-effect filters, or -multiple-comparison control can be added as new method versions without -changing the surrounding contract. +## 4. Methods and verdicts + +### 4.1 What a method owns + +A method has a stable name and version, and owns all the statistics: the effect +measure (difference, relative, log ratio, ratio of means), the noise model, the +decision rule, its minimum evidence, and the schemas of its configuration and +its details. + +A method declares its dimensions as a plain tuple, like a generalized ufunc's +core dimensions, and the requirements of §3: + +```python +class Method: + name = "benchx/paired-relative" + version = "v1" + dims = ("round", "side") + requires = ("equal-attempts", "alternating-slots", "duplicate-points") + + def assess(self, layout, evidence, config) -> Assessment: ... +``` + +The framework pivots each unit into that shape and loops over units. A cell holds +the usable rows at those coordinates, and a cell with none is masked. So +`("round", "side")` is a matrix of paired attempts, and `("side",)` is the +attempts of each side, of unequal length if they differ. + +The estimator is not the method's: it belongs to the series (§4.3 of the schema), +which is what the selector grouped by. + +### 4.2 The Assessment + +Every method returns the same record: + +- **verdict**: `improved`, `regressed`, `changed` (a change with no known + direction), `no-change-detected`, `equivalent`, or `indeterminate`; +- **reason**, required when indeterminate, such as `insufficient-rounds`; +- **effect**, optional: the headline change, on a measure the method declares; +- **used** and **excluded**: every result of the unit is in exactly one of them, + and each exclusion has a reason; +- **details**: anything else, validated by the method's own schema. + +An Assessment is deterministic given its inputs, the method version, and the +configuration. + +`no-change-detected` and `equivalent` are different claims. The first says a test +did not find a change, which low power also produces. The second is what an +equivalence test claims: any change is smaller than a declared margin. No current +method returns `equivalent`; the verdict is in the schema so that one can be added +without changing it. + +### 4.3 Noise is defined by the method + +"Noise" has no meaning of its own, so each method records what it computed in +`details`. The choice can decide the verdict. For a single new point against a +reference, the relevant spread is that of the points, because the target is one +more draw. The standard error of the reference shrinks with the window and answers +a different question. On the data of §8.2 the first gives a threshold of 10.1% +and the second 2.9%, so they disagree about a 9% change. + +A fixed multiplier also means different confidence at different sample sizes. With +*k* = 3 on the standard error of *n* paired differences, the two-sided confidence +under a t distribution is about 80% at 2 rounds, 90% at 3, 96% at 5, and 99% at 20. +A method therefore records its degrees of freedom or uses quantiles. + +### 4.4 `benchx/paired-relative/v1` + +Requires alternating slots, equal attempts, and no duplicate points. Per round it +computes d = contender / baseline − 1 from each side's estimate. + +- **Effect:** the mean of d, measure `relative`. +- **Noise:** the standard error of d across rounds, recorded with `noise_kind: + standard-error` and its `degrees_of_freedom`. +- **Decision:** a change when |effect| > *k* × noise, labeled by the quantity's + direction when known, otherwise `changed`. +- **Indeterminate:** `insufficient-rounds` below `min_rounds` pairs, and never with + fewer than two since the noise needs a spread; `zero-baseline` when a baseline + estimate is zero; `deterministic-deferred` for a quantity declared deterministic. + +Exact equality would be the natural rule for a deterministic quantity (schema +§4.4), which needs no noise estimate. No quantity of the implementation is +deterministic, so that is deferred. + +### 4.5 `benchx/mann-whitney/v1` (prototype) + +A prototype in `tests/mann_whitney_prototype.py`, not in the package: it shows that +a second method is one module, which the framework runs unchanged. It requires nothing of the run's schedule or pairing: `dims = ("side",)`. It tests +whether one side's attempt estimates tend to exceed the other's, and is the +unpaired method for a run that did not alternate. + +- **Effect:** the relative difference of the sides' medians, measure + `relative-median`, absent when the baseline median is zero. +- **Decision:** a change when the two-sided p-value is at most `alpha`, labeled by + the direction of the rank shift. The p-value is exact, with ties handled by + permuting midranks; a method for the package would add a normal approximation + for large samples. +- **Indeterminate:** `insufficient-attempts` when the design cannot reach `alpha` + at all. The smallest two-sided p-value is 2 / C(n + m, n), which is 0.1 for 3 + attempts per side and 0.029 for 4. A run too small to detect anything does not + get to report that it detected nothing. + +It ignores the blocking in a paired run, so on the same run it is usually the less +sensitive of the two (§8.1). That is the cost of asking less of the run. + +### 4.6 Change detectors + +A detector is a sibling of a method, sharing the Assessment and not the pipeline: + +```text +Detector.scan(series) -> list[Finding] +Finding = {boundary, assessment} +``` + +A series is the ordered points of §2.2. A finding locates a change: its `boundary` +is a `change-point` at the first result after it, and its assessment says what was +found there, in the same vocabulary as a method's. An accepted finding becomes a +`change-point` boundary, which ends the reference of later evaluations (§2.2), so +a regime change is reported once and then stops inflating noise. Acceptance is +review or policy, not part of the detector. ## 5. Output -A **comparison document** is machine-readable and self-describing. For each -evaluation it records: - -- the mode, and the target and exact reference inputs: reference points in - history mode; the `run_key`, profile, varying coordinate, sides, and pairing - in run mode; -- the series fingerprint, identity-policy schema, estimator, and ordering or - ancestry used; -- the reference window, reset or continuity boundaries, comparison method and - version, and complete configuration; -- the reference estimate, effect measure and value, noise and its source, - threshold or limits, evidence counts, and verdict with reason; and -- exclusions, failed profile invariants, unmatched differences, quality - caveats, and missing target results from a batch request. +A **comparison document** (`schemas/comparison-document/0.1.0`) is machine-readable +and self-describing. It records: + +- the `selector` as `{name, version, params}`, and the `method` requested with its + complete configuration, present even when a failed invariant left no units; +- `sides`, for a selector that has named ones: the varying coordinate, the two + values, and what each side declared about how it was built; +- the identity-policy schema, the estimator, and `local_only`; +- `units`, and for each: the workload, quantity, and series fingerprint; in history + mode an `evidence` record; the outcome; and the caveats; +- exclusions, failed invariants, missing targets, and every input. + +A history unit's **evidence** records what the verdict rests on: the target, the +reference in order, the ordering used, and the boundaries that truncated or joined +it. Run mode needs none, since its inputs are the run. + +A unit's **outcome** is the Assessment: `method`, `verdict`, `reason`, `effect` as +`{measure, value}`, `used`, `excluded`, and `details`. The framework validates +`details` against the method's schema when it produces the document. The document +schema itself treats `details` as an open object, since a new method must not +change it, so `bx validate` does not check them. The document is renderable as a reviewer-speed summary ("these target results regressed, these improved, these show no detected change, these were @@ -217,17 +385,16 @@ indeterminate, and these were missing") and consumable by alerting or CI gates. Comparison documents are derived and recomputed on demand; the result store persists their immutable inputs rather than the documents themselves. Supplying -the recorded inputs, method version, and configuration reproduces the document -even after newer points arrive. +the recorded inputs, selector, method version, and configuration reproduces the +document even after newer points arrive. ## 6. Boundaries - **Not target selection:** selecting "latest", a target run, or a target revision happens before invoking the comparator. -- **Not an arbitrary cross-series pair engine:** history mode evaluates a target - against preceding points in the same series; run mode compares only sides - that a profile defines within one run. -- **Not policy:** *k*, provisional rules, eligibility rules, and gating +- **Not an arbitrary cross-series pair engine:** every selector stays within one + series, or within the sides a profile defines in one run. +- **Not policy:** which method a project uses, its configuration, and gating decisions are project configuration; the comparator reports, humans and policy decide. - **Not the detector:** it consumes a fixed historical reference but does not @@ -238,10 +405,69 @@ even after newer points arrive. ## 7. Open questions -1. **Run-mode default method.** UC-03's sketch reports a ratio with a 95% - confidence interval over paired rounds. Should that be the initial run-mode - method, with the dispersion-scaled rule kept for history mode, or should - both modes start from the same *k* × noise rule? -2. **Unplanned run-mode comparisons.** A run made without alternating sides - fails the profile's interleaving invariant. Should the comparator refuse it, - or allow an explicitly unpaired method whose output is marked as such? +1. **Which method does a project configure?** Earlier drafts asked which single + method run mode and history mode should start from, and whether an unplanned + run that did not alternate should be refused. Both are now this question. + Methods can coexist, so the choice is per project: `bx compare` uses + `paired-relative`, and the `mann-whitney` prototype (§4.5) shows an unpaired + method for an unplanned run. UC-03's sketch reports a ratio + with a 95% confidence interval over paired rounds, which would be a third method. + UC-02 asks for each side's minimum against a fixed margin (§7 of the use case), + which would be another; `variants` uses `paired-relative` until one is written. +2. **Multiple comparisons across units.** The framework calls a method once per + unit, so no method can see the other units' results, and multiple-comparison + control cannot be a method. It would be a step over the Assessments of a whole + document. The need is real at the defaults: *k* = 3 at 3 rounds is about 90% + confidence per unit (§4.3), so a suite of 100 unchanged workloads would be + expected to report about 10 changes. + +## 8. Worked examples + +One series throughout: NumPy `sort`, float64, 10⁶ elements, wall time. **The values +are constructed to show the mechanism, not measured.** Every number below was +computed from these values by test code that returns in a follow-up change. +All three use *k* = 3. + +### 8.1 Run mode: paired by round + +Ten alternating rounds of a contender about 2% slower. Machine state moves both +sides of a round together, so the baseline spreads by 5.4% across rounds while the +ratio within a round spreads by only 0.7%. + +| Treatment | Effect | Standard error | Threshold | Finding | +|---|---|---|---|---| +| Paired by round, `dims=("round", "side")` | +1.99% | 0.22% | 0.65% | regressed | +| Two buckets, `dims=("side",)` | +1.97% | 2.33% | 7.00% | no change detected | + +Pairing cuts the standard error about 11× here, whichever effect measure is used: +10.8× for relative change and ratio of means, 10.9× for log ratio, 11.8× for +difference. The size of the gain comes from the data, not from pairing as such. +With between-round spread σ_b shared by both sides and within-pair spread σ_w, it +is about √(2σ_b² + σ_w²) / σ_w: 10× here, 1.7× when the two spreads are equal, and +1× when rounds share nothing. + +### 8.2 History mode: the latest point against 12 ancestors + +Twelve nights spread by 3.4% around a median of 41.0 ms, and the latest point is +9% above it. The spread between nights sets the threshold at 10.1%, so the method +reports `no-change-detected`. A real regression is hidden because every point +carries a night's worth of machine state, not because the method is wrong. + +Using the standard error of the reference instead would detect it, with a threshold +of 2.9%, but 6 of these 12 ordinary nights are that far from the median too: a single +new point varies by the spread of the points, however many were averaged (§4.3). + +### 8.3 Anchored history: a fixed build measured each night + +Measure a fixed release build beside the subject each night. `run_key` is now +crossed with the build kind, so each night is a block, and the evidence fits +`dims=("run_key", "side")`, the same shape as §8.1. The night's machine state +cancels in the ratio of subject to anchor. + +| | Spread | Threshold | Effect | Finding | +|---|---|---|---|---| +| Subject alone (§8.2) | 3.4% | 10.1% | +9.0% | no change detected | +| Subject / anchor | 0.46% | 1.4% | +9.0% | regressed | + +Nothing about the framework is specific to anchoring. The selector names the sides, +and counting finds that `run_key` is a block. diff --git a/docs/design/prototype-design.md b/docs/design/prototype-design.md index 04e267b..43f3ea6 100644 --- a/docs/design/prototype-design.md +++ b/docs/design/prototype-design.md @@ -241,27 +241,30 @@ criterion 1 holds by construction. ```json { "schema_version": "benchx/comparison-document/0.1.0", - "mode": "run", - "run_key": "urn:uuid:9b2e4f1a-…", - "profile": "environments", - "varying": "provenance.labels.build", - "baseline": "plain", - "contender": "hardened", + "selector": {"name": "benchx/run-profile", "version": "v1", + "params": {"run_key": "urn:uuid:9b2e4f1a-…", "profile": "environments", + "baseline": "plain", "label": "build"}}, "method": {"name": "benchx/paired-relative", "version": "v1", - "estimator": "median", "k": 3.0, "min_rounds": 3}, + "configuration": {"k": 3.0, "min_rounds": 3}}, + "estimator": {"name": "median", "method_version": "benchx/median/v1", "input_level": "observations"}, "identity_schema": "benchx/profile/environments/v1+benchx/benchx-identity/v1", "local_only": true, + "sides": {"varying": "provenance.labels.build", "baseline": "plain", "contender": "hardened"}, "units": [ - {"workload": {"name": "TakeChunked…/4194304/2", "parameters": {}}, + {"workload": {"name": "TakeChunked…/4194304/2"}, "quantity": "wall-time", - "series": {"baseline": "…", "contender": "…"}, - "rounds": 10, - "reference_estimate": 0.0455, "effect": 0.024, - "noise": 0.0021, "noise_source": "paired-rounds", - "verdict": "regressed", "reason": null, + "series": "…", + "method": {"name": "benchx/paired-relative", "version": "v1"}, + "verdict": "regressed", + "effect": {"measure": "relative", "value": 0.024}, + "used": [{"producer": "benchx/gbench-adapter", "ingest_key": "…"}], + "excluded": [], + "details": {"rounds": 10, "reference_estimate": 0.0455, "noise": 0.0021, + "noise_kind": "standard-error", "noise_source": "paired-rounds", + "degrees_of_freedom": 9, "threshold": 0.0063}, "unmatched_differences": ["/coordinates/subject/configuration/options/ARROW_HARDENED"]} ], - "missing": [{"workload": "…", "quantity": "wall-time", "side": "contender"}], + "missing": [{"workload": {"name": "…"}, "quantity": "wall-time", "side": "contender"}], "failed_invariants": [], "exclusions": [], "inputs": [{"producer": "benchx/gbench-adapter", "ingest_key": "…"}] @@ -509,11 +512,12 @@ every fact the policy matches must agree across both sides. | `variants` | the workload parameter `--vary` names, else the only one | that parameter's recorded value, quotes included (`'baseline'`) | everything else the identity policy matches, including subject revision and tree, and every other parameter | A side is one whole axis value, so one label covering two builds is three -sides, not one. Before any verdict, the eligibility guard (§3) checks that the -matched facts agree (`sides-agree`) and the invariants the profile names over -the run: exactly two sides, which for `environments` differ in more than their -label, equal attempt counts per side, and sides alternating by -`procedure.slot`. Results the order did not ask for are not compared. A failed invariant is recorded and the affected units get no +sides, not one. Before any verdict, the eligibility guard (`comparator.md` §3) checks +what the selector owns, that the matched facts agree (`sides-agree`) and that there are +exactly two sides, which for `environments` differ in more than their label; and what +the method asks for, for `paired-relative` equal attempt counts per side, sides +alternating by `procedure.slot`, and no two usable results at one round and side +(`duplicate-points`). Results the order did not ask for are not compared. A failed invariant is recorded and the affected units get no verdict; under `revisions` the same happens to a side with no tree id (the `sides` invariant), to a tree edited during the run (a third side: `two-sides`), and to a benchmark recorded apart from the subject that is not one file on @@ -546,16 +550,18 @@ Per comparison unit (one workload variant and quantity), the method direction when known; otherwise *no change detected*; - is indeterminate with a reason when fewer than `min_rounds` pairs exist, and never with fewer than two, since the noise is a standard error - (`insufficient-rounds`), when a baseline value is zero, when the guard - excluded the unit, or, under `variants`, when an attempt failed (`side-failed`). + (`insufficient-rounds`), when a baseline value is zero, or, under `variants`, + when an attempt failed (`side-failed`, a rule of the selector that every method + follows). A unit whose rounds cannot line the sides up, because it has none or no + round is on both sides, fails the `layout` invariant instead. Deterministic quantities are deferred: none of the implementation's quantities is one, so a unit whose quantity is declared deterministic is indeterminate with reason `deterministic-deferred` rather than compared exactly (comparator §4). Default *k* = 3 and `min_rounds` = 3, overridable per invocation; per-project -configuration is deferred. Choosing a paired method for run mode is the -the implementation's answer to `comparator.md` §7 question 1, open until reviewed. +configuration is deferred. `bx compare` uses this method; which method a project +configures is `comparator.md` §7 question 1, open until reviewed. Comparison documents are recomputed on demand and not stored (`comparator.md` §5); the store keeps their inputs. diff --git a/docs/design/system-decomposition.md b/docs/design/system-decomposition.md index 876deb2..bcdd8c7 100644 --- a/docs/design/system-decomposition.md +++ b/docs/design/system-decomposition.md @@ -348,7 +348,9 @@ Not components of this system: ## 6. Open questions 1. *Resolved:* comparator and change detector are two components sharing a - statistics library, not a shape (see `comparator.md`). + statistics library and one result record, the Assessment, but not a pipeline: + a method sees evidence for one target, a detector scans a whole series (see + `comparator.md` §4.6). 2. *Resolved:* the store records identity; the comparator enforces comparability (see `comparator.md` §3). 3. Is the workbench a distinct deliverable or a thin skin over runner + diff --git a/src/benchx/compare.py b/src/benchx/compare.py index 39b2b98..a49f382 100644 --- a/src/benchx/compare.py +++ b/src/benchx/compare.py @@ -1,440 +1,175 @@ -"""Comparator, run mode only (comparator.md §2.3; prototype-design.md §5). +"""The comparator's framework (comparator.md §2): a selector forms the evidence, a +method assesses it, and this module checks what the method asked for and writes +the comparison document. It holds no statistics. The document depends only on the +inputs, so the same results give the same document from any store. -Two sides of one run, as a profile defines them, paired by round and compared -with the method benchx/paired-relative/v1. A profile is a projection built -here from the identity policy: its axis says what tells the sides apart, the -policy's roles what must match for two results to be compared, and its -invariants what the run must satisfy (schema §5.5). The document depends only -on the input results, so the same results give the same document from any -store. +Only run mode is reachable from the CLI. History mode and detectors are specified +in comparator.md; a selector for them would call `evaluate` as run mode does. """ import json -import math -import re -import statistics -from . import identity -from .core import canonical, catalog -from .projection import Projection, axis_order, get, workload_label -from .snapshot import DIRTY_WARNINGS -from .store import store_project - -METHOD = {"name": "benchx/paired-relative", "version": "v1", "effect_measure": "relative"} -PROFILES = { - # UC-03: two subject trees from one run. - "revisions": Projection(identity.POLICY_DATA, ["/provenance/subject_tree"], - profile="benchx/profile/revisions/v1", - invariants=["one-run", "two-sides", "one-tree-per-side", "equal-attempts", - "alternating-slots", "one-benchmark-tree"]), - # UC-04: one revision measured two ways. A side is what its subject - # configuration, pinned components, and system are; the caller's label, - # put in front of the axis per comparison, only names it. - "environments": Projection(identity.POLICY_DATA, ["/coordinates/subject/configuration", - "/coordinates/subject/components", - "/coordinates/environment/system"], - profile="benchx/profile/environments/v1", - paths={"/revision/key": "match", "/provenance/subject_tree": "match"}, - invariants=["one-run", "two-sides", "sides-differ", "equal-attempts", - "alternating-slots"]), - # UC-02: one revision, one environment, two values of one workload parameter. Everything else - # must agree, so the axis (the parameter) is added per comparison: see `profile_projection`. - "variants": Projection(identity.POLICY_DATA, [], - profile="benchx/profile/variants/v1", - paths={"/revision/key": "match", "/provenance/subject_tree": "match"}, - invariants=["one-run", "two-sides", "equal-attempts", "alternating-slots", - "one-benchmark-tree"]), -} -# Profiles for which a failed attempt on either side fails its unit (UC-02 §10): a variant that -# times out in the slow rounds must not be judged on the fast ones. The other profiles drop the -# attempt's pair and keep the verdict from the rest. -STRICT_STATUS = {"variants"} -FAILED_STATUSES = ("partial", "censored", "error") # `skipped` is not a failure -WORKLOAD = "/coordinates/workload" -PARAMETERS = WORKLOAD + "/parameters/" -_PARAMETER_NAME = re.compile(r"[A-Za-z0-9_-]+") -# The noise is the standard error of the per-round differences, so a verdict needs -# at least two pairs, whatever `min_rounds` a caller asks for. -MIN_PAIRS = 2 - - -class CompareError(Exception): - """The request itself cannot be answered, such as an unknown baseline.""" - - -def _c(value) -> str: - return canonical(value).decode() - - -def profile_projection(profile: str, label: str | None = None, parameter: str | None = None) -> Projection: - """The profile's projection; for environments, with the label that names - the sides at the front of its axis, and for variants, with the workload - parameter whose two values are the sides.""" - if profile not in PROFILES: - raise CompareError(f"profile {profile!r} is not in the implementation ({', '.join(PROFILES)})") - projection = PROFILES[profile] - if profile == "environments": - if not label: - raise CompareError("the environments profile needs the label that names the sides") - projection = projection.with_axis_first(f"/provenance/labels/{label}") - if profile == "variants": - if not parameter: - raise CompareError("the variants profile needs the workload parameter whose values are the sides") - if not _PARAMETER_NAME.fullmatch(parameter): # keeps the document's `varying` a token - raise CompareError(f"workload parameter {parameter!r}: use letters, digits, '_' and '-' only") - projection = projection.with_axis_first(PARAMETERS + parameter) - return projection - - -def _dotted(pointer: str) -> str: - return pointer[1:].replace("/", ".") - - -def _measurand(doc) -> bool: - """Whether the result is what its attempt set out to measure. A condition - measurement (`measurement.kind: condition`, schema §4.4), such as the - runner's load average, is kept beside the measurands and never compared.""" - return doc["measurement"].get("kind", "measurand") == "measurand" - - -def _estimate(doc): - """The attempt's median observation; the method's estimator (METHOD).""" - values = [o["value"] if isinstance(o, dict) else o for o in doc["measurement"].get("observations", [])] - return statistics.median(values) if values else None - - -def _verdict(effect, noise, k, direction): - if abs(effect) <= k * noise: - return "no-change-detected" - if direction == "lower-is-better": - return "regressed" if effect > 0 else "improved" - if direction == "higher-is-better": - return "improved" if effect > 0 else "regressed" - return "changed" - - -def _run_documents(documents, run_key) -> list[dict]: - """The measurands of one run, in a fixed order, so the same results give the same document.""" - found = sorted((d for d in documents if d["provenance"].get("run_key") == run_key and _measurand(d)), - key=lambda d: (d["producer"]["name"], d["ingest_key"])) - if not found: - raise CompareError(f"no results for run {run_key!r}") - return found - - -def infer_parameter(documents, run_key) -> str: - """The workload parameter a variants comparison of this run varies, when the caller names none: - the only parameter the run's results carry. An `-e` variable counts, since the runner records - it as a workload parameter. Read mode only; measure mode asks before running (session).""" - names = sorted({name for d in _run_documents(documents, run_key) - for name in (d["coordinates"]["workload"].get("parameters") or {})}) - if not names: - raise CompareError("nothing to vary: no workload parameter in the results " - "(declare one in `params`/`param_names`)") - if len(names) > 1: - raise CompareError(f"several workload parameters ({', '.join(names)}): give --vary NAME") - return names[0] - - -def compare(documents, *, run_key, profile, baseline, label=None, parameter=None, k=3.0, min_rounds=3) -> dict: - """The run-mode comparison document. `label` names the sides of `environments`, and `parameter` - those of `variants`.""" - projection = profile_projection(profile, label, parameter) - documents = _run_documents(documents, run_key) - naming = projection.axis[0] - varying = _dotted(naming) - doc = {"schema_version": catalog.COMPARISON_DOCUMENT, "mode": "run", "run_key": run_key, "profile": profile, - "varying": varying, "baseline": baseline, "pairing": "procedure.round", - "method": {**METHOD, "configuration": {"k": k, "min_rounds": min_rounds}}, - "estimator": dict(identity.MEDIAN), "identity_schema": projection.identity_schema, - "local_only": any(d["provenance"][f] != "clean" for d in documents - for f in ("subject_dirty", "benchmark_dirty")), - "units": [], "missing": [], "failed_invariants": [], "exclusions": [], - "inputs": [{"producer": d["producer"]["name"], "ingest_key": d["ingest_key"]} for d in documents]} +from jsonschema import Draft202012Validator + +from . import methods +from .core import catalog +from .evidence import SIDE, CompareError, Evidence, Layout, Method, NotApplicable, Selection, pivot +from .projection import workload_label +from .run_profile import PROFILES, RUN_PROFILE, _declared, dotted, infer_parameter, profile_projection # noqa: F401 + + +def compare(documents, *, run_key, profile, baseline, label=None, parameter=None, method=methods.DEFAULT, + **config) -> dict: + """Two sides of one run under a method. `label` names the sides of `environments`, and + `parameter` those of `variants`. Any other keyword is the method's configuration, such + as `k` and `min_rounds` for the default.""" + chosen = methods.get(method) + configuration = _configure(chosen, config) + selection = RUN_PROFILE.select(documents, run_key=run_key, profile=profile, baseline=baseline, label=label, + parameter=parameter) + return evaluate(selection, chosen, configuration) + + +def _configure(method: Method, config: dict) -> dict: + configuration = {**method.defaults, **config} + errors = [e.message for e in Draft202012Validator(method.config_schema).iter_errors(configuration)] + if errors: + raise CompareError(f"{method.id}: {'; '.join(errors)}") + return configuration + + +def evaluate(selection: Selection, method: Method, configuration: dict) -> dict: + """The comparison document for what a selector selected, under a method.""" + doc = {"schema_version": catalog.COMPARISON_DOCUMENT, "selector": selection.selector, + "method": {"name": method.name, "version": method.version, "configuration": configuration}, + "estimator": selection.estimator, "identity_schema": selection.identity_schema, + "local_only": selection.local_only, + **({"sides": selection.sides} if selection.sides else {}), + "units": [], "missing": [], "failed_invariants": list(selection.failed), "exclusions": [], + "inputs": selection.inputs} failed = doc["failed_invariants"] - projected = [projection.apply(d, store_project(d)) for d in documents] - if any(p.axis[0] is None for p in projected): - if profile == "revisions": - # UC-03 §10: a side with no working-tree id (its dirty state is `unknown`) cannot be named. - detail = (f"a result has no {varying}: its tree's dirty state is unknown, so the side cannot be told " - "apart from the other or checked for a change during the run. Make every file in the " - "tree readable, or git-ignore it") - elif profile == "variants": - detail = (f"a result has no workload parameter {parameter}: declare it in the benchmark's " - "`params`/`param_names`") - else: - detail = f"a result has no {varying}: give every bx bench call --label NAME=VALUE, and name that label here" - failed.append({"invariant": "sides", "detail": detail}) - return doc - - # A side is a distinct axis value, named by its first part (a tree, a label). - sides = [_c(p.axis) for p in projected] - names = {side: p.axis[0] for side, p in zip(sides, projected)} - if profile == "revisions": - matches = {side for side, d in zip(sides, documents) - if names[side].startswith(baseline) or d["revision"]["key"].startswith(baseline)} - else: - matches = {side for side in names if names[side] == baseline} - if len({_c(names[side]) for side in matches}) != 1: - raise CompareError(f"baseline {baseline!r} matches {len(matches)} sides of {sorted(map(str, names.values()))}") - base = sorted(matches)[0] # several sides of one name fail two-sides below - doc["baseline"] = names[base] - if len(names) != 2: - failed.append({"invariant": "two-sides", - "detail": f"{len(names)} sides: {sorted(_c(n) for n in names.values())}"}) + if selection.stopped: return doc - contender = next(side for side in names if side != base) - doc["contender"] = names[contender] - doc["sides"] = {role: _declared(next(d for d, x in zip(documents, sides) if x == side)) - for role, side in (("baseline", base), ("contender", contender))} - - # Run-level invariants (schema §5.5, comparator.md §3), as the profile names them. - check = set(projection.invariants) - if "sides-differ" in check: - a, b = (json.loads(side) for side in (base, contender)) - if all(x == y for path, x, y in zip(projection.axis, a, b) if not path.startswith("/provenance/")): - failed.append({"invariant": "sides-differ", - "detail": f"the sides differ only in their label ({varying}): nothing recorded about the " - "environment differs. Describe what differs with a context collector " - "(collectors in benchx.conf.json) or a setup file for each environment"}) - if "one-tree-per-side" in check: - trees = {side: {d["provenance"].get("subject_tree") for d, x in zip(documents, sides) if x == side} - for side in (base, contender)} - if any(len(t) != 1 or None in t for t in trees.values()) or trees[base] == trees[contender]: - failed.append({"invariant": "one-tree-per-side", "detail": "each side must have one tree id, and " - "the two must differ"}) - if "one-benchmark-tree" in check: - failed.extend(_benchmark_file(documents, names, sides)) - if "alternating-slots" in check: - # Variants are usually measured by one command, so say how to get an alternating run. - how = ("; make one bx bench call per value, each with --run-key, --round and --slot, " - "or run bx compare TARGET --profile variants" if profile == "variants" else "") - slots = {} - for d, s in zip(documents, sides): - slot = d.get("procedure", {}).get("slot") - if slot is None: - failed.append({"invariant": "alternating-slots", - "detail": "a result has no slot: give every bx bench call --slot" + how}) - break - slots.setdefault(slot, set()).add(s) - else: - order = [slots[slot] for slot in sorted(slots)] - if any(len(o) != 1 for o in order) or any(a == b for a, b in zip(order, order[1:])): - failed.append({"invariant": "alternating-slots", - "detail": "sides do not alternate by procedure.slot: run the sides in the same " - "order every round (a, b, a, b, ...)" + how}) - - # Units: one workload variant and quantity each. Everything else the - # projection matches must agree across both sides (schema §5.5). - units = {} - for d, p, s in zip(documents, projected, sides): - variant = {ptr: v for ptr, v in p.identity.items() if ptr == WORKLOAD or ptr.startswith(WORKLOAD + "/")} - key = (_c(variant), d["measurement"]["quantity"]["name"]) - units.setdefault(key, {"docs": {base: [], contender: []}, "identities": []}) - units[key]["docs"][s].append(d) - units[key]["identities"].append(p.identity) - for (_, quantity), unit in units.items(): - differing = _differing(unit["identities"]) - if differing: - failed.append({"invariant": "sides-agree", "workload": _workload(unit["docs"], projection.axis), - "quantity": quantity, "detail": f"{', '.join(differing)} differ"}) + if "alternating-slots" in method.requires: + failed.extend(_alternation(selection.slots, selection.advice.get("alternating-slots", ""))) + failed.extend(selection.disagreements) if failed: return doc - for (_, quantity), unit in sorted( - units.items(), - key=lambda item: _unit_order(_workload(item[1]["docs"], projection.axis), item[0][1])): - by_side = unit["docs"] - where = {"workload": _workload(by_side, projection.axis), "quantity": quantity} - absent = [role for role, s in (("baseline", base), ("contender", contender)) if not by_side[s]] - if absent: - doc["missing"].extend({**where, "side": role} for role in absent) + for evidence in selection.units: + where = {"workload": evidence.workload, "quantity": evidence.quantity["name"]} + if evidence.missing: + doc["missing"].extend({**where, "side": side} for side in evidence.missing) continue - if "equal-attempts" in check and len(by_side[base]) != len(by_side[contender]): - failed.append({"invariant": "equal-attempts", **where, - "detail": f"{len(by_side[base])} against {len(by_side[contender])}: run the same " - "number of rounds on both sides"}) + layout, problem = _lay_out(evidence, method) + if problem: + failed.append({"invariant": problem[0], **where, "detail": problem[1]}) continue - rounds, side_failed = {}, False - for s in (base, contender): - for d in by_side[s]: - if d["measurement"]["status"] != "success": - status, reason = d["measurement"]["status"], d["measurement"].get("reason") - result = {"producer": d["producer"]["name"], "ingest_key": d["ingest_key"]} - doc["exclusions"].append({"result": result, "reason": f"{status}: {reason}" if reason else status}) - side_failed = side_failed or (profile in STRICT_STATUS and status in FAILED_STATUSES) - continue - rounds.setdefault(d.get("procedure", {}).get("round"), {})[s] = d - pairs = [(r[base], r[contender]) for _, r in sorted(rounds.items(), key=lambda x: (x[0] is None, x[0])) - if base in r and contender in r] - unit_doc = _unit(where, pairs, by_side, base, contender, k, min_rounds, side_failed) - unit_doc["unmatched_differences"] = _differences(projection, by_side[base], by_side[contender]) - if caveats := _caveats(by_side, base, contender): - unit_doc["caveats"] = caveats - doc["units"].append(unit_doc) + doc["exclusions"].extend({"result": row.result, "reason": row.exclusion} + for row in evidence.rows if row.exclusion) + doc["units"].append(_unit(selection, evidence, layout, method, configuration, where)) return doc -def _benchmark_file(documents, names, sides) -> list[dict]: - """The `one-benchmark-tree` invariant (schema §5.5, UC-03 §10, UC-02 §5). A benchmark recorded apart from - the subject has its own `benchmark_tree`, which differs from `subject_tree`; it must be one file - on both sides and in every round. A benchmark in the subject's checkout has `benchmark_tree` - equal to `subject_tree` and differs between the sides as the subject does, so it is not - checked. A result without a `benchmark_tree` (a thin result) is outside the rule. - - An in-tree record must also name the subject's source as its benchmark's: one that does not is - malformed. The reverse is not required, since a suite in another checkout of the same - repository has the subject's source URI but its own file id.""" - found, malformed = [], [] - for d, side in zip(documents, sides): - provenance = d["provenance"] - tree = provenance.get("benchmark_tree") - if tree is None: - continue - separate = tree != provenance.get("subject_tree") - found.append((side, tree, separate)) - recorded = d.get("benchmark") - if recorded is not None and not separate and _c(recorded.get("source")) != _c(d["source"]): - malformed.append(str(names[side])) - failed = [] - if malformed: - failed.append({"invariant": "one-benchmark-tree", - "detail": "malformed record: the benchmark is in the subject's tree but names another " - f"source (sides {sorted(set(malformed))})"}) - if any(separate for _, _, separate in found): - trees = {tree for _, tree, _ in found} - if len(trees) != 1 or not all(separate for _, _, separate in found): - failed.append({"invariant": "one-benchmark-tree", - "detail": f"a benchmark recorded apart from the subject must be one file on both sides " - f"and in every round: {len(trees)} benchmark trees " - f"({', '.join(sorted(t[:12] for t in trees))})"}) - return failed - - -def _workload(by_side, axis=()) -> dict: - """The unit's workload variant, as the comparison document names it: what both sides share, - so the parameters on the profile's axis (the ones that tell the sides apart) are left out.""" - workload = next(d for docs in by_side.values() for d in docs)["coordinates"]["workload"] - varied = {path.removeprefix(PARAMETERS) for path in axis if path.startswith(PARAMETERS)} - parameters = {k: v for k, v in (workload.get("parameters") or {}).items() if k not in varied} - return {"name": workload["name"], **({"parameters": parameters} if parameters else {})} - - -def _unit_order(workload: dict, quantity: str) -> tuple: - """The order units are listed in: by workload name, then by parameter name and value, then by - quantity, with numbers inside names and values in natural order (`axis_order`), so that `n=50` - comes before `n=100` and `bench_2` before `bench_10`, as people read them.""" - parameters = sorted((workload.get("parameters") or {}).items()) - return (axis_order(workload["name"]), tuple((name, axis_order(value)) for name, value in parameters), quantity) - - -_ABSENT = object() - - -def _diff(pointer: str, values: list) -> list[str]: - """The deepest pointers at which `values` are not all the same.""" - if all(isinstance(v, dict) for v in values): - keys = sorted(set().union(*values)) - return [p for key in keys for p in _diff(f"{pointer}/{key.replace('~', '~0').replace('/', '~1')}", - [v.get(key, _ABSENT) for v in values])] - distinct = {"" if v is _ABSENT else _c(v) for v in values} - return [pointer] if len(distinct) > 1 else [] - - -def _differing(facts: list[dict]) -> list[str]: - """Pointers whose value is not the same in every one of `facts` (pointer -> value).""" - return sorted(p for key in set().union(*facts) for p in _diff(key, [f.get(key, _ABSENT) for f in facts])) - - -def _declared(document) -> dict: - """What a side says about how it was built and entered, beside what the - runner saw: the declarations are recorded as given and never judged.""" - info = document["provenance"].get("info", {}) - side = {k: info[k] for k in ("how_built", "activation", "shell", "target_provider") if k in info} - if isinstance(side.get("how_built"), dict): # facts, shown as the JSON text they are - side["how_built"] = json.dumps(side["how_built"], sort_keys=True, separators=(",", ":")) - system = document["coordinates"]["environment"].get("system") or {} - if env := (system.get("process") or {}).get("environment"): - side["observed_env"] = env - return side - - -def _caveats(by_side, base, contender) -> list[str]: - """The warnings a side's results carry (quality.warnings), by side, for the reader.""" - found = set() - for role, side in (("baseline", base), ("contender", contender)): - for d in by_side[side]: - for w in d.get("quality", {}).get("warnings", []): - found.add(f"{role}: {w.get('code', w) if isinstance(w, dict) else w}") - # A side that is not clean is named whether or not its producer warned (UC-03 §10). - for flag, code in DIRTY_WARNINGS: - if d["provenance"].get(flag) == "dirty": - found.add(f"{role}: {code}") - return sorted(found) - - -def _unit(where, pairs, by_side, base, contender, k, min_rounds, side_failed=False): - first = {s: by_side[s][0] for s in (base, contender)} - quantity = first[base]["measurement"]["quantity"] - unit = {**where, "series": {name: identity.series_fingerprint(first[s], store_project(first[s]), identity.MEDIAN) - for name, s in (("baseline", base), ("contender", contender))}, - "rounds": len(pairs), "noise_source": "paired-rounds", "verdict": "indeterminate"} - if side_failed: - # UC-02 §10: if either side fails to run, the benchmark has failed; no effect is reported. - unit["reason"] = "side-failed" - return unit - estimates = [(_estimate(b), _estimate(c)) for b, c in pairs] - estimates = [(b, c) for b, c in estimates if b is not None and c is not None] - if estimates: - unit["reference_estimate"] = statistics.median(b for b, _ in estimates) - if quantity.get("deterministic"): - # Deferred: no quantity of the implementation is deterministic (prototype-design.md §5). - unit["reason"] = "deterministic-deferred" - return unit - if len(estimates) < max(min_rounds, MIN_PAIRS): - unit["reason"] = "insufficient-rounds" - return unit - if any(b == 0 for b, _ in estimates): - unit["reason"] = "zero-baseline" - return unit - d = [c / b - 1 for b, c in estimates] - effect = statistics.fmean(d) - noise = statistics.stdev(d) / math.sqrt(len(d)) - unit.update(effect=effect, noise=noise, verdict=_verdict(effect, noise, k, quantity.get("direction"))) +def _alternation(slots: str | None, advice: str) -> list[dict]: + if slots == "absent": + detail = "a result has no slot: give every bx bench call --slot" + elif slots == "not-alternating": + detail = ("sides do not alternate by procedure.slot: run the sides in the same order every round " + "(a, b, a, b, ...)") + else: + return [] + return [{"invariant": "alternating-slots", "detail": detail + advice}] + + +def _lay_out(evidence: Evidence, method: Method) -> tuple[Layout | None, tuple[str, str] | None]: + """This unit in the shape the method asked for; or, when it cannot go to this + method, the invariant that says so and why.""" + if "equal-attempts" in method.requires: + counts = [sum(1 for row in evidence.rows if row.side == side) for side in evidence.sides] + if len(set(counts)) > 1: + return None, ("equal-attempts", (f"{' against '.join(map(str, counts))}: run the same number of rounds " + "on both sides")) + try: + layout = pivot(evidence, method.dims) + except NotApplicable as e: + return None, ("layout", f"{method.id} does not apply: {e}") + if "duplicate-points" in method.requires and any(len(leaf) > 1 for leaf in layout.leaves()): + return None, ("duplicate-points", (f"more than one usable result at the same {', '.join(method.dims)}: " + "the comparator never picks one implicitly")) + return layout, None + + +def _unit(selection: Selection, evidence: Evidence, layout: Layout, method: Method, configuration: dict, + where: dict) -> dict: + assessment = method.assess(layout, evidence, configuration) + Draft202012Validator(method.details_schema).validate(assessment.details) + if evidence.no_verdict and (assessment.verdict, assessment.reason, assessment.effect) != ( + "indeterminate", evidence.no_verdict, None): + raise AssertionError(f"{method.id} gave a verdict on a unit its selector says has none " + f"({evidence.no_verdict}); comparator.md §3") + summary = {**({"target": list(evidence.target)} if evidence.target else {}), + **({"reference": list(evidence.reference)} if evidence.reference else {}), + **({"ordering": evidence.ordering} if evidence.ordering else {}), + **({"boundaries": list(evidence.boundaries)} if evidence.boundaries else {})} + caveats = list(evidence.caveats) + if selection.slots == "not-alternating" and "alternating-slots" not in method.requires: + caveats = sorted({*caveats, "sides-did-not-alternate"}) + unit = {**where, "series": evidence.series, **({"evidence": summary} if summary else {}), + "method": {"name": method.name, "version": method.version}, "verdict": assessment.verdict, + **_partition(evidence, assessment, method.dims), "details": assessment.details} + if assessment.reason: + unit["reason"] = assessment.reason + if assessment.effect: + unit["effect"] = assessment.effect + if evidence.unmatched_differences is not None: + unit["unmatched_differences"] = list(evidence.unmatched_differences) + if caveats: + unit["caveats"] = caveats return unit -def _differences(projection: Projection, a_docs, b_docs) -> list[str]: - """What the projection does not match that differs between the sides: its - ignored facts, and the parts of its axis after the first, which name what - the sides differ in (schema §4.3).""" - def facts(docs): - found = [] - for d in docs: - projected = projection.apply(d, store_project(d)) - found.append({**projected.ignored, **dict(list(zip(projection.axis, projected.axis))[1:])}) - return found +def _partition(evidence: Evidence, assessment, dims: tuple) -> dict: + """Every result of the unit in exactly one of used and excluded, in table order.""" + def ref(row): + return row.result["producer"], row.result["ingest_key"] - def at(fact, pointer): - for key, value in fact.items(): - if pointer == key or pointer.startswith(key + "/"): - return get(value, pointer[len(key):]) if pointer != key else value - return None + def unused(row): + lacking = [dim for dim in dims if dim != SIDE and row.columns.get(dim) is None] + return f"no {lacking[0]}" if lacking else "not-used" + + used = {ref(row) for row in assessment.used} + reasons = {ref(row): reason for row, reason in assessment.excluded} + found = {"used": [], "excluded": []} + for row in evidence.rows: + if ref(row) in used: + found["used"].append(row.result) + else: + found["excluded"].append({"result": row.result, + "reason": row.exclusion or reasons.get(ref(row)) or unused(row)}) + return found - def seen(found, pointer): - return {_c(at(fact, pointer)) for fact in found} - a, b = facts(a_docs), facts(b_docs) - return [p for p in _differing(a + b) if seen(a, p) != seen(b, p)] +def _described(unit: dict) -> str: + """A unit's numbers as its method words them, or plainly when the method is not in this implementation.""" + name = f"{unit['method']['name']}/{unit['method']['version']}" + if name in methods.METHODS: + return methods.METHODS[name].describe(unit.get("effect"), unit["details"]) + effect = f"{unit['effect']['value']:+.4g}" if "effect" in unit else "?" + return f"effect {effect} {json.dumps(unit['details'], sort_keys=True)}" def render(doc: dict) -> str: """The reviewer-speed table (comparator.md §5).""" - lines = [f"run {doc['run_key']} profile {doc['profile']} " - f"baseline {str(doc['baseline'])[:12]} contender {str(doc.get('contender'))[:12]}" - + (" [local-only]" if doc["local_only"] else "")] - for role, side in doc.get("sides", {}).items(): - name = doc[role] + params, sides = doc["selector"]["params"], doc.get("sides", {}) + if sides: + header = (f"run {params['run_key']} profile {params['profile']} " + f"baseline {str(sides['baseline'])[:12]} contender {str(sides.get('contender'))[:12]}") + else: # a selector with sides of its own, such as reference and target + header = " ".join([doc["selector"]["name"], *(f"{k} {v}" for k, v in params.items())]) + lines = [header + (" [local-only]" if doc["local_only"] else "")] + for role, side in sides.get("declared", {}).items(): + name = sides[role] for key in ("how_built", "activation", "observed_env"): if key in side: shown = side[key] if key != "observed_env" else ", ".join(f"{k}={v}" for k, v in side[key].items()) @@ -442,22 +177,19 @@ def render(doc: dict) -> str: lines.append(f" {name}: {key.replace('_', ' ')}: {first}" + (f" (+{len(rest)} lines)" if rest else "")) differs = sorted({p for u in doc["units"] for p in u.get("unmatched_differences", [])}) if differs: - shown = ", ".join(_dotted(p) for p in differs[:6]) + shown = ", ".join(dotted(p) for p in differs[:6]) lines.append(f" differs: {shown}" + (f" (+{len(differs) - 6} more)" if len(differs) > 6 else "")) caveats = sorted({c for u in doc["units"] for c in u.get("caveats", [])}) if caveats: lines.append(f" caveats: {', '.join(caveats)}") - for section in ("regressed", "improved", "changed", "no-change-detected", "indeterminate"): + for section in ("regressed", "improved", "changed", "no-change-detected", "equivalent", "indeterminate"): rows = [u for u in doc["units"] if u["verdict"] == section] if not rows: continue lines.append(f"\n{section.replace('-', ' ').upper()} ({len(rows)})") for u in rows: - effect = f"{u['effect']:+8.2%}" if "effect" in u else " ?" - noise = f"{u['noise']:7.2%}" if "noise" in u else " ?" reason = f" ({u['reason']})" if "reason" in u else "" - lines.append(f" {workload_label(u['workload']):<32} {u['quantity']:<10} effect {effect} noise {noise}" - f" rounds={u['rounds']}{reason}") + lines.append(f" {workload_label(u['workload']):<32} {u['quantity']:<10} {_described(u)}{reason}") for key, title in (("missing", "missing"), ("failed_invariants", "failed invariants"), ("exclusions", "excluded")): if doc[key]: diff --git a/src/benchx/evidence.py b/src/benchx/evidence.py new file mode 100644 index 0000000..cd8f714 --- /dev/null +++ b/src/benchx/evidence.py @@ -0,0 +1,234 @@ +"""The fixed middle of the comparator: Evidence and Assessment (comparator.md §2.6, §4). + + Selector -> Evidence -> Method -> Assessment -> comparison document + +Selectors and methods are modular and versioned; what passes between them is +defined here, so that neither knows about the other. Evidence is a flat table, +and the only structure the framework reads from it is what counting can show: +whether a key is crossed with the side or nested in it, and whether the slots +alternated. Statistics belong to methods. +""" + +from dataclasses import dataclass, field +from typing import Any + +from .core import canonical +from .key import Key + +SIDE = "side" +# The columns of the evidence table, each read by a key. `side` is not among +# them: the selector assigns it. +COLUMNS = { + "round": Key("/procedure/round"), + "slot": Key("/procedure/slot"), + "run_key": Key("/provenance/run_key"), + "attempt_key": Key("/attempt_key"), + "started_at": Key("/provenance/started_at"), + "revision": Key("/revision/key"), +} + + +class CompareError(Exception): + """The request itself cannot be answered, such as an unknown baseline.""" + + +def result_ref(doc: dict) -> dict: + return {"producer": doc["producer"]["name"], "ingest_key": doc["ingest_key"]} + + +@dataclass(frozen=True) +class Row: + """One attempt's estimate. A row with an `exclusion` stays in the table, + since it still says how the run was designed, but no method sees it.""" + result: dict # producer and ingest_key + side: str # as the selector names it, such as baseline or target + columns: dict # column name -> value, None where the result has none + estimate: float | None = None + observations: tuple = () + exclusion: str | None = None + + +@dataclass(frozen=True) +class Evidence: + """One unit: the rows of one series, and what the selector knows about them.""" + series: str + workload: dict + quantity: dict # the measurement's quantity: name, unit, direction, ... + sides: tuple # in order: the reference side first + rows: tuple + missing: tuple = () # sides with no result, which the selector expected + unmatched_differences: tuple | None = None + caveats: tuple = () + # History evidence (comparator.md §2.2): `target` and `reference` are result + # references, the reference oldest first. + target: tuple = () + reference: tuple = () + ordering: str | None = None + boundaries: tuple = () + calibration: tuple = () # past comparisons in the series, for a method's threshold + # Why no verdict may be given though the unit has evidence, such as side-failed + # (UC-02 §10). A selector's rule, not a statistic: every method returns + # indeterminate with this reason, and the framework checks that it did. + no_verdict: str | None = None + + +@dataclass +class Selection: + """What a selector hands the framework: the units, and the facts about the + request that hold for all of them.""" + selector: dict # name, version, params + estimator: dict + identity_schema: str + local_only: bool + inputs: list + units: list = field(default_factory=list) + sides: dict | None = None + slots: str | None = None # alternating, not-alternating, or absent, over the whole run + failed: list = field(default_factory=list) # selector invariants + disagreements: list = field(default_factory=list) # selector invariants found after the method's run-level ones + advice: dict = field(default_factory=dict) # invariant -> what the selector adds to a method requirement's detail + stopped: bool = False # a failure so basic that nothing else can be checked + + +@dataclass(frozen=True) +class Assessment: + """What a method found. Deterministic given its inputs, method version, and configuration.""" + verdict: str + reason: str | None = None + effect: dict | None = None # measure and value + used: tuple = () # rows the verdict rests on + excluded: tuple = () # (row, reason): rows the method read and set aside + details: dict = field(default_factory=dict) + + +class Method: + """A comparison method owns all the statistics (comparator.md §4.1). It says + what shape of evidence it needs in `dims`, and what its statistics assume of + the run in `requires`: alternating-slots, equal-attempts, duplicate-points.""" + name: str + version: str + dims: tuple + requires: tuple = () + defaults: dict = {} + config_schema: dict = {"type": "object"} + details_schema: dict = {"type": "object"} + + @property + def id(self) -> str: + return f"{self.name}/{self.version}" + + def assess(self, layout: "Layout", evidence: Evidence, config: dict) -> Assessment: + raise NotImplementedError + + def describe(self, effect: dict | None, details: dict) -> str: + """One line of the reviewer's table for a unit this method assessed.""" + raise NotImplementedError + + +class Selector: + """A selector forms the evidence (comparator.md §2.2, §2.3, §2.5).""" + name: str + version: str + + def select(self, documents, **params) -> Selection: + raise NotImplementedError + + +# -- what counting shows ------------------------------------------------------- + +def _sides_by_value(rows, column) -> dict: + found = {} + for row in rows: + value = row.columns.get(column) + if value is not None: + found.setdefault(canonical(value), set()).add(row.side) + return found + + +def relation(rows, column) -> str: + """How a column relates to the side. Excluded rows count: they show how the + run was designed, which does not depend on which attempts succeeded.""" + found = _sides_by_value(rows, column) + if not found: + return "absent" + return "crossed" if any(len(sides) > 1 for sides in found.values()) else "nested" + + +def slot_schedule(slots_and_sides) -> str: + """Whether the sides alternate, from (slot, side) over a whole run. A slot + is a position in the run's realized order, so a single unit cannot show it: + a unit that skipped one attempt has gaps that say nothing about the schedule.""" + slots = {} + for slot, side in slots_and_sides: + if slot is None: + return "absent" + slots.setdefault(slot, set()).add(side) + order = [slots[slot] for slot in sorted(slots)] + if any(len(o) != 1 for o in order) or any(a == b for a, b in zip(order, order[1:])): + return "not-alternating" + return "alternating" + + +# -- layout -------------------------------------------------------------------- + +@dataclass(frozen=True) +class Layout: + """A unit's usable rows over named dimensions. `cells` nests one list per + dimension, in `dims` order, along `coords`. A leaf is the tuple of rows at + those coordinates: empty means masked, and it holds several where a dimension + does not tell rows apart, as `("side",)` does not.""" + dims: tuple + coords: dict + cells: Any + + def leaves(self): + def walk(node, depth): + if depth == len(self.dims): + yield node + else: + for child in node: + yield from walk(child, depth + 1) + return walk(self.cells, 0) + + +class NotApplicable(Exception): + """A unit cannot be laid out over a method's dimensions; the message says why.""" + + +def _value(row: Row, dim: str): + return row.side if dim == SIDE else row.columns.get(dim) + + +def pivot(evidence: Evidence, dims: tuple) -> Layout: + """`evidence` over `dims`, with missing cells masked. Every dimension except + the side must be crossed with it, or there is nothing to line the sides up by.""" + rows = evidence.rows + coords = {} + for dim in dims: + if dim == SIDE: + coords[dim] = tuple(evidence.sides) + continue + found = relation(rows, dim) + if found == "absent": + raise NotApplicable(f"it needs {dim}, and no result has one") + if found == "nested": + raise NotApplicable(f"it needs {dim}, and no value of {dim} appears on more than one side") + values = {canonical(_value(r, dim)): _value(r, dim) for r in rows if _value(r, dim) is not None} + coords[dim] = tuple(sorted(values.values(), key=COLUMNS[dim].sort_key)) + + def nest(rows, remaining): + if not remaining: + return tuple(rows) + dim, *rest = remaining + return [nest([r for r in rows if _value(r, dim) == value], rest) for value in coords[dim]] + + return Layout(tuple(dims), coords, nest([r for r in rows if r.exclusion is None], dims)) + + +def change_label(signed: float, direction: str | None) -> str: + """What a change of this sign is, by the quantity's direction when it has one.""" + if direction == "lower-is-better": + return "regressed" if signed > 0 else "improved" + if direction == "higher-is-better": + return "improved" if signed > 0 else "regressed" + return "changed" diff --git a/src/benchx/key.py b/src/benchx/key.py new file mode 100644 index 0000000..c77da39 --- /dev/null +++ b/src/benchx/key.py @@ -0,0 +1,117 @@ +"""Keys: how one column is read from a result (comparator.md §2.6). + +A key is a JSON Pointer, an optional normalization rule, and an ordering: one +definition of how a fact is read and ordered. Projections read their axis and +roles through keys, and the comparator's evidence reads its columns through them. +""" + +import re +import urllib.parse +from dataclasses import dataclass, field +from pathlib import Path + +from .core.hashing import canonical + +# -- source URIs (the canonical-uri rule) -------------------------------------- + +_WINDOWS_PATH = re.compile(r"^[A-Za-z]:[\\/]") +_SCP_REMOTE = re.compile(r"^(?:(?P[^@/:\s\[\]]+)@)?(?P\[[0-9A-Fa-f:.]+\]|[^@/:\s\[\]]+):(?P.+)$") + + +def normalize_remote(remote: str, base: str | None = None) -> str: + """A git remote as a URI, whatever form it was given in; idempotent. + + A `scheme://` remote is unchanged. A Windows drive path or a local path + (resolved against `base`, default the current directory) becomes a + `file://` URI. git's SCP-style `[user@]host:path` becomes `ssh://`: an + absolute path stays absolute, and a relative one (relative to the login + directory) is kept relative as `ssh://host/~/path`, since `ssh://host/path` + would name a different repository.""" + if not remote or "://" in remote: + return remote + if _WINDOWS_PATH.match(remote): + return "file:///" + urllib.parse.quote(remote.replace("\\", "/"), safe="/:") + match = _SCP_REMOTE.match(remote) + if match: + user = f"{match['user']}@" if match["user"] else "" + path = match["path"] if match["path"].startswith("/") else "/~/" + match["path"] + return f"ssh://{user}{match['host']}{path}" + return Path(base or ".", remote).resolve().as_uri() + + +def canonical_uri(uri: str) -> str: + parts = urllib.parse.urlsplit(normalize_remote(uri)) + path = parts.path.rstrip("/").removesuffix(".git") + return urllib.parse.urlunsplit((parts.scheme.lower(), parts.netloc.lower(), path, "", "")) + + +def _normalize(rule: dict, value): + if not isinstance(value, (str, int, float)) or isinstance(value, bool): + return value # rules are defined on scalars; anything else matches as is + if rule["rule"] == "canonical-uri": + return canonical_uri(str(value)) + return rule["map"].get(str(value), value) # map: unlisted values are kept + + +# -- pointers ------------------------------------------------------------------ + +def escape(key: str) -> str: + return key.replace("~", "~0").replace("/", "~1") + + +def get(doc: dict, pointer: str): + """The value at a JSON Pointer, or None when any step is missing.""" + for part in pointer.split("/")[1:]: + part = part.replace("~1", "/").replace("~0", "~") + if not isinstance(doc, dict) or part not in doc: + return None + doc = doc[part] + return doc + + +# -- ordering ------------------------------------------------------------------ + +_DIGITS = re.compile(r"(\d+)") + + +def axis_order(value) -> tuple: + """A sort key for one axis value: numbers by value, strings in natural + order so that versions and sizes sort as people read them (`6.8` before + `6.10`, `size-20` before `size-100`), then anything else by its canonical + form, and a missing value last. Every key starts with a rank for its kind, + so values of different kinds never compare directly.""" + if value is None: + return (4,) + if isinstance(value, bool): + return (0, value) + if isinstance(value, (int, float)): + return (1, value) + if isinstance(value, str): + # re.split with a group alternates text and digits, so equal positions + # always hold the same type; the string itself breaks ties such as `07` + # against `7`. + parts = _DIGITS.split(value) + return (2, tuple(int(p) if i % 2 else p for i, p in enumerate(parts)), value) + return (3, canonical(value).decode()) + + +_ORDERINGS = {"natural": axis_order} + + +# -- keys ---------------------------------------------------------------------- + +@dataclass(frozen=True) +class Key: + pointer: str + normalize: dict | None = field(default=None, hash=False) + order: str = "natural" + + def normalized(self, value): + return value if self.normalize is None else _normalize(self.normalize, value) + + def read(self, doc: dict): + """This key's value in `doc`, normalized; None when `doc` has none.""" + return self.normalized(get(doc, self.pointer)) + + def sort_key(self, value) -> tuple: + return _ORDERINGS[self.order](value) diff --git a/src/benchx/methods/__init__.py b/src/benchx/methods/__init__.py new file mode 100644 index 0000000..0e875c0 --- /dev/null +++ b/src/benchx/methods/__init__.py @@ -0,0 +1,14 @@ +"""The registry of comparison methods (comparator.md §4). Adding one is its own +module and a line here.""" + +from ..evidence import CompareError, Method +from . import paired_relative + +METHODS = {method.id: method for method in (paired_relative.METHOD,)} +DEFAULT = paired_relative.METHOD.id + + +def get(name: str) -> Method: + if name not in METHODS: + raise CompareError(f"method {name!r} is not in the implementation ({', '.join(METHODS)})") + return METHODS[name] diff --git a/src/benchx/methods/paired_relative.py b/src/benchx/methods/paired_relative.py new file mode 100644 index 0000000..2f2c3ef --- /dev/null +++ b/src/benchx/methods/paired_relative.py @@ -0,0 +1,83 @@ +"""benchx/paired-relative/v1 (comparator.md §4.4): mean relative difference over paired rounds.""" + +import math +import statistics + +from ..evidence import Assessment, Method, change_label + +# The noise is the standard error of the per-round differences, so a verdict needs +# at least two pairs, whatever `min_rounds` a caller asks for. +MIN_PAIRS = 2 + + +class PairedRelative(Method): + name = "benchx/paired-relative" + version = "v1" + dims = ("round", "side") + requires = ("equal-attempts", "alternating-slots", "duplicate-points") + defaults = {"k": 3.0, "min_rounds": 3} + config_schema = { + "type": "object", + "additionalProperties": False, + "required": ["k", "min_rounds"], + "properties": {"k": {"type": "number"}, "min_rounds": {"type": "integer"}}, + } + details_schema = { + "type": "object", + "additionalProperties": False, + "required": ["rounds", "noise_kind", "noise_source"], + "properties": { + "rounds": {"description": "Paired rounds", "type": "integer", "minimum": 0}, + "reference_estimate": {"description": "Median of the baseline's estimates", "type": "number"}, + "noise": {"type": "number", "minimum": 0}, + "noise_kind": {"const": "standard-error"}, + "noise_source": {"const": "paired-rounds"}, + "degrees_of_freedom": {"type": "integer", "minimum": 1}, + "threshold": {"description": "k times the noise", "type": "number", "minimum": 0}, + }, + } + + def assess(self, layout, evidence, config): + pairs, excluded = [], [] + for baseline, contender in layout.cells: # one per round, the sides in evidence order + if baseline and contender and baseline[0].estimate is not None and contender[0].estimate is not None: + pairs.append((baseline[0], contender[0])) + else: + excluded.extend((row, "unpaired") for row in (*baseline, *contender)) + used = tuple(row for pair in pairs for row in pair) + details = {"rounds": sum(1 for baseline, contender in layout.cells if baseline and contender), + "noise_kind": "standard-error", "noise_source": "paired-rounds"} + + def indeterminate(reason): + return Assessment("indeterminate", reason=reason, used=used, excluded=tuple(excluded), details=details) + + if evidence.no_verdict: + return indeterminate(evidence.no_verdict) + estimates = [(b.estimate, c.estimate) for b, c in pairs] + if estimates: + details["reference_estimate"] = statistics.median(b for b, _ in estimates) + + if evidence.quantity.get("deterministic"): + # Deferred: no quantity of the implementation is deterministic (comparator.md §4.4). + return indeterminate("deterministic-deferred") + if len(estimates) < max(config["min_rounds"], MIN_PAIRS): + return indeterminate("insufficient-rounds") + if any(b == 0 for b, _ in estimates): + return indeterminate("zero-baseline") + differences = [c / b - 1 for b, c in estimates] + effect = statistics.fmean(differences) + noise = statistics.stdev(differences) / math.sqrt(len(differences)) + threshold = config["k"] * noise + details.update(noise=noise, degrees_of_freedom=len(differences) - 1, threshold=threshold) + verdict = ("no-change-detected" if abs(effect) <= threshold + else change_label(effect, evidence.quantity.get("direction"))) + return Assessment(verdict, effect={"measure": "relative", "value": effect}, used=used, + excluded=tuple(excluded), details=details) + + def describe(self, effect, details): + shown = f"{effect['value']:+8.2%}" if effect else " ?" + noise = f"{details['noise']:7.2%}" if "noise" in details else " ?" + return f"effect {shown} noise {noise} rounds={details['rounds']}" + + +METHOD = PairedRelative() diff --git a/src/benchx/projection.py b/src/benchx/projection.py index 5995d1c..f6bc2f2 100644 --- a/src/benchx/projection.py +++ b/src/benchx/projection.py @@ -11,14 +11,12 @@ projections are built in code. Nothing here reads a store. """ -import re import statistics -import urllib.parse from dataclasses import dataclass -from pathlib import Path from .core import sha256_hex from .core.hashing import canonical +from .key import Key, axis_order, canonical_uri, escape, get, normalize_remote # noqa: F401 # The reported coordinates (schema §4.3), which every role applies to, and the # facts outside them a projection may name: considered only when it does. @@ -36,47 +34,6 @@ def _hash(prefix: str, *parts: bytes) -> str: return sha256_hex(prefix.encode() + b"\0" + b"\0".join(parts)) -# -- source URIs (the canonical-uri rule) -------------------------------------- - -_WINDOWS_PATH = re.compile(r"^[A-Za-z]:[\\/]") -_SCP_REMOTE = re.compile(r"^(?:(?P[^@/:\s\[\]]+)@)?(?P\[[0-9A-Fa-f:.]+\]|[^@/:\s\[\]]+):(?P.+)$") - - -def normalize_remote(remote: str, base: str | None = None) -> str: - """A git remote as a URI, whatever form it was given in; idempotent. - - A `scheme://` remote is unchanged. A Windows drive path or a local path - (resolved against `base`, default the current directory) becomes a - `file://` URI. git's SCP-style `[user@]host:path` becomes `ssh://`: an - absolute path stays absolute, and a relative one (relative to the login - directory) is kept relative as `ssh://host/~/path`, since `ssh://host/path` - would name a different repository.""" - if not remote or "://" in remote: - return remote - if _WINDOWS_PATH.match(remote): - return "file:///" + urllib.parse.quote(remote.replace("\\", "/"), safe="/:") - match = _SCP_REMOTE.match(remote) - if match: - user = f"{match['user']}@" if match["user"] else "" - path = match["path"] if match["path"].startswith("/") else "/~/" + match["path"] - return f"ssh://{user}{match['host']}{path}" - return Path(base or ".", remote).resolve().as_uri() - - -def canonical_uri(uri: str) -> str: - parts = urllib.parse.urlsplit(normalize_remote(uri)) - path = parts.path.rstrip("/").removesuffix(".git") - return urllib.parse.urlunsplit((parts.scheme.lower(), parts.netloc.lower(), path, "", "")) - - -def _normalize(rule: dict, value): - if not isinstance(value, (str, int, float)) or isinstance(value, bool): - return value # rules are defined on scalars; anything else matches as is - if rule["rule"] == "canonical-uri": - return canonical_uri(str(value)) - return rule["map"].get(str(value), value) # map: unlisted values are kept - - # -- estimators ---------------------------------------------------------------- _ESTIMATORS = { @@ -96,21 +53,7 @@ def values(doc: dict) -> list[float]: return [o["value"] if isinstance(o, dict) else o for o in doc["measurement"].get("observations", [])] -# -- pointers ------------------------------------------------------------------ - -def _escape(key: str) -> str: - return key.replace("~", "~0").replace("/", "~1") - - -def get(doc: dict, pointer: str): - """The value at a JSON Pointer, or None when any step is missing.""" - for part in pointer.split("/")[1:]: - part = part.replace("~1", "/").replace("~0", "~") - if not isinstance(doc, dict) or part not in doc: - return None - doc = doc[part] - return doc - +# -- projections --------------------------------------------------------------- def _has_null(value) -> bool: if value is None: @@ -122,8 +65,6 @@ def _has_null(value) -> bool: return False -# -- projections --------------------------------------------------------------- - @dataclass(frozen=True) class Projected: """One result under a projection.""" @@ -148,7 +89,8 @@ def __init__(self, policy: dict, axis: list[str], *, profile: str | None = None, self.policy, self.profile, self.extra = policy, profile, dict(paths or {}) self.identity_schema = (f"{profile}+{policy['identity_schema']}" if profile else policy["identity_schema"]) - self.axis = list(axis) + self.axis_keys = tuple(Key(path) for path in axis) + self.axis = [key.pointer for key in self.axis_keys] self.default = policy["default"] self.paths = {**policy.get("paths", {}), **self.extra} self.invariants = list(invariants) @@ -176,6 +118,13 @@ def role(self, pointer: str): return self.paths[path] return self.default + def key(self, pointer: str) -> Key | None: + """How a fact enters the identity: its role as a key, None when it is ignored.""" + role = self.role(pointer) + if role == "ignore": + return None + return Key(pointer) if role == "match" else Key(pointer, normalize=role["normalize"]) + def apply(self, doc: dict, project: str | None = None) -> Projected: """`doc` under this projection. `project` stands in for the project of a thin result (schema §4.1), so local results of different hosts stay apart.""" @@ -184,21 +133,21 @@ def apply(self, doc: dict, project: str | None = None) -> Projected: identity, ignored = {}, {} def place(pointer, value): - role = self.role(pointer) - if role == "ignore": + key = self.key(pointer) + if key is None: ignored[pointer] = value elif _has_null(value): # A null fact never equals anything (§4.3): this result shares no series. identity[pointer] = {"unmatchable": [doc["producer"]["name"], doc["ingest_key"]]} else: - identity[pointer] = value if role == "match" else _normalize(role["normalize"], value) + identity[pointer] = key.normalized(value) def walk(node, pointer): if pointer in axis_paths: return if isinstance(node, dict) and any(n.startswith(pointer + "/") for n in named): for key, value in node.items(): - walk(value, f"{pointer}/{_escape(key)}") + walk(value, f"{pointer}/{escape(key)}") else: place(pointer, node) @@ -210,7 +159,7 @@ def walk(node, pointer): for pointer in NAMED_ONLY: if pointer in self.paths and pointer not in axis_paths: place(pointer, get(doc, pointer)) - return Projected(tuple(get(doc, path) for path in axis_paths), identity, ignored) + return Projected(tuple(key.read(doc) for key in self.axis_keys), identity, ignored) # -- fingerprints (§4.3) -- @@ -225,31 +174,8 @@ def order_key(self, point_axis: tuple) -> tuple: """A sort key for a point: none for a revision axis, which the implementation orders by time since it has no revision graph (schema §4.1); otherwise the axis values in natural order (`axis_order`).""" - return () if self.axis == ["/revision/key"] else tuple(axis_order(value) for value in point_axis) - - -_DIGITS = re.compile(r"(\d+)") - - -def axis_order(value) -> tuple: - """A sort key for one axis value: numbers by value, strings in natural - order so that versions and sizes sort as people read them (`6.8` before - `6.10`, `size-20` before `size-100`), then anything else by its canonical - form, and a missing value last. Every key starts with a rank for its kind, - so values of different kinds never compare directly.""" - if value is None: - return (4,) - if isinstance(value, bool): - return (0, value) - if isinstance(value, (int, float)): - return (1, value) - if isinstance(value, str): - # re.split with a group alternates text and digits, so equal positions - # always hold the same type; the string itself breaks ties such as `07` - # against `7`. - parts = _DIGITS.split(value) - return (2, tuple(int(p) if i % 2 else p for i, p in enumerate(parts)), value) - return (3, canonical(value).decode()) + return () if self.axis == ["/revision/key"] else tuple( + key.sort_key(value) for key, value in zip(self.axis_keys, point_axis)) def workload_label(workload: dict) -> str: diff --git a/src/benchx/run_profile.py b/src/benchx/run_profile.py new file mode 100644 index 0000000..5608302 --- /dev/null +++ b/src/benchx/run_profile.py @@ -0,0 +1,383 @@ +"""The run-profile selector: two sides of one run, as a profile defines them (comparator.md §2.3). + +A profile is a projection built here from the identity policy: its axis says what +tells the sides apart, the policy's roles what must match for two results to be +compared, and its invariants what makes the sides meaningful (schema §5.5). The +checks a method needs, such as alternation, are the method's to ask for. Units +are the profile projection's series. +""" + +import json +import re + +from . import identity +from .core import canonical +from .evidence import COLUMNS, CompareError, Evidence, Row, Selection, Selector, result_ref, slot_schedule +from .key import escape, get +from .projection import Projection, axis_order, estimate, values +from .snapshot import DIRTY_WARNINGS +from .store import store_project + +PROFILES = { + # UC-03: two subject trees from one run. + "revisions": Projection(identity.POLICY_DATA, ["/provenance/subject_tree"], + profile="benchx/profile/revisions/v1", + invariants=["one-run", "two-sides", "one-tree-per-side", "one-benchmark-tree"]), + # UC-04: one revision measured two ways. A side is what its subject + # configuration, pinned components, and system are; the caller's label, + # put in front of the axis per comparison, only names it. + "environments": Projection(identity.POLICY_DATA, ["/coordinates/subject/configuration", + "/coordinates/subject/components", + "/coordinates/environment/system"], + profile="benchx/profile/environments/v1", + paths={"/revision/key": "match", "/provenance/subject_tree": "match"}, + invariants=["one-run", "two-sides", "sides-differ"]), + # UC-02: one revision, one environment, two values of one workload parameter. Everything else + # must agree, so the axis (the parameter) is added per comparison: see `profile_projection`. + "variants": Projection(identity.POLICY_DATA, [], + profile="benchx/profile/variants/v1", + paths={"/revision/key": "match", "/provenance/subject_tree": "match"}, + invariants=["one-run", "two-sides", "one-benchmark-tree"]), +} +# Profiles for which a failed attempt on either side leaves its unit without a verdict (UC-02 §10): +# a variant that times out in the slow rounds must not be judged on the fast ones. Under the other +# profiles a method sees the remaining attempts. A selector's rule, so it holds for every method. +STRICT_STATUS = {"variants"} +FAILED_STATUSES = ("partial", "censored", "error") # `skipped` is not a failure +WORKLOAD = "/coordinates/workload" +PARAMETERS = WORKLOAD + "/parameters/" +_PARAMETER_NAME = re.compile(r"[A-Za-z0-9_-]+") +ROLES = ("baseline", "contender") + + +def _c(value) -> str: + return canonical(value).decode() + + +def profile_projection(profile: str, label: str | None = None, parameter: str | None = None) -> Projection: + """The profile's projection; for environments, with the label that names + the sides at the front of its axis, and for variants, with the workload + parameter whose two values are the sides.""" + if profile not in PROFILES: + raise CompareError(f"profile {profile!r} is not in the implementation ({', '.join(PROFILES)})") + projection = PROFILES[profile] + if profile == "environments": + if not label: + raise CompareError("the environments profile needs the label that names the sides") + projection = projection.with_axis_first(f"/provenance/labels/{label}") + if profile == "variants": + if not parameter: + raise CompareError("the variants profile needs the workload parameter whose values are the sides") + if not _PARAMETER_NAME.fullmatch(parameter): # keeps the document's `varying` a token + raise CompareError(f"workload parameter {parameter!r}: use letters, digits, '_' and '-' only") + projection = projection.with_axis_first(PARAMETERS + parameter) + return projection + + +def _run_documents(documents, run_key) -> list[dict]: + """The measurands of one run, in a fixed order, so the same results give the same document.""" + found = sorted((d for d in documents if d["provenance"].get("run_key") == run_key and _measurand(d)), + key=lambda d: (d["producer"]["name"], d["ingest_key"])) + if not found: + raise CompareError(f"no results for run {run_key!r}") + return found + + +def infer_parameter(documents, run_key) -> str: + """The workload parameter a variants comparison of this run varies, when the caller names none: + the only parameter the run's results carry. An `-e` variable counts, since the runner records + it as a workload parameter. Read mode only; measure mode asks before running (session).""" + names = sorted({name for d in _run_documents(documents, run_key) + for name in (d["coordinates"]["workload"].get("parameters") or {})}) + if not names: + raise CompareError("nothing to vary: no workload parameter in the results " + "(declare one in `params`/`param_names`)") + if len(names) > 1: + raise CompareError(f"several workload parameters ({', '.join(names)}): give --vary NAME") + return names[0] + + +def dotted(pointer: str) -> str: + return pointer[1:].replace("/", ".") + + +def _measurand(doc) -> bool: + """Whether the result is what its attempt set out to measure. A condition + measurement (`measurement.kind: condition`, schema §4.4), such as the + runner's load average, is kept beside the measurands and never compared.""" + return doc["measurement"].get("kind", "measurand") == "measurand" + + +def _failed(invariant: str, detail: str, **where) -> dict: + return {"invariant": invariant, **where, "detail": detail} + + +class RunProfile(Selector): + name = "benchx/run-profile" + version = "v1" + + def select(self, documents, *, run_key, profile, baseline, label=None, parameter=None) -> Selection: + projection = profile_projection(profile, label, parameter) + documents = _run_documents(documents, run_key) + naming = projection.axis[0] + varying = dotted(naming) + params = {"run_key": run_key, "profile": profile, "baseline": baseline, **({"label": label} if label else {}), + **({"parameter": parameter} if profile == "variants" else {})} + selection = Selection( + selector={"name": self.name, "version": self.version, "params": params}, + estimator=dict(identity.MEDIAN), identity_schema=projection.identity_schema, + local_only=any(d["provenance"][f] != "clean" for d in documents + for f in ("subject_dirty", "benchmark_dirty")), + inputs=[result_ref(d) for d in documents], + sides={"varying": varying, "baseline": baseline}) + if profile == "variants": + # Variants are usually measured by one command, so say how to get an alternating run. + selection.advice["alternating-slots"] = ("; make one bx bench call per value, each with --run-key, " + "--round and --slot, or run bx compare TARGET --profile variants") + projected = [projection.apply(d, store_project(d)) for d in documents] + if any(p.axis[0] is None for p in projected): + if profile == "revisions": + # UC-03 §10: a side with no working-tree id (its dirty state is `unknown`) cannot be named. + detail = (f"a result has no {varying}: its tree's dirty state is unknown, so the side cannot be told " + "apart from the other or checked for a change during the run. Make every file in the " + "tree readable, or git-ignore it") + elif profile == "variants": + detail = (f"a result has no workload parameter {parameter}: declare it in the benchmark's " + "`params`/`param_names`") + else: + detail = (f"a result has no {varying}: give every bx bench call --label NAME=VALUE, " + "and name that label here") + selection.failed.append(_failed("sides", detail)) + selection.stopped = True + return selection + + # A side is a distinct axis value, named by its first part (a tree, a label). + sides = [_c(p.axis) for p in projected] + names = {side: p.axis[0] for side, p in zip(sides, projected)} + if profile == "revisions": + matches = {side for side, d in zip(sides, documents) + if names[side].startswith(baseline) or d["revision"]["key"].startswith(baseline)} + else: + matches = {side for side in names if names[side] == baseline} + if len({_c(names[side]) for side in matches}) != 1: + raise CompareError(f"baseline {baseline!r} matches {len(matches)} sides of " + f"{sorted(map(str, names.values()))}") + base = sorted(matches)[0] # several sides of one name fail two-sides below + # The side, not how the caller abbreviated it: one comparison, one document. + params["baseline"] = selection.sides["baseline"] = names[base] + if len(names) != 2: + selection.failed.append(_failed("two-sides", f"{len(names)} sides: " + f"{sorted(_c(n) for n in names.values())}")) + selection.stopped = True + return selection + contender = next(side for side in names if side != base) + selection.sides["contender"] = names[contender] + selection.sides["declared"] = {role: _declared(next(d for d, x in zip(documents, sides) if x == side)) + for role, side in zip(ROLES, (base, contender))} + selection.slots = slot_schedule((COLUMNS["slot"].read(d), s) for d, s in zip(documents, sides)) + selection.failed.extend(self._profile_invariants(projection, documents, names, sides, base, contender, varying)) + groups = self._series(projection, documents, projected, sides, base, contender) + selection.disagreements.extend(self._disagreements(projection, groups)) + strict = profile in STRICT_STATUS + selection.units.extend( + self._unit(projection, group, base, contender, strict) + for group in sorted(groups.values(), + key=lambda g: _unit_order(_workload(g["docs"], projection.axis), g["quantity"]))) + return selection + + def _profile_invariants(self, projection, documents, names, sides, base, contender, varying) -> list[dict]: + """Run-level invariants (schema §5.5, comparator.md §3), as the profile names them.""" + check, failed = set(projection.invariants), [] + if "sides-differ" in check: + a, b = (json.loads(side) for side in (base, contender)) + if all(x == y for path, x, y in zip(projection.axis, a, b) if not path.startswith("/provenance/")): + failed.append(_failed("sides-differ", + f"the sides differ only in their label ({varying}): nothing recorded about the " + "environment differs. Describe what differs with a context collector " + "(collectors in benchx.conf.json) or a setup file for each environment")) + if "one-tree-per-side" in check: + trees = {side: {d["provenance"].get("subject_tree") for d, x in zip(documents, sides) if x == side} + for side in (base, contender)} + if any(len(t) != 1 or None in t for t in trees.values()) or trees[base] == trees[contender]: + failed.append(_failed("one-tree-per-side", "each side must have one tree id, and the two must differ")) + if "one-benchmark-tree" in check: + failed.extend(_benchmark_file(documents, names, sides)) + return failed + + def _series(self, projection, documents, projected, sides, base, contender) -> dict: + """Units are series: the results grouped by fingerprint under the profile's projection.""" + groups = {} + for d, p, s in zip(documents, projected, sides): + quantity = d["measurement"]["quantity"]["name"] + fingerprint = projection.series_fingerprint(p, quantity, identity.MEDIAN) + group = groups.setdefault(fingerprint, {"series": fingerprint, "quantity": quantity, "identity": p.identity, + "docs": {base: [], contender: []}}) + group["docs"][s].append(d) + return groups + + def _disagreements(self, projection, groups) -> list[dict]: + """Several series for one workload variant and quantity mean the sides + disagree on a matched fact (schema §5.5). A variant on one side only + is one series, and merely missing; this is what tells the two apart.""" + variants = {} + for group in groups.values(): + variant = {ptr: v for ptr, v in group["identity"].items() + if ptr == WORKLOAD or ptr.startswith(WORKLOAD + "/")} + variants.setdefault((_c(variant), group["quantity"]), []).append(group) + failed = [] + for (_, quantity), found in variants.items(): + if len(found) > 1: + differing = _differing([g["identity"] for g in found]) + failed.append(_failed("sides-agree", f"{', '.join(differing)} differ", + workload=_workload(found[0]["docs"], projection.axis), quantity=quantity)) + return failed + + def _unit(self, projection, group, base, contender, strict=False) -> Evidence: + by_side = group["docs"] + workload = _workload(by_side, projection.axis) + rows = tuple(_row(d, role) for role, side in zip(ROLES, (base, contender)) for d in by_side[side]) + absent = tuple(role for role, side in zip(ROLES, (base, contender)) if not by_side[side]) + if absent: + quantity = next(d for docs in by_side.values() for d in docs)["measurement"]["quantity"] + return Evidence(group["series"], workload, quantity, ROLES, rows, missing=absent) + first = {side: by_side[side][0] for side in (base, contender)} + return Evidence( + group["series"], workload, first[base]["measurement"]["quantity"], ROLES, rows, + unmatched_differences=tuple(_differences(projection, by_side[base], by_side[contender])), + caveats=tuple(_caveats(by_side, base, contender)), + no_verdict="side-failed" if strict and any(d["measurement"]["status"] in FAILED_STATUSES + for docs in by_side.values() for d in docs) else None) + + +RUN_PROFILE = RunProfile() + + +def _row(doc: dict, side: str) -> Row: + measurement = doc["measurement"] + columns = {name: key.read(doc) for name, key in COLUMNS.items()} + if measurement["status"] != "success": + status, reason = measurement["status"], measurement.get("reason") + return Row(result_ref(doc), side, columns, exclusion=f"{status}: {reason}" if reason else status) + batch = values(doc) + return Row(result_ref(doc), side, columns, estimate(identity.MEDIAN, batch), tuple(batch)) + + +def _benchmark_file(documents, names, sides) -> list[dict]: + """The `one-benchmark-tree` invariant (schema §5.5, UC-03 §10, UC-02 §5). A benchmark recorded apart from + the subject has its own `benchmark_tree`, which differs from `subject_tree`; it must be one file + on both sides and in every round. A benchmark in the subject's checkout has `benchmark_tree` + equal to `subject_tree` and differs between the sides as the subject does, so it is not + checked. A result without a `benchmark_tree` (a thin result) is outside the rule. + + An in-tree record must also name the subject's source as its benchmark's: one that does not is + malformed. The reverse is not required, since a suite in another checkout of the same + repository has the subject's source URI but its own file id.""" + found, malformed = [], [] + for d, side in zip(documents, sides): + provenance = d["provenance"] + tree = provenance.get("benchmark_tree") + if tree is None: + continue + separate = tree != provenance.get("subject_tree") + found.append((side, tree, separate)) + recorded = d.get("benchmark") + if recorded is not None and not separate and _c(recorded.get("source")) != _c(d["source"]): + malformed.append(str(names[side])) + failed = [] + if malformed: + failed.append(_failed("one-benchmark-tree", + "malformed record: the benchmark is in the subject's tree but names another " + f"source (sides {sorted(set(malformed))})")) + if any(separate for _, _, separate in found): + trees = {tree for _, tree, _ in found} + if len(trees) != 1 or not all(separate for _, _, separate in found): + failed.append(_failed("one-benchmark-tree", + f"a benchmark recorded apart from the subject must be one file on both sides " + f"and in every round: {len(trees)} benchmark trees " + f"({', '.join(sorted(t[:12] for t in trees))})")) + return failed + + +def _workload(by_side, axis=()) -> dict: + """The unit's workload variant, as the comparison document names it: what both sides share, + so the parameters on the profile's axis (the ones that tell the sides apart) are left out.""" + workload = next(d for docs in by_side.values() for d in docs)["coordinates"]["workload"] + varied = {path.removeprefix(PARAMETERS) for path in axis if path.startswith(PARAMETERS)} + parameters = {k: v for k, v in (workload.get("parameters") or {}).items() if k not in varied} + return {"name": workload["name"], **({"parameters": parameters} if parameters else {})} + + +def _unit_order(workload: dict, quantity: str) -> tuple: + """The order units are listed in: by workload name, then by parameter name and value, then by + quantity, with numbers inside names and values in natural order (`axis_order`), so that `n=50` + comes before `n=100` and `bench_2` before `bench_10`, as people read them.""" + parameters = sorted((workload.get("parameters") or {}).items()) + return (axis_order(workload["name"]), tuple((name, axis_order(value)) for name, value in parameters), quantity) + + +_ABSENT = object() + + +def _diff(pointer: str, found: list) -> list[str]: + """The deepest pointers at which `found` are not all the same.""" + if all(isinstance(v, dict) for v in found): + keys = sorted(set().union(*found)) + return [p for key in keys for p in _diff(f"{pointer}/{escape(key)}", [v.get(key, _ABSENT) for v in found])] + distinct = {"" if v is _ABSENT else _c(v) for v in found} + return [pointer] if len(distinct) > 1 else [] + + +def _differing(facts: list[dict]) -> list[str]: + """Pointers whose value is not the same in every one of `facts` (pointer -> value).""" + return sorted(p for key in set().union(*facts) for p in _diff(key, [f.get(key, _ABSENT) for f in facts])) + + +def _declared(document) -> dict: + """What a side says about how it was built and entered, beside what the + runner saw: the declarations are recorded as given and never judged.""" + info = document["provenance"].get("info", {}) + side = {k: info[k] for k in ("how_built", "activation", "shell", "target_provider") if k in info} + if isinstance(side.get("how_built"), dict): # facts, shown as the JSON text they are + side["how_built"] = json.dumps(side["how_built"], sort_keys=True, separators=(",", ":")) + system = document["coordinates"]["environment"].get("system") or {} + if env := (system.get("process") or {}).get("environment"): + side["observed_env"] = env + return side + + +def _caveats(by_side, base, contender) -> list[str]: + """The warnings a side's results carry (quality.warnings), by side, for the reader.""" + found = set() + for role, side in zip(ROLES, (base, contender)): + for d in by_side[side]: + for w in d.get("quality", {}).get("warnings", []): + found.add(f"{role}: {w.get('code', w) if isinstance(w, dict) else w}") + # A side that is not clean is named whether or not its producer warned (UC-03 §10). + for flag, code in DIRTY_WARNINGS: + if d["provenance"].get(flag) == "dirty": + found.add(f"{role}: {code}") + return sorted(found) + + +def _differences(projection: Projection, a_docs, b_docs) -> list[str]: + """What the projection does not match that differs between the sides: its + ignored facts, and the parts of its axis after the first, which name what + the sides differ in (schema §4.3).""" + def facts(docs): + found = [] + for d in docs: + projected = projection.apply(d, store_project(d)) + found.append({**projected.ignored, **dict(list(zip(projection.axis, projected.axis))[1:])}) + return found + + def at(fact, pointer): + for key, value in fact.items(): + if pointer == key or pointer.startswith(key + "/"): + return get(value, pointer[len(key):]) if pointer != key else value + return None + + def seen(found, pointer): + return {_c(at(fact, pointer)) for fact in found} + + a, b = facts(a_docs), facts(b_docs) + return [p for p in _differing(a + b) if seen(a, p) != seen(b, p)] diff --git a/src/benchx/schemas/comparison-document/0.1.0/examples/history.json b/src/benchx/schemas/comparison-document/0.1.0/examples/history.json index 7c733b2..e6b9ae9 100644 --- a/src/benchx/schemas/comparison-document/0.1.0/examples/history.json +++ b/src/benchx/schemas/comparison-document/0.1.0/examples/history.json @@ -1,10 +1,19 @@ { "schema_version": "benchx/comparison-document/0.1.0", - "mode": "history", + "selector": { + "name": "benchx/preceding-window", + "version": "v1", + "params": { + "target": { + "producer": "example.org/arrow-adapter", + "ingest_key": "ci-1240:parquet-read/snappy:wall-time" + }, + "window": 20 + } + }, "method": { "name": "benchx/dispersion-scaled", "version": "v1", - "effect_measure": "relative", "configuration": { "k": 3.0, "window": 20, @@ -18,6 +27,7 @@ }, "identity_schema": "arrow/identity/v2", "local_only": false, + "failed_invariants": [], "units": [ { "workload": { @@ -28,11 +38,53 @@ }, "quantity": "wall-time", "series": "5b7d9f1a3c5e7b9d1f3a5c7e9b1d3f5a7c9e1b3d5f7a9c1e3b5d7f9a1c3e5b7d", - "target": { - "producer": "example.org/arrow-adapter", - "ingest_key": "ci-1240:parquet-read/snappy:wall-time" + "evidence": { + "target": [ + { + "producer": "example.org/arrow-adapter", + "ingest_key": "ci-1240:parquet-read/snappy:wall-time" + } + ], + "reference": [ + { + "producer": "example.org/arrow-adapter", + "ingest_key": "ci-1236:parquet-read/snappy:wall-time" + }, + { + "producer": "example.org/arrow-adapter", + "ingest_key": "ci-1237:parquet-read/snappy:wall-time" + }, + { + "producer": "example.org/arrow-adapter", + "ingest_key": "ci-1238:parquet-read/snappy:wall-time" + }, + { + "producer": "example.org/arrow-adapter", + "ingest_key": "ci-1239:parquet-read/snappy:wall-time" + } + ], + "ordering": "ancestry", + "boundaries": [ + { + "kind": "distribution-reset", + "at": { + "producer": "example.org/arrow-adapter", + "ingest_key": "ci-1236:parquet-read/snappy:wall-time" + }, + "detail": "runner kernel upgraded" + } + ] + }, + "method": { + "name": "benchx/dispersion-scaled", + "version": "v1" + }, + "verdict": "no-change-detected", + "effect": { + "measure": "relative", + "value": -0.008 }, - "reference": [ + "used": [ { "producer": "example.org/arrow-adapter", "ingest_key": "ci-1236:parquet-read/snappy:wall-time" @@ -48,25 +100,28 @@ { "producer": "example.org/arrow-adapter", "ingest_key": "ci-1239:parquet-read/snappy:wall-time" + }, + { + "producer": "example.org/arrow-adapter", + "ingest_key": "ci-1240:parquet-read/snappy:wall-time" } ], - "ordering": "ancestry", - "boundaries": [ + "excluded": [ { - "kind": "distribution-reset", - "at": { + "result": { "producer": "example.org/arrow-adapter", - "ingest_key": "ci-1236:parquet-read/snappy:wall-time" + "ingest_key": "ci-1235:parquet-read/snappy:wall-time" }, - "detail": "runner kernel upgraded" + "reason": "status error" } ], - "reference_estimate": 0.0102, - "effect": -0.008, - "noise": 0.004, - "noise_source": "reference-points", - "threshold": 0.012, - "verdict": "no-change-detected", + "details": { + "reference_estimate": 0.0102, + "noise": 0.004, + "noise_kind": "standard-deviation", + "noise_source": "reference-points", + "threshold": 0.012 + }, "unmatched_differences": [ "/coordinates/environment/system/os/kernel" ] diff --git a/src/benchx/schemas/comparison-document/0.1.0/examples/run.json b/src/benchx/schemas/comparison-document/0.1.0/examples/run.json index 1f4d8fd..b47cb77 100644 --- a/src/benchx/schemas/comparison-document/0.1.0/examples/run.json +++ b/src/benchx/schemas/comparison-document/0.1.0/examples/run.json @@ -1,10 +1,18 @@ { "schema_version": "benchx/comparison-document/0.1.0", - "mode": "run", + "selector": { + "name": "benchx/run-profile", + "version": "v1", + "params": { + "run_key": "urn:uuid:9b2e4f1a-6c3d-4e8b-a1f0-2d7c5b9e3a41", + "profile": "environments", + "baseline": "plain", + "label": "build" + } + }, "method": { "name": "benchx/paired-relative", "version": "v1", - "effect_measure": "relative", "configuration": { "k": 3.0, "min_rounds": 3 @@ -17,12 +25,11 @@ }, "identity_schema": "benchx/profile/environments/v1+benchx/benchx-identity/v1", "local_only": true, - "run_key": "urn:uuid:9b2e4f1a-6c3d-4e8b-a1f0-2d7c5b9e3a41", - "profile": "environments", - "varying": "provenance.labels.build", - "baseline": "plain", - "contender": "hardened", - "pairing": "procedure.round", + "sides": { + "varying": "provenance.labels.build", + "baseline": "plain", + "contender": "hardened" + }, "failed_invariants": [], "units": [ { @@ -30,17 +37,52 @@ "name": "TakeChunked/4194304/2" }, "quantity": "wall-time", - "series": { - "baseline": "3f1c9a0b7d2e4f6a8b1c3d5e7f9a0b2c4d6e8f0a1b3c5d7e9f0a2b4c6d8e0f1a", - "contender": "8e2d4c6b0a9f1e3d5c7b9a0f2e4d6c8b0a1f3e5d7c9b0a2f4e6d8c0b1a3f5e7d" + "series": "c4a8e21f6b0d3975e1f8a2c6d4b07e3915fa8c2d6e0b4a7193d5c8f2e6a1b047", + "method": { + "name": "benchx/paired-relative", + "version": "v1" }, - "rounds": 3, - "reference_estimate": 0.0455, - "effect": 0.024, - "noise": 0.0021, - "noise_source": "paired-rounds", - "threshold": 0.0063, "verdict": "regressed", + "effect": { + "measure": "relative", + "value": 0.024 + }, + "used": [ + { + "producer": "benchx/gbench-adapter", + "ingest_key": "run-9b2e:TakeChunked/4194304/2:slot-1:wall-time" + }, + { + "producer": "benchx/gbench-adapter", + "ingest_key": "run-9b2e:TakeChunked/4194304/2:slot-2:wall-time" + }, + { + "producer": "benchx/gbench-adapter", + "ingest_key": "run-9b2e:TakeChunked/4194304/2:slot-3:wall-time" + }, + { + "producer": "benchx/gbench-adapter", + "ingest_key": "run-9b2e:TakeChunked/4194304/2:slot-4:wall-time" + }, + { + "producer": "benchx/gbench-adapter", + "ingest_key": "run-9b2e:TakeChunked/4194304/2:slot-5:wall-time" + }, + { + "producer": "benchx/gbench-adapter", + "ingest_key": "run-9b2e:TakeChunked/4194304/2:slot-6:wall-time" + } + ], + "excluded": [], + "details": { + "rounds": 3, + "reference_estimate": 0.0455, + "noise": 0.0021, + "noise_kind": "standard-error", + "noise_source": "paired-rounds", + "degrees_of_freedom": 2, + "threshold": 0.0063 + }, "unmatched_differences": [] }, { @@ -48,10 +90,38 @@ "name": "TakeChunked/65536/2" }, "quantity": "wall-time", - "rounds": 2, - "noise_source": "paired-rounds", + "series": "7d13f0b9c2e64a8815d7ca30e9f2b6417ac8d05e3b1f96c2704ea8d1c3b5f692", + "method": { + "name": "benchx/paired-relative", + "version": "v1" + }, "verdict": "indeterminate", - "reason": "insufficient-rounds" + "reason": "insufficient-rounds", + "used": [ + { + "producer": "benchx/gbench-adapter", + "ingest_key": "run-9b2e:TakeChunked/65536/2:slot-7:wall-time" + }, + { + "producer": "benchx/gbench-adapter", + "ingest_key": "run-9b2e:TakeChunked/65536/2:slot-8:wall-time" + }, + { + "producer": "benchx/gbench-adapter", + "ingest_key": "run-9b2e:TakeChunked/65536/2:slot-9:wall-time" + }, + { + "producer": "benchx/gbench-adapter", + "ingest_key": "run-9b2e:TakeChunked/65536/2:slot-10:wall-time" + } + ], + "excluded": [], + "details": { + "rounds": 2, + "noise_kind": "standard-error", + "noise_source": "paired-rounds" + }, + "unmatched_differences": [] } ], "missing": [ diff --git a/src/benchx/schemas/comparison-document/0.1.0/schema.json b/src/benchx/schemas/comparison-document/0.1.0/schema.json index b63bc5d..660323a 100644 --- a/src/benchx/schemas/comparison-document/0.1.0/schema.json +++ b/src/benchx/schemas/comparison-document/0.1.0/schema.json @@ -2,19 +2,20 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "urn:benchx:schema:comparison-document:0.1.0", "title": "BenchX comparison document", - "description": "The comparator's output: verdicts for target results against their references, in history mode or run mode, with the evidence behind each. See docs/design/comparator.md section 5. Derived and recomputed on demand; the store keeps its inputs, not the document.", - "$comment": "Rules the schema cannot express and the comparator enforces: inputs lists every result the document refers to; local_only is true when any input is not clean in subject_dirty or benchmark_dirty (benchmark-result-schema.md section 5.5); a unit affected by a failed invariant has no verdict and appears in no units entry; in history mode every reference result precedes the target under the recorded ordering.", + "description": "The comparator's output: verdicts for target results against their references, with the selector that formed the evidence and the method that assessed it. See docs/design/comparator.md section 5. Derived and recomputed on demand; the store keeps its inputs, not the document.", + "$comment": "Rules the schema cannot express and the comparator enforces: inputs lists every result the document refers to; local_only is true when any input is not clean in subject_dirty or benchmark_dirty (benchmark-result-schema.md section 5.5); a unit affected by a failed invariant has no verdict and appears in no units entry; every result of a unit is in exactly one of its used and excluded; details conforms to the schema of the method that wrote it, which the framework checks when it produces the document; under the preceding-window selector every reference result precedes the target under the recorded ordering.", "type": "object", "additionalProperties": false, "required": [ "schema_version", - "mode", + "selector", "method", "estimator", "identity_schema", "local_only", "units", "missing", + "failed_invariants", "exclusions", "inputs" ], @@ -23,14 +24,11 @@ "description": "Message contract version this object conforms to.", "const": "benchx/comparison-document/0.1.0" }, - "mode": { - "description": "History mode compares a target series point against preceding points of its series; run mode compares two sides measured in one run, as a profile defines them.", - "enum": [ - "history", - "run" - ] + "selector": { + "$ref": "#/$defs/selector" }, "method": { + "description": "The method requested and its complete configuration, recorded even when a failed invariant left no units.", "$ref": "#/$defs/method" }, "estimator": { @@ -45,60 +43,55 @@ "description": "True when any input is not clean; such a comparison never promotes into tracked history.", "type": "boolean" }, - "run_key": { - "description": "Run mode: the run both sides were measured in.", - "$ref": "#/$defs/token" - }, - "profile": { - "description": "Run mode: the comparison profile the run was checked against (benchmark-result-schema.md section 5.5).", - "enum": [ - "single-run", - "variants", - "revisions", - "environments", - "cross-machine" - ] - }, - "varying": { - "description": "Run mode: path of the coordinate whose value distinguishes the sides, such as provenance.subject_tree or coordinates.workload.parameters.role.", - "$ref": "#/$defs/token" - }, - "baseline": { - "description": "Run mode: the reference side, as its value of the varying coordinate.", - "$ref": "#/$defs/side" - }, - "contender": { - "description": "Run mode: the target side. Absent when a failed invariant prevented identifying it.", - "$ref": "#/$defs/side" - }, "sides": { - "description": "Run mode: what each side declared about how it was built and entered, recorded as given and never judged, beside the environment the runner observed for it.", + "description": "The sides the selector named, when it names them. Absent from a selector whose sides are its own, such as reference and target.", "type": "object", "additionalProperties": false, + "required": [ + "varying", + "baseline" + ], "properties": { + "varying": { + "description": "Path of the coordinate whose value distinguishes the sides, such as provenance.subject_tree or coordinates.workload.parameters.role.", + "$ref": "#/$defs/token" + }, "baseline": { - "$ref": "#/$defs/declared" + "description": "The reference side, as its value of the varying coordinate.", + "$ref": "#/$defs/side" }, "contender": { - "$ref": "#/$defs/declared" + "description": "The target side. Absent when a failed invariant prevented identifying it.", + "$ref": "#/$defs/side" + }, + "declared": { + "description": "What each side declared about how it was built and entered, recorded as given and never judged, beside the environment the runner observed for it.", + "type": "object", + "additionalProperties": false, + "properties": { + "baseline": { + "$ref": "#/$defs/declared" + }, + "contender": { + "$ref": "#/$defs/declared" + } + } } } }, - "pairing": { - "description": "Run mode: how the two sides' attempts are paired.", - "$comment": "Only paired comparison is defined; comparator.md open question 2 asks whether an unpaired method should be allowed.", - "const": "procedure.round" - }, "failed_invariants": { - "description": "Run mode: profile invariants the run did not satisfy.", + "description": "Invariants the evidence did not satisfy, whether the selector's or the method's (comparator.md section 3).", "type": "array", "items": { "$ref": "#/$defs/failedInvariant" } }, "units": { - "description": "One evaluation each: a series point in history mode, a workload variant and quantity in run mode.", - "type": "array" + "description": "One evaluation each: one series and its assessment.", + "type": "array", + "items": { + "$ref": "#/$defs/unit" + } }, "missing": { "description": "Expected targets with no result, such as a side absent from a run or a series with no result from a batch request's target.", @@ -108,7 +101,7 @@ } }, "exclusions": { - "description": "Results left out of the evidence, with the reason.", + "description": "Results left out of the evidence of any evaluated unit because they have no usable estimate, with the reason. Each also appears in its unit's excluded.", "type": "array", "items": { "$ref": "#/$defs/exclusion" @@ -123,60 +116,14 @@ } } }, - "allOf": [ - { - "if": { - "properties": { - "mode": { - "const": "run" - } - } - }, - "then": { - "required": [ - "run_key", - "profile", - "varying", - "baseline", - "pairing", - "failed_invariants" - ], - "properties": { - "units": { - "items": { - "$ref": "#/$defs/runUnit" - } - } - } - }, - "else": { - "properties": { - "run_key": false, - "profile": false, - "varying": false, - "baseline": false, - "contender": false, - "sides": false, - "pairing": false, - "failed_invariants": false, - "units": { - "items": { - "$ref": "#/$defs/historyUnit" - } - } - } - } - } - ], "$defs": { "method": { - "description": "Comparison method: a stable name and version, the effect measure it reports, and its complete configuration, such as k and minimum evidence.", + "description": "Comparison method: a stable name and version and its complete configuration, such as k and minimum evidence.", "type": "object", "additionalProperties": false, "required": [ "name", "version", - "effect_measure", "configuration" ], "properties": { @@ -186,185 +133,11 @@ "version": { "$ref": "#/$defs/token" }, - "effect_measure": { - "description": "How effect is measured from the reference estimate, such as relative, absolute, ratio, or log-ratio.", - "$ref": "#/$defs/token" - }, "configuration": { "type": "object" } } }, - "outcome": { - "description": "What one evaluation found. Shared by both unit kinds.", - "type": "object", - "required": [ - "workload", - "quantity", - "verdict" - ], - "properties": { - "workload": { - "$ref": "#/$defs/workload" - }, - "quantity": { - "$ref": "#/$defs/token" - }, - "reference_estimate": { - "description": "Expected value from the reference points or the baseline side.", - "type": "number" - }, - "effect": { - "description": "Change from the reference estimate, in the method's effect measure.", - "type": "number" - }, - "noise": { - "description": "Variation on the effect's scale. Absent for a deterministic quantity.", - "type": "number", - "minimum": 0 - }, - "noise_source": { - "description": "Where the noise came from, such as reference-points or paired-rounds.", - "$ref": "#/$defs/token" - }, - "provisional": { - "description": "True when the noise came from a provisional rule rather than evidence.", - "type": "boolean" - }, - "threshold": { - "description": "The effect magnitude the method required to indicate a change.", - "type": "number", - "minimum": 0 - }, - "verdict": { - "enum": [ - "improved", - "regressed", - "changed", - "no-change-detected", - "indeterminate" - ] - }, - "reason": { - "description": "Why the verdict is indeterminate, such as insufficient-history or duplicate-points.", - "$ref": "#/$defs/token" - }, - "unmatched_differences": { - "description": "Coordinates the projection does not match, by JSON Pointer, whose values differ between target and reference.", - "type": "array", - "uniqueItems": true, - "items": { - "$ref": "#/$defs/token" - } - }, - "caveats": { - "description": "Quality caveats the reader should see.", - "type": "array", - "items": { - "$ref": "#/$defs/nonEmptyString" - } - } - }, - "if": { - "properties": { - "verdict": { - "const": "indeterminate" - } - } - }, - "then": { - "required": [ - "reason" - ] - }, - "else": { - "required": [ - "reference_estimate", - "effect" - ] - } - }, - "runUnit": { - "description": "One workload variant and quantity, its two sides paired by round.", - "allOf": [ - { - "$ref": "#/$defs/outcome" - } - ], - "unevaluatedProperties": false, - "required": [ - "rounds" - ], - "properties": { - "rounds": { - "description": "Paired rounds used as evidence.", - "type": "integer", - "minimum": 0 - }, - "series": { - "description": "The series each side's results belong to, when the store assigns them one. Run mode reads no history from them.", - "type": "object", - "additionalProperties": false, - "required": [ - "baseline", - "contender" - ], - "properties": { - "baseline": { - "$ref": "#/$defs/sha256" - }, - "contender": { - "$ref": "#/$defs/sha256" - } - } - } - } - }, - "historyUnit": { - "description": "One target series point against preceding points of its series.", - "allOf": [ - { - "$ref": "#/$defs/outcome" - } - ], - "unevaluatedProperties": false, - "required": [ - "series", - "target", - "reference" - ], - "properties": { - "series": { - "description": "Series fingerprint of the target and every reference point.", - "$ref": "#/$defs/sha256" - }, - "target": { - "$ref": "#/$defs/resultRef" - }, - "reference": { - "description": "The reference points the reference estimate was formed from, oldest first.", - "type": "array", - "items": { - "$ref": "#/$defs/resultRef" - } - }, - "ordering": { - "description": "How the reference points were ordered before the target. Absent when no order was available.", - "enum": [ - "ancestry", - "native-order", - "native-time" - ] - }, - "boundaries": { - "description": "Resets, change points, and continuity joins that truncated or joined the reference.", - "type": "array", - "items": { - "$ref": "#/$defs/boundary" - } - } - } - }, "boundary": { "type": "object", "additionalProperties": false, @@ -470,14 +243,11 @@ "$ref": "#/$defs/token" }, "side": { - "description": "Run mode: the side with no result.", - "enum": [ - "baseline", - "contender" - ] + "description": "The side with no result, as the selector names it, such as baseline or target.", + "$ref": "#/$defs/token" }, "series": { - "description": "History mode: the expected series.", + "description": "The expected series, when the request named it.", "$ref": "#/$defs/sha256" } } @@ -615,6 +385,191 @@ "nonEmptyString": { "type": "string", "minLength": 1 + }, + "selector": { + "description": "How the evidence was formed: a versioned selector and the parameters it was given, with anything the caller abbreviated resolved, so one comparison has one document however it was asked for.", + "type": "object", + "additionalProperties": false, + "required": [ + "name", + "version", + "params" + ], + "properties": { + "name": { + "$ref": "#/$defs/token" + }, + "version": { + "$ref": "#/$defs/token" + }, + "params": { + "type": "object" + } + } + }, + "methodRef": { + "description": "The method that wrote an outcome. Its configuration is in the document's method.", + "type": "object", + "additionalProperties": false, + "required": [ + "name", + "version" + ], + "properties": { + "name": { + "$ref": "#/$defs/token" + }, + "version": { + "$ref": "#/$defs/token" + } + } + }, + "evidence": { + "description": "What a history verdict rests on: the target, the reference it was compared with, how the reference was ordered, and what truncated or joined it.", + "type": "object", + "additionalProperties": false, + "properties": { + "target": { + "description": "The results being evaluated, when the selector distinguishes them from a reference.", + "type": "array", + "minItems": 1, + "items": { + "$ref": "#/$defs/resultRef" + } + }, + "reference": { + "description": "The reference points the reference estimate was formed from, oldest first.", + "type": "array", + "items": { + "$ref": "#/$defs/resultRef" + } + }, + "ordering": { + "description": "How the reference points were ordered before the target. Absent when no order was available.", + "enum": [ + "ancestry", + "native-order", + "native-time" + ] + }, + "boundaries": { + "description": "Resets, change points, and continuity joins that truncated or joined the reference.", + "type": "array", + "items": { + "$ref": "#/$defs/boundary" + } + } + } + }, + "unit": { + "description": "One series and the method's assessment of it.", + "type": "object", + "additionalProperties": false, + "required": [ + "workload", + "quantity", + "series", + "method", + "verdict", + "used", + "excluded", + "details" + ], + "properties": { + "workload": { + "$ref": "#/$defs/workload" + }, + "quantity": { + "$ref": "#/$defs/token" + }, + "series": { + "description": "Series fingerprint under the projection the selector grouped by.", + "$ref": "#/$defs/sha256" + }, + "evidence": { + "$ref": "#/$defs/evidence" + }, + "method": { + "$ref": "#/$defs/methodRef" + }, + "verdict": { + "enum": [ + "improved", + "regressed", + "changed", + "no-change-detected", + "equivalent", + "indeterminate" + ] + }, + "reason": { + "description": "Why the verdict is indeterminate, such as insufficient-rounds or insufficient-history.", + "$ref": "#/$defs/token" + }, + "effect": { + "description": "The headline change, when the method reports one.", + "type": "object", + "additionalProperties": false, + "required": [ + "measure", + "value" + ], + "properties": { + "measure": { + "description": "The method's effect measure, such as relative, difference, or log-ratio.", + "$ref": "#/$defs/token" + }, + "value": { + "type": "number" + } + } + }, + "used": { + "description": "The results the verdict rests on.", + "type": "array", + "items": { + "$ref": "#/$defs/resultRef" + } + }, + "excluded": { + "description": "The unit's other results, each with the reason.", + "type": "array", + "items": { + "$ref": "#/$defs/exclusion" + } + }, + "details": { + "description": "Whatever else the method reports, such as what it computed as noise; validated by the method's own schema.", + "type": "object" + }, + "unmatched_differences": { + "description": "Coordinates the projection does not match, by JSON Pointer, whose values differ between target and reference.", + "type": "array", + "uniqueItems": true, + "items": { + "$ref": "#/$defs/token" + } + }, + "caveats": { + "description": "Quality caveats the reader should see.", + "type": "array", + "items": { + "$ref": "#/$defs/nonEmptyString" + } + } + }, + "if": { + "properties": { + "verdict": { + "const": "indeterminate" + } + } + }, + "then": { + "required": [ + "reason" + ] + } } } } diff --git a/tests/mann_whitney_prototype.py b/tests/mann_whitney_prototype.py new file mode 100644 index 0000000..dc914aa --- /dev/null +++ b/tests/mann_whitney_prototype.py @@ -0,0 +1,117 @@ +"""PROTOTYPE, not in the package: benchx/mann-whitney/v1 (comparator.md §4.5). + +An unpaired rank test on attempt estimates. It asks nothing of the run's schedule +or pairing, and is kept here to show that a second method is one module: the +framework runs it through `compare.evaluate` without knowing it exists. Exact +p-values only, so it is for small samples. +""" + +import statistics +from math import comb + +from benchx.evidence import Assessment, Method, change_label + + +def _doubled_midranks(pooled: list[float]) -> list[int]: + """Twice each value's midrank (1-based), so that tied ranks stay integers.""" + order = sorted(range(len(pooled)), key=pooled.__getitem__) + ranks = [0] * len(pooled) + i = 0 + while i < len(order): + j = i + while j + 1 < len(order) and pooled[order[j + 1]] == pooled[order[i]]: + j += 1 + for position in order[i:j + 1]: + ranks[position] = i + j + 2 # (i + 1 + j + 1) / 2, doubled + i = j + 1 + return ranks + + +def _exact_p(ranks: list[int], observed: int, m: int) -> float: + """The two-sided p-value over every way to choose `m` of the pooled ranks: the + share of choices whose rank sum is at least as far from its mean as `observed`. + Counts by rank sum, so ties need no special handling.""" + counts = [{} for _ in range(m + 1)] + counts[0][0] = 1 + for rank in ranks: + for chosen in range(m - 1, -1, -1): + for total, ways in counts[chosen].items(): + counts[chosen + 1][total + rank] = counts[chosen + 1].get(total + rank, 0) + ways + center = m * (len(ranks) + 1) # the mean of a doubled rank sum, in the same units + extreme = sum(ways for total, ways in counts[m].items() if abs(total - center) >= abs(observed - center)) + return extreme / comb(len(ranks), m) + + +def rank_test(reference: list[float], target: list[float]) -> tuple[float, float, str]: + """(U of the target, exact two-sided p-value).""" + n, m = len(reference), len(target) + ranks = _doubled_midranks([*reference, *target]) + observed = sum(ranks[n:]) + return (observed - m * (m + 1)) / 2, _exact_p(ranks, observed, m) + + +class MannWhitney(Method): + name = "benchx/mann-whitney" + version = "v1" + dims = ("side",) + requires = () + defaults = {"alpha": 0.05} + config_schema = { + "type": "object", + "additionalProperties": False, + "required": ["alpha"], + "properties": {"alpha": {"type": "number", "exclusiveMinimum": 0, "maximum": 1}}, + } + details_schema = { + "type": "object", + "additionalProperties": False, + "required": ["attempts"], + "properties": { + "attempts": {"description": "Usable attempts per side, the reference side first", + "type": "array", "items": {"type": "integer", "minimum": 0}, "minItems": 2, "maxItems": 2}, + "smallest_p": {"description": "The smallest two-sided p-value this many attempts can reach", + "type": "number", "minimum": 0, "maximum": 1}, + "reference_estimate": {"description": "Median of the reference side's estimates", "type": "number"}, + "u": {"description": "Mann-Whitney U of the target side", "type": "number", "minimum": 0}, + "p_value": {"type": "number", "minimum": 0, "maximum": 1}, + }, + } + + def assess(self, layout, evidence, config): + reference, target = ([row for row in side if row.estimate is not None] for side in layout.cells) + used = (*reference, *target) + n, m = len(reference), len(target) + excluded = tuple((row, "no-estimate") for side in layout.cells for row in side if row.estimate is None) + details = {"attempts": [n, m]} + + def indeterminate(reason): + return Assessment("indeterminate", reason=reason, used=used, excluded=excluded, details=details) + + if evidence.no_verdict: + return indeterminate(evidence.no_verdict) + if evidence.quantity.get("deterministic"): + return indeterminate("deterministic-deferred") + if not n or not m: + return indeterminate("insufficient-attempts") + details["smallest_p"] = min(1.0, 2 / comb(n + m, n)) + before = [row.estimate for row in reference] + after = [row.estimate for row in target] + center = statistics.median(before) + details["reference_estimate"] = center + # A design that cannot reach alpha would report no change whatever it measured. + if details["smallest_p"] > config["alpha"]: + return indeterminate("insufficient-attempts") + u, p = rank_test(before, after) + details.update(u=u, p_value=p) + verdict = (change_label(u - n * m / 2, evidence.quantity.get("direction")) if p <= config["alpha"] + else "no-change-detected") + effect = None if center == 0 else {"measure": "relative-median", "value": statistics.median(after) / center - 1} + return Assessment(verdict, effect=effect, used=used, excluded=excluded, details=details) + + def describe(self, effect, details): + shown = f"{effect['value']:+8.2%}" if effect else " ?" + p = f"{details['p_value']:7.4f}" if "p_value" in details else " ?" + return f"effect {shown} p {p} attempts={details['attempts'][0]}+{details['attempts'][1]}" + + +METHOD = MannWhitney() diff --git a/tests/test_build_import_path.py b/tests/test_build_import_path.py index c27039a..35dcaf9 100644 --- a/tests/test_build_import_path.py +++ b/tests/test_build_import_path.py @@ -91,7 +91,7 @@ def test_the_build_root_imports_nothing(tmp, toy, monkeypatch): monkeypatch.delenv("PYTHONPATH", raising=False) doc = compare(tmp, toy, monkeypatch, reported="build") unit = doc["units"][0] - assert unit["rounds"] == 0 + assert unit["details"]["rounds"] == 0 assert unit["verdict"] == "indeterminate" assert doc["exclusions"] and {e["reason"] for e in doc["exclusions"]} == {"error: harness.error"} @@ -106,8 +106,8 @@ def test_one_pythonpath_measures_one_build_on_both_sides(tmp, toy, monkeypatch): monkeypatch.setenv("PYTHONPATH", str(toy["sides"]["base"]["package"])) doc = compare(tmp, toy, monkeypatch, reported="build") unit = doc["units"][0] - assert unit["rounds"] == 3 - assert abs(unit["effect"]) < 1 # a real 100-fold difference is an effect of about 100 + assert unit["details"]["rounds"] == 3 + assert abs(unit["effect"]["value"]) < 1 # a real 100-fold difference is an effect of about 100 def test_the_package_directory_as_target_imports_each_build(tmp, toy, monkeypatch): @@ -117,6 +117,6 @@ def test_the_package_directory_as_target_imports_each_build(tmp, toy, monkeypatc monkeypatch.delenv("PYTHONPATH", raising=False) doc = compare(tmp, toy, monkeypatch, reported="package") unit = doc["units"][0] - assert unit["rounds"] == 3 + assert unit["details"]["rounds"] == 3 assert unit["verdict"] == "regressed" - assert unit["effect"] > 10 + assert unit["effect"]["value"] > 10 diff --git a/tests/test_comparator.py b/tests/test_comparator.py index d5dcbea..de711be 100644 --- a/tests/test_comparator.py +++ b/tests/test_comparator.py @@ -73,7 +73,7 @@ def test_pairs_by_round_not_by_arrival(): d["procedure"]["round"] = r d["measurement"]["observations"] = [(r + 1) * 1.1] unit = evaluate(docs)["units"][0] - assert unit["effect"] == pytest.approx(0.1) and unit["noise"] == pytest.approx(0, abs=1e-12) + assert unit["effect"]["value"] == pytest.approx(0.1) and unit["details"]["noise"] == pytest.approx(0, abs=1e-12) def test_equal_attempts(): @@ -120,7 +120,7 @@ def test_exclusions(): doc = evaluate(docs) result = {"producer": "benchx/gbench-adapter", "ingest_key": docs[2]["ingest_key"]} assert doc["exclusions"] == [{"result": result, "reason": "error: harness.error"}] - assert doc["units"][0]["rounds"] == 3 + assert doc["units"][0]["details"]["rounds"] == 3 def test_min_rounds(): @@ -136,7 +136,7 @@ def test_one_pair_has_no_noise_whatever_min_rounds_says(min_rounds): of them: with one pair the unit is indeterminate, not a StatisticsError.""" unit = evaluate(run(rounds=1), min_rounds=min_rounds)["units"][0] assert (unit["verdict"], unit["reason"]) == ("indeterminate", "insufficient-rounds") - assert unit["rounds"] == 1 and "noise" not in unit + assert unit["details"]["rounds"] == 1 and "noise" not in unit["details"] def test_no_pair_at_all_is_indeterminate_too(): @@ -145,7 +145,7 @@ def test_no_pair_at_all_is_indeterminate_too(): for d in docs[1::2]: d["measurement"] = {"quantity": d["measurement"]["quantity"], "status": "error", "reason": "harness.error"} unit = evaluate(docs, min_rounds=0)["units"][0] - assert (unit["verdict"], unit["reason"], unit["rounds"]) == ("indeterminate", "insufficient-rounds", 0) + assert (unit["verdict"], unit["reason"], unit["details"]["rounds"]) == ("indeterminate", "insufficient-rounds", 0) def test_two_pairs_are_enough_when_min_rounds_allows(): @@ -210,7 +210,7 @@ def labeled(docs, configurations=("plain", "hardened")): def test_environments_sides_are_named_by_label(): doc = compare.compare(labeled(run()), run_key="run", profile="environments", baseline="plain", label="build") assert core.check(doc, "comparison-document") == [] - assert (doc["varying"], doc["baseline"], doc["contender"]) == ("provenance.labels.build", "plain", "hardened") + assert (doc["sides"]["varying"], doc["sides"]["baseline"], doc["sides"]["contender"]) == ("provenance.labels.build", "plain", "hardened") assert doc["failed_invariants"] == [] and doc["units"][0]["verdict"] == "regressed" assert doc["units"][0]["unmatched_differences"] == ["/coordinates/subject/configuration/hardening"] @@ -387,18 +387,18 @@ def vevaluate(docs, parameter="role", baseline="'baseline'", **kwargs): def test_variants_a_clean_run_has_one_unit_per_benchmark(): doc = vevaluate(variants_run()) - assert invariants(doc) == [] and doc["profile"] == "variants" - assert doc["varying"] == "coordinates.workload.parameters.role" - assert (doc["baseline"], doc["contender"]) == VALUES + assert invariants(doc) == [] and doc["selector"]["params"]["profile"] == "variants" + assert doc["sides"]["varying"] == "coordinates.workload.parameters.role" + assert (doc["sides"]["baseline"], doc["sides"]["contender"]) == VALUES [unit] = doc["units"] assert unit["workload"] == {"name": "bench_truncate"} # the varied parameter is not part of the unit - assert unit["verdict"] == "regressed" and unit["effect"] == pytest.approx(0.1) + assert unit["verdict"] == "regressed" and unit["effect"]["value"] == pytest.approx(0.1) assert doc["local_only"] is False def test_variants_the_baseline_can_be_either_value(): doc = vevaluate(variants_run(), baseline="'variant'") - assert invariants(doc) == [] and (doc["baseline"], doc["contender"]) == VALUES[::-1] + assert invariants(doc) == [] and (doc["sides"]["baseline"], doc["sides"]["contender"]) == VALUES[::-1] assert doc["units"][0]["verdict"] == "improved" @@ -415,7 +415,7 @@ def test_variants_another_parameter_makes_one_unit_per_value_of_it(): def test_variants_no_parameter_name_is_special(): doc = vevaluate(variants_run(name="impl"), parameter="impl") - assert invariants(doc) == [] and doc["varying"] == "coordinates.workload.parameters.impl" + assert invariants(doc) == [] and doc["sides"]["varying"] == "coordinates.workload.parameters.impl" def test_variants_a_third_value_is_a_third_side(): @@ -472,7 +472,7 @@ def test_variants_a_failed_attempt_on_either_side_fails_the_unit(status, which): doc = vevaluate(docs) [unit] = doc["units"] assert invariants(doc) == [] and unit["verdict"] == "indeterminate" and unit["reason"] == "side-failed" - assert "effect" not in unit and "noise" not in unit and unit["rounds"] == 3 + assert "effect" not in unit and "noise" not in unit["details"] and unit["details"]["rounds"] == 3 [excluded] = doc["exclusions"] assert excluded["result"]["ingest_key"] == failing["ingest_key"] and excluded["reason"].startswith(status) assert "(side-failed)" in compare.render(doc) @@ -490,7 +490,7 @@ def test_variants_a_skipped_attempt_is_an_ordinary_exclusion(): docs[2]["measurement"] = {"quantity": docs[2]["measurement"]["quantity"], "status": "skipped", "reason": "n/a"} doc = vevaluate(docs) [unit] = doc["units"] - assert unit["verdict"] == "regressed" and unit["rounds"] == 3 and "reason" not in unit + assert unit["verdict"] == "regressed" and unit["details"]["rounds"] == 3 and "reason" not in unit assert len(doc["exclusions"]) == 1 @@ -498,7 +498,7 @@ def test_a_failed_attempt_under_revisions_still_gives_a_verdict_from_the_remaini docs = run(rounds=4, base_values=(1.0, 2.0, 3.0, 4.0)) fail(docs[2], "error") unit = evaluate(docs)["units"][0] - assert unit["verdict"] == "regressed" and unit["rounds"] == 3 and "reason" not in unit + assert unit["verdict"] == "regressed" and unit["details"]["rounds"] == 3 and "reason" not in unit def test_variants_the_sides_must_share_the_subject_tree(): diff --git a/tests/test_compare_harnesses.py b/tests/test_compare_harnesses.py index a218b59..578a183 100644 --- a/tests/test_compare_harnesses.py +++ b/tests/test_compare_harnesses.py @@ -224,8 +224,8 @@ def measurands(store, run_key="v"): def test_variants_run_one_order_per_value_and_round_with_that_values_cases_only(tmp): pytest.importorskip("pyperf") doc, store, said = run_variants(tmp, variants_tree(tmp, SIZED), parameter="role") - assert doc["failed_invariants"] == [] and doc["missing"] == [] and doc["varying"].endswith("parameters.role") - assert (doc["baseline"], doc["contender"]) == ("'baseline'", "'variant'") + assert doc["failed_invariants"] == [] and doc["missing"] == [] and doc["sides"]["varying"].endswith("parameters.role") + assert (doc["sides"]["baseline"], doc["sides"]["contender"]) == ("'baseline'", "'variant'") assert [u["workload"] for u in doc["units"]] == [ {"name": "bench_truncate", "parameters": {"size": "10"}}, {"name": "bench_truncate", "parameters": {"size": "100"}}] @@ -253,12 +253,12 @@ def test_variants_the_baseline_is_the_first_declared_value_and_can_be_swapped(tm pytest.importorskip("pyperf") tree = variants_tree(tmp, VARIANTS) doc, _, said = run_variants(tmp, tree) - assert doc["failed_invariants"] == [] and (doc["baseline"], doc["contender"]) == ("'baseline'", "'variant'") + assert doc["failed_invariants"] == [] and (doc["sides"]["baseline"], doc["sides"]["contender"]) == ("'baseline'", "'variant'") assert "varying: role (the only parameter)" in said - assert doc["units"][0]["workload"] == {"name": "bench_truncate"} and doc["units"][0]["effect"] > 0 + assert doc["units"][0]["workload"] == {"name": "bench_truncate"} and doc["units"][0]["effect"]["value"] > 0 swapped, _, _ = run_variants(tmp, tree, baseline="'variant'", out=tmp / "out2", run_key="v2") - assert swapped["failed_invariants"] == [] and (swapped["baseline"], swapped["contender"]) == ("'variant'", "'baseline'") - assert swapped["units"][0]["effect"] < 0 + assert swapped["failed_invariants"] == [] and (swapped["sides"]["baseline"], swapped["sides"]["contender"]) == ("'variant'", "'baseline'") + assert swapped["units"][0]["effect"]["value"] < 0 # Slot 0 of the swapped run is the variant. assert next(d for d in measurands(Store(tmp / "s.parquet"), "v2") if d["procedure"]["slot"] == 0)[ "coordinates"]["workload"]["parameters"] == {"role": "'variant'"} diff --git a/tests/test_e2e_numpy_revisions.py b/tests/test_e2e_numpy_revisions.py index a74b441..3ce2c56 100644 --- a/tests/test_e2e_numpy_revisions.py +++ b/tests/test_e2e_numpy_revisions.py @@ -154,7 +154,7 @@ def build_of(worktrees, ref): def check(doc, probed, worktrees, *, local_only=False): assert doc["failed_invariants"] == [] and doc["missing"] == [] assert doc["local_only"] is local_only # two tags, clean trees, unless a checkout was made dirty - assert [u["rounds"] for u in doc["units"]] == [3] + assert [u["details"]["rounds"] for u in doc["units"]] == [3] expected = {ref.removeprefix("v"): build_of(worktrees, ref) for ref in REVISIONS} seen = {} for line in probed: diff --git a/tests/test_evidence.py b/tests/test_evidence.py new file mode 100644 index 0000000..46e5acc --- /dev/null +++ b/tests/test_evidence.py @@ -0,0 +1,328 @@ +"""The framework between selectors and methods (comparator.md §2.6, §3, §4): what +counting shows, the layout a method asked for, and who owns which check.""" + +import copy +import json + +import pytest + +from benchx import compare, core, methods +from benchx.core import catalog +from benchx.evidence import Assessment, Evidence, Method, NotApplicable, Row, pivot, relation, slot_schedule +from benchx.key import Key +from test_comparator import BASE, HEAD, evaluate, invariants, labeled, result, run + + +def row(side, round_=None, estimate=1.0, name=None, exclusion=None): + name = name or f"{side}-{round_}-{estimate}" + return Row({"producer": "p", "ingest_key": name}, side, {"round": round_}, estimate, (estimate,), exclusion) + + +def evidence(*rows): + return Evidence("f" * 64, {"name": "w"}, {"name": "wall-time"}, ("baseline", "contender"), tuple(rows)) + + +# --- keys --------------------------------------------------------------------- + +def test_a_key_reads_normalizes_and_orders(): + key = Key("/source/uri", normalize={"rule": "canonical-uri"}) + assert key.read({"source": {"uri": "https://Example.org/demo.git/"}}) == "https://example.org/demo" + assert key.read({}) is None + assert sorted(["6.10", "6.8"], key=Key("/k").sort_key) == ["6.8", "6.10"] + + +# --- what counting shows ------------------------------------------------------ + +def test_a_key_on_both_sides_is_crossed_and_one_side_only_is_nested(): + assert relation([row("baseline", 0), row("contender", 0)], "round") == "crossed" + assert relation([row("baseline", 0), row("contender", 1)], "round") == "nested" + assert relation([row("baseline"), row("contender")], "round") == "absent" + + +def test_a_failed_attempt_still_shows_the_design(): + """Every contender attempt failed: the rounds were still crossed, and the + method sees masked cells rather than a run that was never blocked.""" + rows = [row("baseline", 0), row("contender", 0, estimate=None, exclusion="error")] + assert relation(rows, "round") == "crossed" + assert pivot(evidence(*rows), ("round", "side")).cells == [[(rows[0],), ()]] + + +@pytest.mark.parametrize("schedule, expected", [ + ([(0, "a"), (1, "b"), (2, "a"), (3, "b")], "alternating"), + ([(0, "a"), (1, "a"), (2, "b"), (3, "b")], "not-alternating"), + ([(0, "a"), (0, "b")], "not-alternating"), # one slot, two sides + ([(0, "a"), (None, "b")], "absent"), +]) +def test_slot_schedule(schedule, expected): + assert slot_schedule(schedule) == expected + + +# --- the layout --------------------------------------------------------------- + +def test_a_layout_is_over_the_dimensions_a_method_names(): + a0, b0, a1 = row("baseline", 0), row("contender", 0), row("baseline", 1) + layout = pivot(evidence(a0, b0, a1), ("round", "side")) + assert layout.coords == {"round": (0, 1), "side": ("baseline", "contender")} + assert layout.cells == [[(a0,), (b0,)], [(a1,), ()]] + + +def test_rounds_order_by_value_not_as_strings(): + rows = [row(side, r) for r in (10, 2, 1) for side in ("baseline", "contender")] + assert pivot(evidence(*rows), ("round", "side")).coords["round"] == (1, 2, 10) + + +def test_one_dimension_holds_every_attempt_of_a_side(): + a0, a1, b0 = row("baseline", 0), row("baseline", 1), row("contender", 0) + assert pivot(evidence(a0, a1, b0), ("side",)).cells == [(a0, a1), (b0,)] + + +@pytest.mark.parametrize("rows, match", [ + ([row("baseline"), row("contender")], "no result has one"), + ([row("baseline", 0), row("contender", 1)], "no value of round appears on more than one side"), +]) +def test_a_unit_that_cannot_be_laid_out_says_why(rows, match): + with pytest.raises(NotApplicable, match=match): + pivot(evidence(*rows), ("round", "side")) + + +# --- units are series (decision 7) -------------------------------------------- + +def test_sides_that_disagree_are_two_series_and_refused(): + docs = run() + for d in docs[1::2]: + d["coordinates"]["subject"]["configuration"] = {"build_type": "Debug"} + doc = evaluate(docs) + assert invariants(doc) == ["sides-agree"] and doc["missing"] == [] and doc["units"] == [] + + +def test_a_variant_on_one_side_is_one_series_and_merely_missing(): + """The same two one-sided groups as a disagreement, by shape: what differs is + that they are one workload variant or two.""" + docs = run() + for slot, (side, value) in enumerate(((BASE, 1.0), (HEAD, 1.1)), start=6): + docs.append(result(side, 3, slot, value, workload=f"BM_only_{slot}")) + doc = evaluate(docs) + assert invariants(doc) == [] + assert {(m["workload"]["name"], m["side"]) for m in doc["missing"]} == {("BM_only_6", "contender"), + ("BM_only_7", "baseline")} + + +def test_one_unit_has_one_series(): + unit = evaluate(run())["units"][0] + assert len(unit["series"]) == 64 + + +# --- who owns which check (decision 6) ---------------------------------------- + +def test_a_method_owns_what_its_statistics_assume(): + docs = run() + docs[1]["procedure"]["slot"], docs[2]["procedure"]["slot"] = 2, 1 # a, a, b, b, a, b + doc = evaluate(docs) + assert invariants(doc) == ["alternating-slots"] + + +def test_alternation_is_judged_over_the_run_not_the_unit(): + """The run alternates. BM_Partial alone reads a, b, b, a, because it skipped + two attempts, and that says nothing about the schedule.""" + docs, slot = [], 0 + for r in range(3): + for side, value in ((BASE, 1.0 + r), (HEAD, (1.0 + r) * 1.1)): + docs.append(result(side, r, slot, value)) + if (r, side) not in ((1, BASE), (2, HEAD)): + docs.append(result(side, r, slot, value, workload="BM_Partial")) + slot += 1 + doc = evaluate(docs) + assert invariants(doc) == [] and {u["workload"]["name"] for u in doc["units"]} == {"BM_W", "BM_Partial"} + + +def test_two_usable_results_in_a_cell_are_refused_not_overwritten(): + """Both sides retried round 0. The old comparator kept whichever sorted last, + so a base of 100 silently replaced 1.0 and turned +10% into -26%.""" + docs = run() + retries = [result(BASE, 0, 6, 100.0), result(HEAD, 0, 7, 1.1)] + for d, name in zip(retries, ("retry-a", "retry-b")): + d["ingest_key"] = name + doc = evaluate(docs + retries) + assert invariants(doc) == ["duplicate-points"] and doc["units"] == [] + assert doc["failed_invariants"][0]["workload"] == {"name": "BM_W"} + + +def test_a_failed_attempt_and_its_retry_are_one_usable_result_in_the_cell(): + """Schema §5.3: a retry keeps its round, so that pairing survives.""" + docs = run() + failed = copy.deepcopy(docs[:2]) + for d in failed: + d["ingest_key"] += ":failed" + d["measurement"] = {"quantity": d["measurement"]["quantity"], "status": "error", "reason": "harness.error"} + doc = evaluate(docs + failed) + assert invariants(doc) == [] and doc["units"][0]["details"]["rounds"] == 3 + + +@pytest.mark.parametrize("change, detail", [ + (lambda d: d["procedure"].pop("round"), "no result has one"), + (lambda d: d["procedure"].update(round=d["procedure"]["round"] + 10 * (d["revision"]["key"] == HEAD)), + "no value of round appears on more than one side"), +]) +def test_a_method_that_does_not_apply_says_so_rather_than_counting_no_rounds(change, detail): + docs = run() + for d in docs: + change(d) + doc = evaluate(docs) + assert invariants(doc) == ["layout"] and doc["units"] == [] + assert detail in doc["failed_invariants"][0]["detail"] + + +def test_results_with_no_round_are_not_paired_with_each_other(): + """The old comparator treated a missing round as a round named None.""" + docs = run(rounds=4, base_values=(1.0, 2.0, 3.0, 4.0)) + docs[0]["procedure"].pop("round") + docs[1]["procedure"].pop("round") + unit = evaluate(docs)["units"][0] + assert unit["details"]["rounds"] == 3 + assert {e["reason"] for e in unit["excluded"]} == {"no round"} and len(unit["excluded"]) == 2 + + +# --- the outcome --------------------------------------------------------------- + +def test_every_result_is_used_or_excluded_exactly_once(): + docs = run(rounds=4, base_values=(1.0, 2.0, 3.0, 4.0)) + docs[2]["measurement"] = {"quantity": docs[2]["measurement"]["quantity"], "status": "error", "reason": "x"} + unit = evaluate(docs)["units"][0] + used = {u["ingest_key"] for u in unit["used"]} + excluded = {e["result"]["ingest_key"]: e["reason"] for e in unit["excluded"]} + assert not used & set(excluded) and used | set(excluded) == {d["ingest_key"] for d in docs} + assert excluded == {docs[2]["ingest_key"]: "error: x", docs[3]["ingest_key"]: "unpaired"} + + +def test_the_noise_says_what_it_is_and_records_its_degrees_of_freedom(): + details = evaluate(run(rounds=5, base_values=(1.0, 2.0, 3.0, 4.0, 5.0)))["units"][0]["details"] + assert details["noise_kind"] == "standard-error" and details["degrees_of_freedom"] == 4 + assert details["threshold"] == pytest.approx(3.0 * details["noise"]) + + +def test_the_method_and_selector_are_recorded_even_with_no_units(): + docs = run() + docs.append(result("c" * 40, 3, 6, 1.0)) + doc = evaluate(docs) + assert doc["units"] == [] and doc["method"]["configuration"] == {"k": 3.0, "min_rounds": 3} + assert doc["selector"]["params"] == {"run_key": "run", "profile": "revisions", "baseline": BASE} + + +def test_one_comparison_has_one_document_however_the_baseline_was_abbreviated(): + full = compare.compare(run(), run_key="run", profile="revisions", baseline=BASE) + assert compare.compare(run(), run_key="run", profile="revisions", baseline=BASE[:6]) == full + + +def test_the_same_inputs_give_the_same_document_in_any_order(): + docs = run() + assert evaluate(docs) == evaluate(list(reversed(copy.deepcopy(docs)))) + + +def test_labels_reach_the_selector_params(): + doc = compare.compare(labeled(run()), run_key="run", profile="environments", baseline="plain", label="build") + assert doc["selector"]["params"]["label"] == "build" + + +# --- a method's own schemas ----------------------------------------------------- + +def test_an_unknown_method_is_refused(): + with pytest.raises(compare.CompareError, match="not in the implementation"): + compare.compare(run(), run_key="run", profile="revisions", baseline=BASE, method="nobody/v1") + + +def test_configuration_is_checked_against_the_methods_schema(): + with pytest.raises(compare.CompareError, match="alpha"): + compare.compare(run(), run_key="run", profile="revisions", baseline=BASE, alpha=0.05) + with pytest.raises(compare.CompareError, match="benchx/paired-relative/v1"): + compare.compare(run(), run_key="run", profile="revisions", baseline=BASE, k="three") + + +class Careless(Method): + name, version, dims = "test/careless", "v1", ("side",) + details_schema = {"type": "object", "additionalProperties": False} + + def assess(self, layout, evidence, config): + return Assessment("no-change-detected", details={"undeclared": 1}) + + +def test_details_are_checked_against_the_methods_schema(): + from jsonschema import ValidationError + + from benchx.run_profile import RUN_PROFILE + selection = RUN_PROFILE.select(run(), run_key="run", profile="revisions", baseline=BASE) + with pytest.raises(ValidationError, match="undeclared"): + compare.evaluate(selection, Careless(), {}) + + +class TakesNothing(Method): + """Uses nothing and says nothing: the framework still accounts for every result.""" + name, version, dims = "test/takes-nothing", "v1", ("side",) + + def assess(self, layout, evidence, config): + return Assessment("indeterminate", reason="declined") + + +def test_a_result_a_method_does_not_mention_is_still_accounted_for(): + from benchx.run_profile import RUN_PROFILE + selection = RUN_PROFILE.select(run(), run_key="run", profile="revisions", baseline=BASE) + doc = compare.evaluate(selection, TakesNothing(), {}) + assert core.check(doc, "comparison-document") == [] + unit = doc["units"][0] + assert unit["used"] == [] and {e["reason"] for e in unit["excluded"]} == {"not-used"} and len(unit["excluded"]) == 6 + + +def test_the_default_method_is_registered(): + assert methods.DEFAULT in methods.METHODS and catalog.COMPARISON_DOCUMENT.endswith("/0.1.0") + + +# --- a selector's no-verdict rule (UC-02 §10) holds for every method ---------------------------- + +def _variants_with_a_failed_attempt(): + from test_comparator import fail, variants_run + docs = variants_run(rounds=5, base_values=(1.0, 2.0, 3.0, 4.0, 5.0), ratio=1.5) + fail(docs[2], "error") + return docs + + +@pytest.mark.parametrize("method", sorted(methods.METHODS)) +def test_a_failed_attempt_under_variants_leaves_no_verdict_whatever_the_method(method): + doc = compare.compare(_variants_with_a_failed_attempt(), run_key="run", profile="variants", + baseline="'baseline'", parameter="role", method=method) + assert core.check(doc, "comparison-document") == [] + [unit] = doc["units"] + assert (unit["verdict"], unit["reason"]) == ("indeterminate", "side-failed") and "effect" not in unit + + +class Ignores(Method): + """Gives a verdict whatever the selector says.""" + name, version, dims = "test/ignores", "v1", ("side",) + + def assess(self, layout, evidence, config): + return Assessment("regressed", effect={"measure": "relative", "value": 0.5}) + + +def test_a_method_that_ignores_the_selectors_rule_is_a_bug_not_a_verdict(): + from benchx.run_profile import RUN_PROFILE + selection = RUN_PROFILE.select(_variants_with_a_failed_attempt(), run_key="run", profile="variants", + baseline="'baseline'", parameter="role") + with pytest.raises(AssertionError, match="side-failed"): + compare.evaluate(selection, Ignores(), {}) + + +def test_the_varied_parameter_is_recorded_with_the_selector(): + from test_comparator import variants_run + doc = compare.compare(variants_run(), run_key="run", profile="variants", baseline="'baseline'", parameter="role") + assert doc["selector"]["params"] == {"run_key": "run", "profile": "variants", "baseline": "'baseline'", + "parameter": "role"} + + +def test_a_document_whose_selector_names_no_sides_renders(): + """History mode's sides are its own (reference and target), so its document has no `sides`.""" + import dataclasses + + from benchx.run_profile import RUN_PROFILE + selection = RUN_PROFILE.select(run(), run_key="run", profile="revisions", baseline=BASE) + doc = compare.evaluate(dataclasses.replace(selection, sides=None), methods.get(methods.DEFAULT), + {"k": 3.0, "min_rounds": 3}) + assert core.check(doc, "comparison-document") == [] and "sides" not in doc + assert compare.render(doc).splitlines()[0].startswith("benchx/run-profile run_key run profile revisions") diff --git a/tests/test_mann_whitney.py b/tests/test_mann_whitney.py new file mode 100644 index 0000000..8d215f5 --- /dev/null +++ b/tests/test_mann_whitney.py @@ -0,0 +1,42 @@ +"""A second method lands as one module (comparator.md §4.5): the prototype in +mann_whitney_prototype.py asks for other dimensions than the default and nothing of +the run's schedule, and the framework runs it unchanged.""" + +import pytest + +from benchx import compare, core +from benchx.run_profile import RUN_PROFILE +from mann_whitney_prototype import METHOD, rank_test +from test_comparator import BASE, HEAD, invariants, result, run + +VALUES = (1.0, 1.02, 0.98, 1.01, 0.99) + + +def evaluate(docs, **config): + selection = RUN_PROFILE.select(docs, run_key="run", profile="revisions", baseline=BASE) + doc = compare.evaluate(selection, METHOD, compare._configure(METHOD, config)) + assert core.check(doc, "comparison-document") == [] + return doc + + +def test_a_run_that_did_not_alternate_is_refused_by_the_default_and_compared_by_this_one(): + docs = [result(BASE, r, r, VALUES[r]) for r in range(4)] + [result(HEAD, r, 4 + r, VALUES[r] * 1.5) for r in range(4)] + assert invariants(compare.compare(docs, run_key="run", profile="revisions", baseline=BASE)) == ["alternating-slots"] + unit = evaluate(docs)["units"][0] + assert unit["verdict"] == "regressed" and unit["details"]["p_value"] == pytest.approx(2 / 70) + assert unit["effect"] == {"measure": "relative-median", "value": pytest.approx(0.5)} + assert "sides-did-not-alternate" in unit["caveats"] + + +def test_a_design_that_cannot_reach_alpha_is_indeterminate(): + """Three attempts per side: the smallest two-sided p-value is 0.1.""" + unit = evaluate(run(rounds=3, base_values=VALUES[:3], ratio=1.5))["units"][0] + assert (unit["verdict"], unit["reason"]) == ("indeterminate", "insufficient-attempts") + + +@pytest.mark.parametrize("a, b, u, p", [ + ([1, 2, 3, 4], [5, 6, 7, 8], 16.0, 0.02857142857142857), # scipy.stats.mannwhitneyu(b, a, method="exact") + ([1, 2, 2, 3], [2, 3, 3, 4], 13.0, 0.2857142857142857), # brute force over midranks, with ties +]) +def test_exact_p_values(a, b, u, p): + assert rank_test(a, b) == (u, pytest.approx(p)) diff --git a/tests/test_prototype.py b/tests/test_prototype.py index 204d77d..a9bb2aa 100644 --- a/tests/test_prototype.py +++ b/tests/test_prototype.py @@ -83,7 +83,7 @@ def test_revisions_profile(revisions_run, repo, tmp): # criterion 5: +2% on a quiet benchmark flags, +3% on a noisy one does not assert verdicts(doc) == {QUIET: "regressed", NOISY: "no-change-detected"} assert doc["local_only"] is False - assert all(u["rounds"] == 6 for u in doc["units"]) + assert all(u["details"]["rounds"] == 6 for u in doc["units"]) def test_environments_profile(environments_run, tmp): @@ -91,7 +91,7 @@ def test_environments_profile(environments_run, tmp): store.ingest_paths([environments_run]) doc = compare.compare(store.documents("env-1"), run_key="env-1", profile="environments", label="build", baseline="plain") - assert doc["varying"] == "provenance.labels.build" + assert doc["sides"]["varying"] == "provenance.labels.build" assert verdicts(doc) == {QUIET: "regressed", NOISY: "no-change-detected"} diff --git a/tests/test_session.py b/tests/test_session.py index afda316..f6fc403 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -45,7 +45,7 @@ def test_session_environments(repo, tmp): profile="environments", adapter="google-benchmark", suite="demo-bench", rounds=6, label=("build", "plain", "hardened"), project="demo", store=Store(tmp / "store.parquet"), run_key="e1", out=tmp / "out") - assert doc["varying"] == "provenance.labels.build" and doc["failed_invariants"] == [] + assert doc["sides"]["varying"] == "provenance.labels.build" and doc["failed_invariants"] == [] assert verdicts(doc) == {QUIET: "regressed", NOISY: "no-change-detected"} @@ -127,7 +127,7 @@ def test_measure_mode_equals_read_mode(repo, tmp, builds, capsys): measured = json.loads(capsys.readouterr().out) assert cli.main(["compare", "--run", "cli-1", *common, "--baseline", repo["base"][:10], "--json"]) == cli.EXIT_OK assert json.loads(capsys.readouterr().out) == measured - assert measured["mode"] == "run" and verdicts(measured)[QUIET] == "regressed" + assert measured["selector"]["name"] == "benchx/run-profile" and verdicts(measured)[QUIET] == "regressed" # criterion 8: the output directory, orders subdirectory and all, compares to the same document assert cli.main(["compare", "--run", "cli-1", *common, "--baseline", repo["base"][:10], "--json", "--results", str(tmp / "o")]) == cli.EXIT_OK diff --git a/tests/test_target.py b/tests/test_target.py index 78ba474..da320a4 100644 --- a/tests/test_target.py +++ b/tests/test_target.py @@ -131,8 +131,8 @@ def test_declared_text_reaches_orders_results_and_the_comparison(repo, tmp, prov assert "activation" not in results["plain"]["provenance"]["info"] assert core.check(doc, "comparison-document") == [] - assert doc["sides"]["baseline"] == {"how_built": "cmake plain", "target_provider": info["target_provider"]} - assert doc["sides"]["contender"]["activation"] == "source env.sh" + assert doc["sides"]["declared"]["baseline"] == {"how_built": "cmake plain", "target_provider": info["target_provider"]} + assert doc["sides"]["declared"]["contender"]["activation"] == "source env.sh" assert "hardened: activation: source env.sh" in compare.render(doc) @@ -152,7 +152,7 @@ def test_sidecar_route_needs_no_provider(repo, tmp): rounds=3, label=("build", "plain", "hardened"), store=Store(tmp / "store.parquet"), run_key="t3", out=tmp / "out", source_uri=SOURCE_URI) # one side has no description to name it - assert doc["sides"] == {"baseline": {}, "contender": {"how_built": "by hand"}} + assert doc["sides"]["declared"] == {"baseline": {}, "contender": {"how_built": "by hand"}} def test_a_declared_build_is_refused_before_anything_runs(repo, tmp): diff --git a/tests/test_uc02.py b/tests/test_uc02.py index 4d70f3d..3ca3ca7 100644 --- a/tests/test_uc02.py +++ b/tests/test_uc02.py @@ -88,10 +88,10 @@ def test_the_loop_compares_the_two_values(tree, capsys): assert code == cli.EXIT_OK and doc["failed_invariants"] == [] assert core.check(doc, "comparison-document") == [] assert "varying: role (the only parameter)" in err # inferred, and said so - assert doc["varying"] == "coordinates.workload.parameters.role" - assert (doc["baseline"], doc["contender"]) == (BASELINE, VARIANT) + assert doc["sides"]["varying"] == "coordinates.workload.parameters.role" + assert (doc["sides"]["baseline"], doc["sides"]["contender"]) == (BASELINE, VARIANT) [unit] = doc["units"] - assert unit["workload"] == {"name": "bench_truncate"} and unit["rounds"] == 3 and unit["effect"] > 0 + assert unit["workload"] == {"name": "bench_truncate"} and unit["details"]["rounds"] == 3 and unit["effect"]["value"] > 0 assert doc["local_only"] is False # a clean tree: the store and the results are not part of it @@ -99,8 +99,8 @@ def test_the_baseline_can_be_the_other_value(tree, capsys): loop(["mod.py"], by_case([BASELINE, VARIANT]), PYPERF) code, doc, _ = compare(capsys, baseline=VARIANT) assert code == cli.EXIT_OK and doc["failed_invariants"] == [] - assert (doc["baseline"], doc["contender"]) == (VARIANT, BASELINE) - assert doc["units"][0]["effect"] < 0 + assert (doc["sides"]["baseline"], doc["sides"]["contender"]) == (VARIANT, BASELINE) + assert doc["units"][0]["effect"]["value"] < 0 def test_an_unquoted_baseline_is_refused_with_the_recorded_values(tree, capsys): @@ -121,7 +121,7 @@ def test_a_second_parameter_makes_one_unit_per_value_of_it(tmp, monkeypatch, cap assert [u["workload"] for u in doc["units"]] == [ {"name": "bench_truncate", "parameters": {"size": "10"}}, {"name": "bench_truncate", "parameters": {"size": "100"}}] - assert all(u["effect"] > 0 for u in doc["units"]) + assert all(u["effect"]["value"] > 0 for u in doc["units"]) cli.main(["compare", "--run", RUN, "--profile", "variants", "--baseline", BASELINE, "--vary", "role"]) shown = capsys.readouterr().out assert "bench_truncate [size=10]" in shown and "bench_truncate [size=100]" in shown @@ -199,7 +199,7 @@ def test_the_loop_with_an_asv_suite_names_cases_by_module(tmp, monkeypatch, caps assert code == cli.EXIT_OK and doc["failed_invariants"] == [] assert "varying: role (the only parameter)" in err [unit] = doc["units"] - assert unit["workload"] == {"name": "trunc.time_truncate"} and unit["effect"] > 0 + assert unit["workload"] == {"name": "trunc.time_truncate"} and unit["effect"]["value"] > 0 def test_the_loop_with_an_asv_suite_and_a_second_parameter(tmp, monkeypatch, capsys): @@ -226,9 +226,9 @@ def test_measure_mode_runs_the_loop_in_one_command_and_equals_read_mode(tree, ca code, doc, err = measure(capsys) assert code == cli.EXIT_OK and doc["failed_invariants"] == [] and core.check(doc, "comparison-document") == [] assert "varying: role (the only parameter)" in err and "round 3/3 role='variant'" in err - assert (doc["baseline"], doc["contender"]) == (BASELINE, VARIANT) and doc["local_only"] is False + assert (doc["sides"]["baseline"], doc["sides"]["contender"]) == (BASELINE, VARIANT) and doc["local_only"] is False [unit] = doc["units"] - assert unit["workload"] == {"name": "bench_truncate"} and unit["rounds"] == 3 and unit["effect"] > 0 + assert unit["workload"] == {"name": "bench_truncate"} and unit["details"]["rounds"] == 3 and unit["effect"]["value"] > 0 # What the session stored compares to the same document when read back (criterion 8). code, again, _ = compare(capsys) assert code == cli.EXIT_OK and again == doc @@ -237,7 +237,7 @@ def test_measure_mode_runs_the_loop_in_one_command_and_equals_read_mode(tree, ca def test_measure_mode_takes_the_baseline_value(tree, capsys): code, doc, _ = measure(capsys, "--baseline", VARIANT) assert code == cli.EXIT_OK and doc["failed_invariants"] == [] - assert (doc["baseline"], doc["contender"]) == (VARIANT, BASELINE) and doc["units"][0]["effect"] < 0 + assert (doc["sides"]["baseline"], doc["sides"]["contender"]) == (VARIANT, BASELINE) and doc["units"][0]["effect"]["value"] < 0 def test_measure_mode_refuses_a_baseline_that_is_not_a_value_before_running(tree, capsys): diff --git a/tests/test_validation.py b/tests/test_validation.py index 0b0a5cf..35f6de5 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -45,7 +45,9 @@ def test_lookup_of_missing_schema_is_none(): @pytest.mark.parametrize( - "file", sorted(COMPARISONS.glob("*.json")), ids=lambda path: path.stem + "file", + sorted(COMPARISONS.glob("*.json")), + ids=lambda path: path.stem, ) def test_comparison_examples_fit(file): data = json.loads(file.read_text()) @@ -53,13 +55,28 @@ def test_comparison_examples_fit(file): assert errors == [] and list(validation.check(data, validator)) == [] -def test_history_comparison_takes_no_run_fields(): - data = json.loads((COMPARISONS / "history.json").read_text()) - data["run_key"] = "rev-1" +@pytest.mark.parametrize("field", ["mode", "run_key", "profile", "baseline", "pairing"]) +def test_comparison_has_no_mode_specific_fields(field): + data = json.loads((COMPARISONS / "run.json").read_text()) + data[field] = "x" validator, _ = validation.find_schema(data, "comparison-document") [issue] = validation.check(data, validator) - # jsonschema reports a `false` subschema without its path or keyword. - assert issue.message == "False schema does not allow 'rev-1'" + assert field in issue.message + + +def test_comparison_details_are_open_since_a_new_method_must_not_change_the_schema(): + data = json.loads((COMPARISONS / "run.json").read_text()) + data["units"][0]["details"] = {"anything": {"a": "method"}, "may": "say"} + validator, _ = validation.find_schema(data, "comparison-document") + assert list(validation.check(data, validator)) == [] + + +def test_an_indeterminate_comparison_needs_its_reason(): + data = json.loads((COMPARISONS / "run.json").read_text()) + del data["units"][1]["reason"] + validator, _ = validation.find_schema(data, "comparison-document") + [issue] = validation.check(data, validator) + assert "reason" in issue.message def test_finds_result_schema(adhoc):