diff --git a/.buildkite/pipeline.yaml b/.buildkite/pipeline.yaml index c287cae..1887dc8 100644 --- a/.buildkite/pipeline.yaml +++ b/.buildkite/pipeline.yaml @@ -1,4 +1,12 @@ steps: + - label: ":lock: lint workloads" + agents: + queue: small_cpu_queue_premerge + commands: + - python3 -m ensurepip --upgrade --default-pip 2>/dev/null || true + - python3 -m pip install --user pyyaml + - export PATH="$HOME/.local/bin:$PATH" && python3 scripts/lint_workload.py --all + - label: ":pipeline: generate steps" agents: queue: small_cpu_queue_premerge diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..43331c5 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,9 @@ +repos: + - repo: local + hooks: + - id: lint-workload + name: lint workload recipes + entry: python3 scripts/lint_workload.py + language: system + files: ^workloads/.*\.yaml$ + pass_filenames: true diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..2ccb1f3 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,73 @@ +# Contributing + +How to add or change a workload recipe without breaking the nightly. + +## Setup + +Install pre-commit so recipes are linted on commit: + +```bash +pip install pre-commit +pre-commit install +``` + +## Add or change a recipe + +1. Copy an existing workload that targets the same GPU (e.g. + `workloads/minimax_m3_b200.yaml` for B200) into `workloads/_.yaml`. +2. Edit the fields for your model and tasks. The recipe schema is documented in + [README.md](./README.md). Set `nightly: true` to include it in the nightly + schedule; leave it off for opt-in recipes. +3. Lint locally (see below), open a PR, and validate on hardware before merge. + +## Lint (local + CI) + +`scripts/lint_workload.py` runs the same structural checks the pipeline relies +on, with no GPU and no model download: + +- the recipe passes `lib/parse_workload.py` (required fields, known GPU profile, + well-formed `lm_eval` / `vllm_bench` / `bfcl` blocks, and, when `lm_eval` is + installed, task names against the registry); +- `.buildkite/generate_pipeline.py` emits a buildkite step for it. + +```bash +scripts/lint_workload.py workloads/_.yaml # one or more paths +scripts/lint_workload.py --changed # recipes changed vs origin/main +scripts/lint_workload.py --all # every recipe +``` + +This is what catches the "commit garbage" cases: a typo'd field, an unknown +GPU, a malformed `serve_args`, or a task that generates no step. + +## What runs on your PR + +- **DCO**: every commit needs a `Signed-off-by` line (`git commit -s`). +- **lint workloads**: the buildkite pipeline runs `lint_workload.py --all` on + every PR and fails the build if any recipe is structurally invalid. + +The lint does **not** prove accuracy. It only proves the recipe is well-formed +and will produce a runnable step. + +## Validate on hardware (before merge) + +Prove the recipe actually serves and scores by triggering a build scoped to +just your recipe(s), against a known-good vLLM image: + +```bash +bk build create \ + --pipeline perf-eval \ + --commit "" \ + --branch "" \ + --env "VLLM_IMAGE=" \ + --env "VLLM_COMMIT=" \ + --env "WORKLOADS=workloads/_.yaml" +``` + +Link the passing build in your PR so reviewers can see it ran on the target GPU. +A build takes ~30-90 minutes; GPU queues are shared, so don't trigger duplicate +builds for the same commit. + +## AI assistance + +See [CLAUDE.md](./CLAUDE.md) for the disclosure convention (PR body line plus a +commit trailer for non-trivial agent-authored changes). diff --git a/lib/parse_workload.py b/lib/parse_workload.py index 076aec8..4472b29 100644 --- a/lib/parse_workload.py +++ b/lib/parse_workload.py @@ -162,7 +162,7 @@ def precision_from_model(model: str) -> str: def validate_tasks(tasks: list, path: str) -> None: if not tasks: sys.exit(f"{path}: missing or empty `lm_eval.tasks`") - skip_registry = env_truthy("BENCH_ONLY") + skip_registry = env_truthy("BENCH_ONLY") or env_truthy("SKIP_TASK_REGISTRY") known = set() if skip_registry else known_task_names() for t in tasks: extra = set(t) - TASK_FIELDS diff --git a/scripts/lint_workload.py b/scripts/lint_workload.py new file mode 100755 index 0000000..fafb6c3 --- /dev/null +++ b/scripts/lint_workload.py @@ -0,0 +1,106 @@ +#!/usr/bin/env python3 +"""Lint workload recipes before merge -- no GPU, no model download required. + +Runs the same structural checks the nightly pipeline relies on, so a broken +recipe fails in review instead of at 3am on a B200: + + 1. ``lib/parse_workload.py`` accepts the recipe -- required fields, known GPU + profile, well-formed lm_eval / vllm_bench / bfcl blocks, and (when + ``lm_eval`` is importable) task names against the registry. + 2. ``.buildkite/generate_pipeline.py`` emits a buildkite step for it. + +This is the automatic gate; it does NOT prove accuracy. The real proof is a +scoped hardware run -- see CONTRIBUTING.md ("Validate on hardware"). + +Usage: + scripts/lint_workload.py workloads/foo_b200.yaml [more.yaml ...] + scripts/lint_workload.py --changed # recipes added/changed vs origin/main + scripts/lint_workload.py --all # every workloads/*.yaml +""" + +import argparse +import glob +import os +import subprocess +import sys + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +def _run(cmd, extra_env=None): + env = dict(os.environ) + if extra_env: + env.update(extra_env) + return subprocess.run( + cmd, cwd=ROOT, env=env, capture_output=True, text=True + ) + + +def _lm_eval_available(): + return _run([sys.executable, "-c", "import lm_eval"]).returncode == 0 + + +def lint_one(path, skip_registry): + parse_env = {"SKIP_TASK_REGISTRY": "1"} if skip_registry else {} + parsed = _run([sys.executable, "lib/parse_workload.py", path], parse_env) + if parsed.returncode != 0: + return False, (parsed.stderr or parsed.stdout).strip() + + gen = _run( + [sys.executable, ".buildkite/generate_pipeline.py"], + {"WORKLOADS": path, "VLLM_IMAGE": "vllm/vllm-openai:lint"}, + ) + if gen.returncode != 0: + return False, "generate_pipeline: " + gen.stderr.strip() + if "label:" not in gen.stdout: + return False, "generate_pipeline emitted no step for this recipe" + return True, "ok" + + +def changed_workloads(): + base = os.environ.get("LINT_BASE_REF", "origin/main") + diff = _run( + ["git", "diff", "--name-only", "--diff-filter=d", base, "--", "workloads/"] + ) + return [ln for ln in diff.stdout.splitlines() if ln.endswith(".yaml")] + + +def main(): + ap = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter + ) + ap.add_argument("paths", nargs="*", help="workload YAML paths to lint") + ap.add_argument( + "--changed", action="store_true", help="lint recipes changed vs origin/main" + ) + ap.add_argument("--all", action="store_true", help="lint every workloads/*.yaml") + args = ap.parse_args() + + os.chdir(ROOT) + if args.all: + paths = sorted(glob.glob("workloads/*.yaml")) + elif args.changed: + paths = changed_workloads() + else: + paths = args.paths + if not paths: + print("lint_workload: no workloads to lint") + return 0 + + skip_registry = not _lm_eval_available() + if skip_registry: + print("lint_workload: lm_eval not importable; skipping task-name validation") + + failed = 0 + for path in paths: + ok, msg = lint_one(path, skip_registry) + print(f"{'PASS' if ok else 'FAIL'} {path}") + if not ok: + print(f" {msg}") + failed += 1 + print(f"lint_workload: {len(paths) - failed}/{len(paths)} passed") + return 1 if failed else 0 + + +if __name__ == "__main__": + sys.exit(main())