From 00ecd506b2f186f1ac829d7d52f35f5848c5d4d0 Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Wed, 26 Aug 2026 12:52:28 +0800 Subject: [PATCH] feat(experiments): validate reproducible performance records --- .github/pull_request_template.md | 1 + CONTRIBUTING.md | 13 +++ c/experiment_manifest.py | 115 +++++++++++++++++++++++++ c/tests/test_experiment_manifest.py | 67 ++++++++++++++ docs/experiments/manifest.example.json | 31 +++++++ 5 files changed, 227 insertions(+) create mode 100644 c/experiment_manifest.py create mode 100644 c/tests/test_experiment_manifest.py create mode 100644 docs/experiments/manifest.example.json diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 3259f416b..cdfaae414 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -7,6 +7,7 @@ Describe the problem and the smallest change that solves it. - [ ] `make -C c check` - [ ] CUDA changes were tested with `make -C c cuda-test` (if applicable) - [ ] Performance claims include hardware, commands, and repeatable measurements +- [ ] Performance claims include a validated experiment manifest with raw evidence ## Compatibility diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6ce28cb7e..28cebe5da 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -38,3 +38,16 @@ make -C c cuda-test CUDA_ARCH=native Benchmark reports should include the commit, exact commands, hardware and storage details, warm-up policy, run count, and median throughput. + +Performance PRs should attach an experiment manifest based on +`docs/experiments/manifest.example.json`. Validate it before submission with: + +```sh +python3 c/experiment_manifest.py path/to/result.json +``` + +The validator requires a full commit identity, at least three raw throughput +samples per arm, medians derived from those samples, hashed raw evidence, a +passing correctness gate, and exactly one changed configuration variable. +Negative and no-change results use the same record and remain first-class +evidence. diff --git a/c/experiment_manifest.py b/c/experiment_manifest.py new file mode 100644 index 000000000..49b6571ec --- /dev/null +++ b/c/experiment_manifest.py @@ -0,0 +1,115 @@ +#!/usr/bin/env python3 +"""Validate reproducible, one-variable Colibri experiment records.""" + +from __future__ import annotations + +import argparse +import json +import math +import statistics +from pathlib import Path + + +def _text(value, field): + if not isinstance(value, str) or not value.strip(): + raise ValueError(f"{field} must be a non-empty string") + + +def _object(value, field): + if not isinstance(value, dict): + raise ValueError(f"{field} must be an object") + return value + + +def _run(record, name): + run = _object(record.get(name), name) + config = _object(run.get("config"), f"{name}.config") + samples = _object(run.get("samples"), f"{name}.samples") + speeds = samples.get("tok_s") + if (not isinstance(speeds, list) or len(speeds) < 3 or + any(isinstance(v, bool) or not isinstance(v, (int, float)) or + not math.isfinite(v) or v <= 0 for v in speeds)): + raise ValueError(f"{name}.samples.tok_s must contain at least 3 positive finite values") + median = run.get("median_tok_s") + if (isinstance(median, bool) or not isinstance(median, (int, float)) or + not math.isclose(float(median), statistics.median(speeds), rel_tol=1e-6)): + raise ValueError(f"{name}.median_tok_s must equal the sample median") + evidence = _object(run.get("evidence"), f"{name}.evidence") + _text(evidence.get("uri"), f"{name}.evidence.uri") + digest = evidence.get("sha256") + if not isinstance(digest, str) or len(digest) != 64: + raise ValueError(f"{name}.evidence.sha256 must be 64 hex characters") + try: + bytes.fromhex(digest) + except ValueError as error: + raise ValueError(f"{name}.evidence.sha256 must be hexadecimal") from error + quality = _object(run.get("quality"), f"{name}.quality") + _text(quality.get("method"), f"{name}.quality.method") + if quality.get("passed") is not True: + raise ValueError(f"{name}.quality.passed must be true") + return config + + +def validate(record): + """Return a normalized summary or raise ValueError with an exact field.""" + if record.get("version") != 1: + raise ValueError("version must be 1") + for field in ("hypothesis", "commit", "model", "command", "prompt_hash"): + _text(record.get(field), field) + commit = record["commit"] + if len(commit) != 40: + raise ValueError("commit must be a full 40-character git SHA") + try: + bytes.fromhex(commit) + except ValueError as error: + raise ValueError("commit must be hexadecimal") from error + hardware = _object(record.get("hardware"), "hardware") + for field in ("cpu", "ram", "storage", "os"): + _text(hardware.get(field), f"hardware.{field}") + warmup = record.get("warmup_runs") + if isinstance(warmup, bool) or not isinstance(warmup, int) or warmup < 0: + raise ValueError("warmup_runs must be a non-negative integer") + + baseline = _run(record, "baseline") + trial = _run(record, "trial") + keys = sorted(set(baseline) | set(trial)) + actual = [key for key in keys if baseline.get(key) != trial.get(key)] + declared = record.get("changed_variables") + if not isinstance(declared, list) or len(declared) != 1 or not isinstance(declared[0], str): + raise ValueError("changed_variables must name exactly one variable") + if actual != declared: + raise ValueError(f"changed_variables {declared!r} does not match config diff {actual!r}") + outcome = record.get("outcome") + if outcome not in ("improvement", "regression", "no-change"): + raise ValueError("outcome must be improvement, regression, or no-change") + return { + "variable": actual[0], + "baseline_tok_s": record["baseline"]["median_tok_s"], + "trial_tok_s": record["trial"]["median_tok_s"], + "outcome": outcome, + } + + +def validate_path(path): + path = Path(path) + record = json.loads(path.read_text(encoding="utf-8")) + return validate(record) + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("manifests", nargs="+") + args = parser.parse_args(argv) + failed = False + for name in args.manifests: + try: + summary = validate_path(name) + print(f"{name}: ok ({summary['variable']}, {summary['outcome']})") + except (OSError, ValueError, json.JSONDecodeError) as error: + failed = True + print(f"{name}: {error}") + return int(failed) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/c/tests/test_experiment_manifest.py b/c/tests/test_experiment_manifest.py new file mode 100644 index 000000000..51d75e836 --- /dev/null +++ b/c/tests/test_experiment_manifest.py @@ -0,0 +1,67 @@ +import copy +import unittest + +from experiment_manifest import validate + + +def run(config, speeds): + return { + "config": config, + "samples": {"tok_s": speeds}, + "median_tok_s": sorted(speeds)[1], + "quality": {"method": "token-exact oracle", "passed": True}, + "evidence": {"uri": "https://example.invalid/raw.log", + "sha256": "ab" * 32}, + } + + +def manifest(): + return { + "version": 1, + "hypothesis": "two loader lanes improve cold decode", + "commit": "12" * 20, + "model": "GLM-5.2 int4", + "command": "NGEN=16 PROF=1 ./coli run ...", + "prompt_hash": "sha256:example", + "hardware": {"cpu": "example", "ram": "128 GB", + "storage": "NVMe ext4", "os": "Linux"}, + "warmup_runs": 1, + "changed_variables": ["PIPE_WORKERS"], + "baseline": run({"PIPE_WORKERS": "1", "DIRECT": "1"}, [1.0, 1.1, 1.2]), + "trial": run({"PIPE_WORKERS": "2", "DIRECT": "1"}, [1.2, 1.3, 1.4]), + "outcome": "improvement", + } + + +class ExperimentManifestTest(unittest.TestCase): + def test_accepts_reproducible_one_variable_record(self): + result = validate(manifest()) + self.assertEqual(result["variable"], "PIPE_WORKERS") + + def test_rejects_hidden_second_variable(self): + record = manifest() + record["trial"]["config"]["DIRECT"] = "0" + with self.assertRaisesRegex(ValueError, "config diff"): + validate(record) + + def test_rejects_claimed_median_not_backed_by_samples(self): + record = manifest() + record["trial"]["median_tok_s"] = 9.9 + with self.assertRaisesRegex(ValueError, "sample median"): + validate(record) + + def test_rejects_missing_quality_gate(self): + record = manifest() + record["trial"]["quality"]["passed"] = False + with self.assertRaisesRegex(ValueError, "passed must be true"): + validate(record) + + def test_rejects_unhashed_raw_evidence(self): + record = copy.deepcopy(manifest()) + record["baseline"]["evidence"]["sha256"] = "unknown" + with self.assertRaisesRegex(ValueError, "64 hex"): + validate(record) + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/experiments/manifest.example.json b/docs/experiments/manifest.example.json new file mode 100644 index 000000000..d484f7534 --- /dev/null +++ b/docs/experiments/manifest.example.json @@ -0,0 +1,31 @@ +{ + "version": 1, + "hypothesis": "Describe one causal hypothesis", + "commit": "0000000000000000000000000000000000000000", + "model": "model family, checkpoint and container format", + "command": "exact build and benchmark command", + "prompt_hash": "sha256 of the prompt set", + "hardware": { + "cpu": "CPU model and topology", + "ram": "capacity and relevant topology", + "storage": "device, controller, filesystem and mount options", + "os": "OS, compiler, driver and accelerator runtime" + }, + "warmup_runs": 1, + "changed_variables": ["ONLY_THIS_KEY"], + "baseline": { + "config": {"ONLY_THIS_KEY": "old", "EVERYTHING_ELSE": "fixed"}, + "samples": {"tok_s": [1.0, 1.1, 1.2]}, + "median_tok_s": 1.1, + "quality": {"method": "token-exact oracle", "passed": true}, + "evidence": {"uri": "artifact URL", "sha256": "0000000000000000000000000000000000000000000000000000000000000000"} + }, + "trial": { + "config": {"ONLY_THIS_KEY": "new", "EVERYTHING_ELSE": "fixed"}, + "samples": {"tok_s": [1.2, 1.3, 1.4]}, + "median_tok_s": 1.3, + "quality": {"method": "token-exact oracle", "passed": true}, + "evidence": {"uri": "artifact URL", "sha256": "0000000000000000000000000000000000000000000000000000000000000000"} + }, + "outcome": "improvement" +}