From 991bd064259a34fb7537f6af0c9819273b3ac482 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Thu, 18 Jun 2026 21:07:35 -0400 Subject: [PATCH 01/46] refactor(lib): rename dtni -> utils and split out generic config machinery Rename cvs/lib/dtni to cvs/lib/utils ("utils" says what it is: pure functions any lib can call; "dtni" was a leftover project codename). verdict.py moves unchanged. config_loader.py is trimmed to the framework-agnostic half: the paths/model/image/container schema, the 3-pass placeholder substitution, the enforce_thresholds gate on a new BaseVariantConfig, and a substitute_config() helper (file read + substitution + sibling-threshold discovery). The inference-only schema moves to a sibling module in a later commit. --- cvs/lib/dtni/config_loader.py | 301 ---------------------------- cvs/lib/{dtni => utils}/__init__.py | 0 cvs/lib/utils/config_loader.py | 217 ++++++++++++++++++++ cvs/lib/{dtni => utils}/verdict.py | 8 + 4 files changed, 225 insertions(+), 301 deletions(-) delete mode 100644 cvs/lib/dtni/config_loader.py rename cvs/lib/{dtni => utils}/__init__.py (100%) create mode 100644 cvs/lib/utils/config_loader.py rename cvs/lib/{dtni => utils}/verdict.py (83%) diff --git a/cvs/lib/dtni/config_loader.py b/cvs/lib/dtni/config_loader.py deleted file mode 100644 index d1010f2a..00000000 --- a/cvs/lib/dtni/config_loader.py +++ /dev/null @@ -1,301 +0,0 @@ -''' -Copyright 2025 Advanced Micro Devices, Inc. -All rights reserved. - -Typed config loader for the dtni vllm_single PoC. - -Loads a per-variant `config.json` + sibling `*_threshold.json`, validates -shape with pydantic v2 (extra="forbid"), and runs a 3-pass placeholder -substitution: - 1. cluster placeholders (`{user-id}`) anywhere - 2. self-reference within `paths` (e.g. `{shared_fs}`) - 3. cross-block (`{paths.models_dir}`, etc.) into the rest of the doc - -A loaded variant is returned as a `VariantConfig` instance whose `container` -field (`lifetime`, `name`, `image`, `runtime`) matches the dict shape that -`cvs.core.orchestrators.factory.OrchestratorConfig` already understands. -`runtime` is a nested `RuntimeSpec`, not a flat dict; `container.model_dump()` -serialises it to the `runtime: {name, args}` shape the factory consumes (the -vllm conftest does exactly this before building the orchestrator). The result -also carries a `thresholds` field with the parsed threshold contents. - -`model.remote=1` raises NotImplementedError -- schema is present, but the -download/resolve logic lives in cvs-dtni-v1's `resource_resolver.py` and -is out of scope for this PoC. - -GENERALIZATION SEAM (do this on the SECOND consumer, not speculatively): -`Paths`/`ModelSpec`/`ImageSpec`/`ContainerSpec`/`thresholds`, the placeholder -substitution, the `enforce_thresholds` gate, and `load_variant` are -framework-agnostic. `Params`, `Sweep`, `Roles`, and `cell_key`/`expected_cells` -(the ISL/OSL/TP/CONC key shape) are vllm_single-specific. When the next -framework lands (sglang-disagg, distributed, or a training suite), split this -into a `BaseVariantConfig` (the generic half) + per-framework subclasses -carrying their own `Params`/`Sweep`/`cell_key`. Extracting now -- with one -consumer -- would be guessing the seam. -''' - -from __future__ import annotations - -import getpass -import json -import re -import warnings -from pathlib import Path -from typing import Any, Dict, List - -from pydantic import BaseModel, ConfigDict, Field, model_validator -from typing_extensions import Literal - - -# ---------- pydantic models ---------- - - -class _Forbid(BaseModel): - model_config = ConfigDict(extra="forbid") - - -class _Allow(BaseModel): - model_config = ConfigDict(extra="allow") - - -class Paths(_Forbid): - shared_fs: str - models_dir: str - log_dir: str - benchmark_scripts_dir: str - hf_token_file: str - - -class ModelSpec(_Forbid): - id: str - remote: Literal[0, 1] - precision: str - - -class ImageSpec(_Forbid): - tag: str - remote: Literal[0, 1] - - -class RuntimeSpec(_Allow): - name: str - args: Dict[str, Any] = Field(default_factory=dict) - - -class ContainerSpec(_Forbid): - lifetime: Literal["no_launch", "per_run", "persistent"] = "per_run" - name: str - image: str - runtime: RuntimeSpec - - -class RoleServer(_Forbid): - server_script: str - - -class Roles(_Forbid): - server: RoleServer - - -class SeqCombo(_Forbid): - isl: str - osl: str - name: str - - -class Sweep(_Forbid): - sequence_combinations: List[SeqCombo] - concurrency_levels: List[int] - - -class Params(_Forbid): - backend: str = "vllm" - base_url: str = "http://0.0.0.0" - port_no: str = "8888" - dataset_name: str = "random" - burstiness: str = "1.0" - seed: str = "0" - request_rate: str = "inf" - max_model_length: str = "9216" - random_range_ratio: str = "0.8" - random_prefix_len: str = "0" - tensor_parallelism: str = "1" - tokenizer_mode: str = "auto" - percentile_metrics: str = "ttft,tpot,itl,e2el" - metric_percentiles: str = "99" - num_prompts: str = "3200" - # Completion-poll budget for the bench client = client_poll_count * 60s - # (plus a 120s initial wait). Large-output cells (high osl) need a bigger - # budget; the poll loop exits as soon as the client finishes, so raising - # this never slows down fast cells. See regressions REG-20260609-001. - client_poll_count: str = "20" - - -class VariantConfig(_Forbid): - schema_version: Literal[1] - framework: Literal["vllm_single"] - gpu_arch: str - # When false, the threshold-coverage gate warns instead of raising and the - # test records metrics without asserting pass/fail (record-only). Use for - # un-calibrated shapes (e.g. a throughput characterization whose published - # numbers are curves, not tabulated values). Default true keeps the gate - # strict for calibrated configs -- no regression to the remediation work. - enforce_thresholds: bool = True - paths: Paths - model: ModelSpec - image: ImageSpec - container: ContainerSpec - roles: Roles - params: Params - sweep: Sweep - thresholds: Dict[str, Dict[str, Any]] = Field(default_factory=dict) - - # pydantic runs @model_validator(mode="after") hooks in definition order. - # This remote check is intentionally first: an unimplemented remote config - # fails fast (NotImplementedError) before _check_thresholds_cover_sweep runs, - # which is meaningless for a config we are going to reject anyway. - @model_validator(mode="after") - def _check_remote_not_implemented(self): - if self.model.remote == 1: - raise NotImplementedError( - "model.remote=1 (remote model download) is not implemented in the PoC. " - "Port from cvs-dtni-v1/resource_resolver.py before enabling." - ) - return self - - def cell_key(self, isl, osl, concurrency): - """The canonical threshold key for one sweep cell. - - Single source of truth shared by the loader's coverage check and the - test's verdict lookup -- so the two can never drift on whitespace, - ordering, or field names. - """ - return f"ISL={isl},OSL={osl},TP={self.params.tensor_parallelism},CONC={concurrency}" - - def expected_cells(self): - """Every (isl, osl, conc) cell the sweep will parametrize.""" - return [ - self.cell_key(combo.isl, combo.osl, conc) - for combo in self.sweep.sequence_combinations - for conc in self.sweep.concurrency_levels - ] - - @model_validator(mode="after") - def _check_thresholds_cover_sweep(self): - """Fail at load time if any sweep cell lacks a threshold entry. - - Without this, a mistyped/whitespaced threshold key (or a new sweep - entry without a matching threshold) makes the test silently skip its - verdict and report a green PASS with zero assertions. - - When `enforce_thresholds` is false the same mismatch is reported as a - warning rather than an error -- the config loads as a record-only - scaffold (metrics captured, no assertions) instead of failing. - """ - expected = set(self.expected_cells()) - present = set(self.thresholds.keys()) - missing = sorted(expected - present) - extra = sorted(present - expected) - problems = [] - if missing: - problems.append(f"sweep cells with no threshold entry: {missing}") - if extra: - problems.append(f"threshold keys matching no sweep cell (typo?): {extra}") - if problems: - msg = "threshold.json does not match the sweep matrix; " + "; ".join(problems) - if self.enforce_thresholds: - raise ValueError(msg) - warnings.warn(f"{msg} (enforce_thresholds=false -> record-only)", stacklevel=2) - return self - - -# ---------- placeholder substitution ---------- - -_PLACEHOLDER_RE = re.compile(r"\{([a-zA-Z0-9_.\-]+)\}") - - -def _walk_substitute(node, mapping): - if isinstance(node, str): - - def repl(m): - key = m.group(1) - if key in mapping: - return str(mapping[key]) - return m.group(0) - - return _PLACEHOLDER_RE.sub(repl, node) - if isinstance(node, list): - return [_walk_substitute(x, mapping) for x in node] - if isinstance(node, dict): - return {k: _walk_substitute(v, mapping) for k, v in node.items()} - return node - - -def _flatten_paths(d, prefix=""): - out = {} - for k, v in d.items(): - key = f"{prefix}.{k}" if prefix else k - if isinstance(v, dict): - out.update(_flatten_paths(v, key)) - elif isinstance(v, (str, int, float)): - out[key] = str(v) - return out - - -def _resolve_cluster_mapping(cluster_dict): - raw = cluster_dict.get("username") or "{user-id}" - user = getpass.getuser() if raw == "{user-id}" else raw - return {"user-id": user} - - -# ---------- public API ---------- - - -def load_variant(config_path, cluster_dict): - """Load and validate a single variant config + its sibling threshold file. - - The threshold file is the sole `*threshold.json` next to the config (e.g. - `w1_..._threshold.json` beside `w1_..._config.json`), so config and - threshold can share a descriptive per-variant prefix. - """ - config_path = Path(config_path) - if not config_path.is_file(): - raise FileNotFoundError(f"variant config not found: {config_path}") - - raw = json.loads(config_path.read_text()) - - threshold_candidates = sorted(config_path.parent.glob("*threshold.json")) - if not threshold_candidates: - raise FileNotFoundError(f"no *threshold.json next to config: {config_path.parent}") - if len(threshold_candidates) > 1: - raise ValueError(f"multiple *threshold.json files next to config (ambiguous): {threshold_candidates}") - threshold_path = threshold_candidates[0] - thresholds = json.loads(threshold_path.read_text()) - - # Pass 1: cluster placeholders ({user-id}) everywhere. - cluster_map = _resolve_cluster_mapping(cluster_dict) - raw = _walk_substitute(raw, cluster_map) - - # Pass 2: self-reference within paths ({shared_fs} inside paths.*). - paths_block = raw.get("paths", {}) - if isinstance(paths_block, dict): - for _ in range(len(paths_block) + 1): - new = { - k: _walk_substitute(v, {pk: pv for pk, pv in paths_block.items() if isinstance(pv, str)}) - for k, v in paths_block.items() - } - if new == paths_block: - break - paths_block = new - raw["paths"] = paths_block - - # Pass 3: cross-block ({paths.models_dir} -> anywhere else). - flat_map = _flatten_paths({"paths": raw.get("paths", {})}) - raw = _walk_substitute(raw, flat_map) - - # Drop threshold-file comment keys (e.g. "_comment") before coverage check. - thresholds = {k: v for k, v in thresholds.items() if not k.startswith("_")} - - # Validate sibling thresholds, attach, build VariantConfig. - raw["thresholds"] = thresholds - return VariantConfig(**raw) diff --git a/cvs/lib/dtni/__init__.py b/cvs/lib/utils/__init__.py similarity index 100% rename from cvs/lib/dtni/__init__.py rename to cvs/lib/utils/__init__.py diff --git a/cvs/lib/utils/config_loader.py b/cvs/lib/utils/config_loader.py new file mode 100644 index 00000000..3091d100 --- /dev/null +++ b/cvs/lib/utils/config_loader.py @@ -0,0 +1,217 @@ +''' +Copyright 2025 Advanced Micro Devices, Inc. +All rights reserved. + +Framework-agnostic config machinery shared by every CVS suite. + +Holds the generic half of what used to be one monolithic loader: the +container/paths/model/image schema, the 3-pass placeholder substitution, the +`enforce_thresholds` gate carried on `BaseVariantConfig`, and the +`substitute_config` helper that reads a variant `config.json` + sibling +`*threshold.json` and resolves placeholders. A per-framework module subclasses +`BaseVariantConfig` and adds its own `Params`/`Sweep`/`cell_key` (see +`cvs.lib.inference.utils.inferencing_config_loader` for the vllm flavour). + +3-pass placeholder substitution: + 1. cluster placeholders (`{user-id}`) anywhere + 2. self-reference within `paths` (e.g. `{shared_fs}`) + 3. cross-block (`{paths.models_dir}`, etc.) into the rest of the doc + +A loaded variant's `container` field (`lifetime`, `name`, `image`, `runtime`) +matches the dict shape that `cvs.core.orchestrators.factory.OrchestratorConfig` +already understands. `runtime` is a nested `RuntimeSpec`, not a flat dict; +`container.model_dump()` serialises it to the `runtime: {name, args}` shape the +factory consumes (the vllm conftest does exactly this before building the +orchestrator). The loaded variant also carries a `thresholds` field with the +parsed threshold contents. + +`model.remote=1` raises NotImplementedError -- schema is present, but the +download/resolve logic lives in cvs-dtni-v1's `resource_resolver.py` and is +out of scope for this PoC. +''' + +from __future__ import annotations + +import getpass +import json +import re +from pathlib import Path +from typing import Any, Dict + +from pydantic import BaseModel, ConfigDict, Field, model_validator +from typing_extensions import Literal + + +# ---------- pydantic models (generic) ---------- + + +class _Forbid(BaseModel): + model_config = ConfigDict(extra="forbid") + + +class _Allow(BaseModel): + model_config = ConfigDict(extra="allow") + + +class Paths(_Forbid): + shared_fs: str + models_dir: str + log_dir: str + hf_token_file: str + + +class ModelSpec(_Forbid): + id: str + remote: Literal[0, 1] + precision: str + + +class ImageSpec(_Forbid): + tag: str + remote: Literal[0, 1] + + +class RuntimeSpec(_Allow): + name: str + args: Dict[str, Any] = Field(default_factory=dict) + + +class ContainerSpec(_Forbid): + lifetime: Literal["no_launch", "per_run", "persistent"] = "per_run" + name: str + image: str + runtime: RuntimeSpec + + +class BaseVariantConfig(_Forbid): + """The framework-agnostic skeleton of a variant config. + + Carries the fields every suite shares (schema/paths/model/image/container/ + thresholds + the enforce gate) and the remote-not-implemented guard. + Per-framework subclasses add their own `framework`/`Params`/`Sweep` and the + `cell_key`/coverage-check pair that depend on them. + """ + + schema_version: Literal[1] + # When false, the threshold-coverage gate warns instead of raising and the + # test records metrics without asserting pass/fail (record-only). Use for + # un-calibrated shapes (e.g. a throughput characterization whose published + # numbers are curves, not tabulated values). Default true keeps the gate + # strict for calibrated configs -- no regression to the remediation work. + enforce_thresholds: bool = True + paths: Paths + model: ModelSpec + image: ImageSpec + container: ContainerSpec + thresholds: Dict[str, Dict[str, Any]] = Field(default_factory=dict) + + # pydantic runs @model_validator(mode="after") hooks in definition order, + # parent-class hooks before subclass hooks. This remote check is intentionally + # the first to run: an unimplemented remote config fails fast + # (NotImplementedError) before any subclass's threshold-coverage check runs, + # which is meaningless for a config we are going to reject anyway. + @model_validator(mode="after") + def _check_remote_not_implemented(self): + if self.model.remote == 1: + raise NotImplementedError( + "model.remote=1 (remote model download) is not implemented in the PoC. " + "Port from cvs-dtni-v1/resource_resolver.py before enabling." + ) + return self + + +# ---------- placeholder substitution ---------- + +_PLACEHOLDER_RE = re.compile(r"\{([a-zA-Z0-9_.\-]+)\}") + + +def _walk_substitute(node, mapping): + if isinstance(node, str): + + def repl(m): + key = m.group(1) + if key in mapping: + return str(mapping[key]) + return m.group(0) + + return _PLACEHOLDER_RE.sub(repl, node) + if isinstance(node, list): + return [_walk_substitute(x, mapping) for x in node] + if isinstance(node, dict): + return {k: _walk_substitute(v, mapping) for k, v in node.items()} + return node + + +def _flatten_paths(d, prefix=""): + out = {} + for k, v in d.items(): + key = f"{prefix}.{k}" if prefix else k + if isinstance(v, dict): + out.update(_flatten_paths(v, key)) + elif isinstance(v, (str, int, float)): + out[key] = str(v) + return out + + +def _resolve_cluster_mapping(cluster_dict): + raw = cluster_dict.get("username") or "{user-id}" + user = getpass.getuser() if raw == "{user-id}" else raw + return {"user-id": user} + + +# ---------- public API (generic) ---------- + + +def substitute_config(config_path, cluster_dict): + """Read a variant config + sibling threshold file and resolve placeholders. + + Returns `(raw_dict, thresholds)`: the substituted config dict (NOT yet + validated -- the caller's per-framework `VariantConfig(**raw)` does that) + and the parsed, comment-stripped threshold dict. The threshold file is the + sole `*threshold.json` next to the config (e.g. `..._threshold.json` beside + `..._config.json`), so config and threshold can share a descriptive + per-variant prefix. + + This is the framework-neutral body of the old `load_variant`: file read + + 3-pass substitution + sibling-threshold discovery/strip. Per-framework + loaders call it, attach `thresholds`, then build their typed config. + """ + config_path = Path(config_path) + if not config_path.is_file(): + raise FileNotFoundError(f"variant config not found: {config_path}") + + raw = json.loads(config_path.read_text()) + + threshold_candidates = sorted(config_path.parent.glob("*threshold.json")) + if not threshold_candidates: + raise FileNotFoundError(f"no *threshold.json next to config: {config_path.parent}") + if len(threshold_candidates) > 1: + raise ValueError(f"multiple *threshold.json files next to config (ambiguous): {threshold_candidates}") + threshold_path = threshold_candidates[0] + thresholds = json.loads(threshold_path.read_text()) + + # Pass 1: cluster placeholders ({user-id}) everywhere. + cluster_map = _resolve_cluster_mapping(cluster_dict) + raw = _walk_substitute(raw, cluster_map) + + # Pass 2: self-reference within paths ({shared_fs} inside paths.*). + paths_block = raw.get("paths", {}) + if isinstance(paths_block, dict): + for _ in range(len(paths_block) + 1): + new = { + k: _walk_substitute(v, {pk: pv for pk, pv in paths_block.items() if isinstance(pv, str)}) + for k, v in paths_block.items() + } + if new == paths_block: + break + paths_block = new + raw["paths"] = paths_block + + # Pass 3: cross-block ({paths.models_dir} -> anywhere else). + flat_map = _flatten_paths({"paths": raw.get("paths", {})}) + raw = _walk_substitute(raw, flat_map) + + # Drop threshold-file comment keys (e.g. "_comment") before coverage check. + thresholds = {k: v for k, v in thresholds.items() if not k.startswith("_")} + + return raw, thresholds diff --git a/cvs/lib/dtni/verdict.py b/cvs/lib/utils/verdict.py similarity index 83% rename from cvs/lib/dtni/verdict.py rename to cvs/lib/utils/verdict.py index 467f4432..d6f0a498 100644 --- a/cvs/lib/dtni/verdict.py +++ b/cvs/lib/utils/verdict.py @@ -43,6 +43,8 @@ def _check_one(metric, actual_raw, spec): actuals = spec.get("_actuals", {}) if ref_metric not in actuals: return f"{metric}: reference metric '{ref_metric}' missing from actuals" + if actuals[ref_metric] is None: + return f"{metric}: reference '{ref_metric}' is None (metric unavailable for this run)" ref_actual = _to_float(actuals[ref_metric]) if ref_actual == 0: return f"{metric}: reference '{ref_metric}' is 0; cannot compute ratio" @@ -60,6 +62,12 @@ def evaluate_all(actuals, thresholds): if metric not in actuals: violations.append(f"{metric}: missing from actuals") continue + if actuals[metric] is None: + # A derived/goodput metric that could not be computed (e.g. + # decode_latency_ratio with no p50, or goodput with no SLO run). + # Loud violation, not a float(None) TypeError. + violations.append(f"{metric}: value is None (metric unavailable for this run)") + continue spec_with_actuals = dict(spec) if spec.get("kind") == "min_ratio": spec_with_actuals["_actuals"] = actuals From c0e741383695becf0ca471c46ae2de93c26a431e Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Thu, 18 Jun 2026 21:07:35 -0400 Subject: [PATCH 02/46] refactor(inference): move vllm_parsing into cvs/lib/inference/utils The client.* metric parser (to_client_metrics + CLIENT_METRICS surface) is inference-specific and should not sit in the shared utils dir. Move it under a new cvs/lib/inference/utils package. Content unchanged. --- cvs/lib/inference/utils/__init__.py | 4 + cvs/lib/inference/utils/vllm_parsing.py | 125 ++++++++++++++++++++++++ 2 files changed, 129 insertions(+) create mode 100644 cvs/lib/inference/utils/__init__.py create mode 100644 cvs/lib/inference/utils/vllm_parsing.py diff --git a/cvs/lib/inference/utils/__init__.py b/cvs/lib/inference/utils/__init__.py new file mode 100644 index 00000000..d3438a6e --- /dev/null +++ b/cvs/lib/inference/utils/__init__.py @@ -0,0 +1,4 @@ +''' +Copyright 2025 Advanced Micro Devices, Inc. +All rights reserved. +''' diff --git a/cvs/lib/inference/utils/vllm_parsing.py b/cvs/lib/inference/utils/vllm_parsing.py new file mode 100644 index 00000000..805a6520 --- /dev/null +++ b/cvs/lib/inference/utils/vllm_parsing.py @@ -0,0 +1,125 @@ +''' +Copyright 2025 Advanced Micro Devices, Inc. +All rights reserved. + +Pure parsers for vLLM benchmark artifacts. + +This module owns the *vocabulary and math* of vLLM metrics -- the mapping from +a raw benchmark artifact to the namespaced metric dict that downstream code +(threshold files, the per-metric HTML rows, `evaluate_all`) keys on. It is +deliberately free of I/O and orchestration: callers fetch the bytes (each job +lays its artifacts out differently -- single-node `cat`, disagg prefill+decode, +distributed rank-0 vs all-ranks) and hand the parsed/raw payload in here. + +Keeping the transforms pure makes them reusable across jobs (single-node, +distributed, disaggregated, InferenceMax) and unit-testable with plain +dict/string fixtures -- no fake orchestrator required. + +Namespacing contract: + - `client.*` -- scalars measured by the load generator (`vllm bench serve`). + - `server.*` -- scalars scraped from the live server `/metrics` endpoint + (future; `to_server_metrics` will live here too). +''' + +from __future__ import annotations + + +def _safe_div(num, den): + """num/den, or None if either is missing/None or the divisor is 0. + + Derived metrics must degrade to None (a clean "not available"), never a + bogus 0 or a KeyError that would crash an otherwise-good run. + """ + if num is None or den is None: + return None + try: + den = float(den) + if den == 0: + return None + return float(num) / den + except (TypeError, ValueError): + return None + + +def to_client_metrics(raw, *, tp, isl): + """Map a stock `vllm bench serve` results dict to the `client.*` namespace. + + `raw` is the already-parsed JSON the load generator writes to its + `--result-dir` `results` artifact. Every stock scalar is namespaced + `client.` 1:1, then a few derived metrics are appended. Pure: no I/O, + no orchestration -- the caller is responsible for fetching and json-loading + the artifact and for raising on missing/unparseable input. + + `tp` (tensor parallelism) and `isl` (input sequence length) are the only + out-of-band scalars the derivations need. + """ + m = {f"client.{k}": v for k, v in raw.items()} + # Friendly alias: stock's request_goodput -> client.goodput + # (the name the results table and threshold file reference). + m["client.goodput"] = raw.get("request_goodput") + + # Derived metrics. _safe_div guards every divisor (0/None/missing). + ttot = raw.get("total_token_throughput") + mean_ttft = raw.get("mean_ttft_ms") + p99_itl = raw.get("p99_itl_ms") + p50_itl = raw.get("p50_itl_ms") + median_tpot = raw.get("median_tpot_ms") + completed = raw.get("completed") + failed = raw.get("failed") + + m["client.per_gpu_throughput"] = _safe_div(ttot, tp) + m["client.normalized_ttft_ms_per_tok"] = _safe_div(mean_ttft, isl) + m["client.decode_latency_ratio"] = _safe_div(p99_itl, p50_itl) + m["client.decode_throughput_p50"] = _safe_div(1000.0, median_tpot) + total_req = None if completed is None or failed is None else completed + failed + m["client.success_rate"] = _safe_div(completed, total_req) + return m + + +# Numeric client.* metrics surfaced as one pytest test (= one HTML row) each. +# (short_name, unit); the value is looked up as "client." in the +# dict to_client_metrics returns. request_rate is omitted (stock emits the +# string "inf"). This is the *display surface* of the client.* vocabulary above +# -- it lives here, beside to_client_metrics, so every vLLM flavor (single-node, +# distributed, disaggregated, InferenceMax) shares one definition instead of +# each suite re-listing the rows. +CLIENT_METRICS = [ + ("max_concurrency", "-"), + ("max_concurrent_requests", "-"), + ("num_prompts", "-"), + ("completed", "-"), + ("failed", "-"), + ("success_rate", "-"), + ("duration", "s"), + ("request_throughput", "req/s"), + ("goodput", "req/s"), + ("output_throughput", "tok/s"), + ("total_token_throughput", "tok/s"), + ("per_gpu_throughput", "tok/s"), + ("decode_throughput_p50", "tok/s"), + ("max_output_tokens_per_s", "tok/s"), + ("total_input_tokens", "-"), + ("total_output_tokens", "-"), + ("mean_ttft_ms", "ms"), + ("median_ttft_ms", "ms"), + ("p90_ttft_ms", "ms"), + ("p95_ttft_ms", "ms"), + ("p99_ttft_ms", "ms"), + ("normalized_ttft_ms_per_tok", "ms/tok"), + ("mean_tpot_ms", "ms"), + ("median_tpot_ms", "ms"), + ("p90_tpot_ms", "ms"), + ("p95_tpot_ms", "ms"), + ("p99_tpot_ms", "ms"), + ("mean_itl_ms", "ms"), + ("median_itl_ms", "ms"), + ("p95_itl_ms", "ms"), + ("p99_itl_ms", "ms"), + ("decode_latency_ratio", "-"), + ("mean_e2el_ms", "ms"), + ("median_e2el_ms", "ms"), + ("p90_e2el_ms", "ms"), + ("p95_e2el_ms", "ms"), + ("p99_e2el_ms", "ms"), +] +CLIENT_METRIC_UNITS = dict(CLIENT_METRICS) From 19a89ac3558a14c7cec868260eb7a5cdc7876d8f Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Thu, 18 Jun 2026 21:07:35 -0400 Subject: [PATCH 03/46] feat(inference): inference config schema with named-combo sweep selector The inference half of the old config_loader: GoodputSlo, SeqCombo, Sweep, Params, Roles, and VariantConfig(BaseVariantConfig) with cell_key and the threshold-coverage check. load_variant() delegates the file read and placeholder substitution to the generic substitute_config(). Replaces the sequence_combinations x concurrency_levels cartesian with a named-combo + explicit runs[] selector: each run is a {combo, concurrency} pair, so the config enumerates exactly the cells to run (no NxM explosion). A model_validator rejects duplicate combo names and runs referencing an unknown combo at load time. --- .../utils/inferencing_config_loader.py | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 cvs/lib/inference/utils/inferencing_config_loader.py diff --git a/cvs/lib/inference/utils/inferencing_config_loader.py b/cvs/lib/inference/utils/inferencing_config_loader.py new file mode 100644 index 00000000..eb8a35fa --- /dev/null +++ b/cvs/lib/inference/utils/inferencing_config_loader.py @@ -0,0 +1,187 @@ +''' +Copyright 2025 Advanced Micro Devices, Inc. +All rights reserved. + +Inference-specific config schema for the vllm_single suite. + +The framework-agnostic machinery (paths/model/image/container schema, the +3-pass placeholder substitution, the `enforce_thresholds` gate, and the +`substitute_config` file-read helper) lives in `cvs.lib.utils.config_loader`. +This module holds the inference half: the sweep selector +(`SeqCombo`/`Run`/`Sweep`), the goodput SLO, the framework `Params`, the +`server` role, and `VariantConfig(BaseVariantConfig)` with the +ISL/OSL/TP/CONC `cell_key` and its sweep-coverage check. + +`Sweep`/`SeqCombo`/`GoodputSlo`/`Roles`/`cell_key` are inference-generic (any +serving framework sweeps sequence shapes at concurrencies); only `Params` (the +`vllm bench serve` flags) is framework-flavored and is the seam to subclass +when a second serving framework lands. +''' + +from __future__ import annotations + +import warnings +from typing import Dict, List, Optional + +from pydantic import model_validator +from typing_extensions import Literal + +from cvs.lib.utils.config_loader import BaseVariantConfig, _Forbid, substitute_config + + +# ---------- pydantic models (inference) ---------- + + +class RoleServer(_Forbid): + # The `vllm serve` command is built in Python (cvs.lib.inference.vllm_orch), + # not cloned from a `.sh` script, so a run is self-contained. Per-model + # server quirks live here: extra `vllm serve` flags and extra env vars + # merged over the defaults the orchestrator sets. Both default empty -- the + # fp8-kv cell needs neither (its --kv-cache-dtype is set unconditionally in + # code). + extra_serve_args: List[str] = [] + env: Dict[str, str] = {} + + +class Roles(_Forbid): + server: RoleServer = RoleServer() + + +class GoodputSlo(_Forbid): + # Per-cell SLOs for the goodput gate, in milliseconds. An INPUT to the run + # (passed to `vllm bench serve --goodput`), NOT a threshold to assert -- so + # it lives in the sweep, not threshold.json. Attached per seq_combo because + # e2el scales ~linearly with osl. _Forbid: a typo'd key fails load, not a + # silently-dropped SLO. + ttft_ms: float + tpot_ms: float + e2el_ms: float + + +class SeqCombo(_Forbid): + # `name` is the join key the `runs` selector references; required. + name: str + isl: str + osl: str + goodput_slo: Optional[GoodputSlo] = None + + +class Run(_Forbid): + # One sweep cell: a named combo run at a single concurrency. The explicit + # list of Runs replaces the old `sequence_combinations x concurrency_levels` + # cartesian -- you enumerate exactly the cells you want, no NxM explosion. + combo: str + concurrency: int + + +class Sweep(_Forbid): + sequence_combinations: List[SeqCombo] + runs: List[Run] + + @model_validator(mode="after") + def _check_runs_reference_known_combos(self): + """Combo names must be unique and every run.combo must name one. + + Without this a duplicate name silently shadows a combo and a typo'd + `run.combo` is a silently-dropped cell -- either way the sweep runs a + different matrix than the config reads. + """ + names = [c.name for c in self.sequence_combinations] + dupes = sorted({n for n in names if names.count(n) > 1}) + if dupes: + raise ValueError(f"duplicate sequence_combination names: {dupes}") + known = set(names) + unknown = sorted({r.combo for r in self.runs if r.combo not in known}) + if unknown: + raise ValueError(f"run.combo names no sequence_combination: {unknown} (known: {sorted(known)})") + return self + + +class Params(_Forbid): + backend: str = "vllm" + base_url: str = "http://0.0.0.0" + port_no: str = "8888" + dataset_name: str = "random" + burstiness: str = "1.0" + seed: str = "0" + request_rate: str = "inf" + random_range_ratio: str = "0.8" + random_prefix_len: str = "0" + tensor_parallelism: str = "1" + tokenizer_mode: str = "auto" + percentile_metrics: str = "ttft,tpot,itl,e2el" + metric_percentiles: str = "50,90,95,99" + num_prompts: str = "3200" + # Completion-poll budget for the bench client = client_poll_count * 60s + # (plus a 120s initial wait). Large-output cells (high osl) need a bigger + # budget; the poll loop exits as soon as the client finishes, so raising + # this never slows down fast cells. See regressions REG-20260609-001. + client_poll_count: str = "20" + + +class VariantConfig(BaseVariantConfig): + framework: Literal["vllm_single"] + gpu_arch: str + roles: Roles = Roles() + params: Params + sweep: Sweep + + def cell_key(self, isl, osl, concurrency): + """The canonical threshold key for one sweep cell. + + Single source of truth shared by the loader's coverage check and the + test's verdict lookup -- so the two can never drift on whitespace, + ordering, or field names. + """ + return f"ISL={isl},OSL={osl},TP={self.params.tensor_parallelism},CONC={concurrency}" + + def expected_cells(self): + """Every (isl, osl, conc) cell the sweep's `runs` selector picks.""" + by_name = {c.name: c for c in self.sweep.sequence_combinations} + return [ + self.cell_key(by_name[r.combo].isl, by_name[r.combo].osl, r.concurrency) + for r in self.sweep.runs + ] + + @model_validator(mode="after") + def _check_thresholds_cover_sweep(self): + """Fail at load time if any sweep cell lacks a threshold entry. + + Without this, a mistyped/whitespaced threshold key (or a new sweep + entry without a matching threshold) makes the test silently skip its + verdict and report a green PASS with zero assertions. + + When `enforce_thresholds` is false the same mismatch is reported as a + warning rather than an error -- the config loads as a record-only + scaffold (metrics captured, no assertions) instead of failing. + """ + expected = set(self.expected_cells()) + present = set(self.thresholds.keys()) + missing = sorted(expected - present) + extra = sorted(present - expected) + problems = [] + if missing: + problems.append(f"sweep cells with no threshold entry: {missing}") + if extra: + problems.append(f"threshold keys matching no sweep cell (typo?): {extra}") + if problems: + msg = "threshold.json does not match the sweep matrix; " + "; ".join(problems) + if self.enforce_thresholds: + raise ValueError(msg) + warnings.warn(f"{msg} (enforce_thresholds=false -> record-only)", stacklevel=2) + return self + + +# ---------- public API (inference) ---------- + + +def load_variant(config_path, cluster_dict): + """Load and validate a vllm_single variant config + its sibling threshold file. + + Delegates the file read + placeholder substitution + threshold discovery to + the generic `substitute_config`, then attaches the thresholds and builds the + typed `VariantConfig`. + """ + raw, thresholds = substitute_config(config_path, cluster_dict) + raw["thresholds"] = thresholds + return VariantConfig(**raw) From a7edb01b0fc85af941799b2c2dae7726523b2507 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Thu, 18 Jun 2026 21:07:35 -0400 Subject: [PATCH 04/46] feat(inference): self-contained server cmd + artifact-based result parsing build_server_cmd/start_server assemble a `vllm serve` arg list in Python (mirroring run_client) instead of cloning and running an external .sh, so a run needs no hand-staged script. --max-model-len is derived per cell from isl/osl/random_range_ratio so any sweep change stays self-consistent. parse_results reads the stock extensionless `results` JSON artifact that `vllm bench serve` writes to --result-dir and delegates namespacing + derived-metric math to vllm_parsing.to_client_metrics, replacing the brittle console-log regex table. Missing/empty/unparseable artifacts hard-fail the cell rather than recording a silently-green empty row. Adds the optional --goodput SLO gate (per-cell, omitted when no SLO) and threads goodput_slo through run(). --- cvs/lib/inference/vllm_orch.py | 144 +++++++++++++++++++++++---------- 1 file changed, 103 insertions(+), 41 deletions(-) diff --git a/cvs/lib/inference/vllm_orch.py b/cvs/lib/inference/vllm_orch.py index 63f079ba..85f93686 100644 --- a/cvs/lib/inference/vllm_orch.py +++ b/cvs/lib/inference/vllm_orch.py @@ -5,7 +5,8 @@ Standalone vLLM single-node job driven by a ContainerOrchestrator. This class talks only to `orch.exec`, which already routes into the running -container, and to a typed `VariantConfig` (see `cvs.lib.dtni.config_loader`). +container, and to a typed `VariantConfig` (see +`cvs.lib.inference.utils.inferencing_config_loader`). It is deliberately single-node and free of the `c_phdl`/`s_phdl` + manual `docker exec` plumbing that `cvs.lib.inference.base.InferenceBaseJob` carries. @@ -27,38 +28,18 @@ from __future__ import annotations +import json +import math import re import shlex import time from cvs.lib import globals +from cvs.lib.inference.utils.vllm_parsing import to_client_metrics log = globals.log -_METRIC_RES = [ - ("successful_requests", re.compile(r"Successful requests:\s+([0-9]+)", re.I)), - ("benchmark_duration", re.compile(r"Benchmark duration\s+\(s\):\s+([0-9\.]+)", re.I)), - ("total_input_tokens", re.compile(r"Total input tokens:\s+([0-9\.]+)", re.I)), - ("total_generated_tokens", re.compile(r"Total generated tokens:\s+([0-9\.]+)", re.I)), - ("request_throughput_per_sec", re.compile(r"Request throughput \(req/s\):\s+([0-9\.]+)", re.I)), - ("output_throughput_per_sec", re.compile(r"Output token throughput \(tok/s\):\s+([0-9\.]+)", re.I)), - ("total_throughput_per_sec", re.compile(r"Total Token throughput \(tok/s\):\s+([0-9\.]+)", re.I)), - ("mean_ttft_ms", re.compile(r"Mean TTFT \(ms\):\s+([0-9\.]+)", re.I)), - ("median_ttft_ms", re.compile(r"Median TTFT \(ms\):\s+([0-9\.]+)", re.I)), - ("p99_ttft_ms", re.compile(r"P99 TTFT \(ms\):\s+([0-9\.]+)", re.I)), - ("mean_tpot_ms", re.compile(r"Mean TPOT \(ms\):\s+([0-9\.]+)", re.I)), - ("median_tpot_ms", re.compile(r"Median TPOT \(ms\):\s+([0-9\.]+)", re.I)), - ("p99_tpot_ms", re.compile(r"P99 TPOT \(ms\):\s+([0-9\.]+)", re.I)), - ("mean_itl_ms", re.compile(r"Mean ITL \(ms\):\s+([0-9\.]+)", re.I)), - ("median_itl_ms", re.compile(r"Median ITL \(ms\):\s+([0-9\.]+)", re.I)), - ("p99_itl_ms", re.compile(r"P99 ITL \(ms\):\s+([0-9\.]+)", re.I)), - ("mean_e2el_ms", re.compile(r"Mean E2EL \(ms\):\s+([0-9\.]+)", re.I)), - ("median_e2el_ms", re.compile(r"Median E2EL \(ms\):\s+([0-9\.]+)", re.I)), - ("p99_e2el_ms", re.compile(r"P99 E2EL \(ms\):\s+([0-9\.]+)", re.I)), -] - - class VllmJob: """Single-node vLLM benchmark job driven by an injected ContainerOrchestrator. @@ -111,6 +92,7 @@ def __init__( osl, concurrency, num_prompts, + goodput_slo=None, log_subdir="vllm", server_precheck_wait_s=30, server_warmup_wait_s=330, @@ -132,12 +114,16 @@ def __init__( self.osl = str(osl) self.concurrency = str(concurrency) self.num_prompts = str(num_prompts) + # Per-cell SLO dict {ttft_ms, tpot_ms, e2el_ms} or None. An INPUT to the + # run (passed to `vllm bench serve --goodput`), threaded per-cell like isl + # because e2el scales with osl. None -> the --goodput flag is omitted and + # stock leaves request_goodput null. + self.goodput_slo = goodput_slo self.log_subdir = log_subdir p = variant.params self.tp = p.tensor_parallelism self.port_no = p.port_no - self.max_model_length = p.max_model_length self.random_range_ratio = p.random_range_ratio self.random_prefix_len = p.random_prefix_len self.burstiness = p.burstiness @@ -151,9 +137,12 @@ def __init__( self.backend = p.backend self.model_id = variant.model.id - self.server_script = variant.roles.server.server_script self.log_dir = variant.paths.log_dir - self.scripts_dir = variant.paths.benchmark_scripts_dir + # Per-model server quirks from config (both default empty): extra + # `vllm serve` flags and extra env vars merged over the orchestrator's + # defaults. The server command itself is Python-built (no .sh script). + self.extra_serve_args = list(variant.roles.server.extra_serve_args) + self.server_env = dict(variant.roles.server.env) # Pin the HF cache onto the mounted models dir. The container binds # models_dir both at /models and (via the home bind mount) at its own # host path, so this path is valid inside the container and the bytes @@ -164,7 +153,7 @@ def __init__( # Single-node: one output directory. self.out_dir = f"{self.log_dir}/{self.log_subdir}/out-node0" - self.server_log = f"{self.out_dir}/{self.server_script}_server.log" + self.server_log = f"{self.out_dir}/vllm_serve_server.log" self.client_log = f"{self.out_dir}/client.log" self._precheck_wait = server_precheck_wait_s @@ -177,13 +166,26 @@ def __init__( # ---------- server side ---------- + # vLLM's RandomDataset samples input in [isl*(1-r), isl*(1+r)] and output in + # [osl*(1-r), osl*(1+r)] (r = random_range_ratio), then prepends random_prefix_len + # fixed tokens. --max-model-len must cover the worst-case input+output+prefix or + # vLLM 400s every over-length request. Derive it per cell so any sweep change + # (isl/osl/ratio) stays self-consistent; +8 absorbs the sampler's integer rounding. + _MML_PAD = 8 + + def _derive_max_model_len(self): + r = float(self.random_range_ratio) + worst = (int(self.isl) + int(self.osl)) * (1.0 + r) + return str(math.ceil(worst) + int(self.random_prefix_len) + self._MML_PAD) + def build_server_cmd(self): - """Write the server-env script and create the per-node out-dir inside the container.""" + """Write the server-env script (sourced by both server and client) + and create the per-node out-dir inside the container.""" env_lines = [ f"export MODEL={shlex.quote(self.model_id)}", f"export ISL={shlex.quote(self.isl)}", f"export OSL={shlex.quote(self.osl)}", - f"export MAX_MODEL_LEN={shlex.quote(self.max_model_length)}", + f"export MAX_MODEL_LEN={shlex.quote(self._derive_max_model_len())}", f"export RANDOM_RANGE_RATIO={shlex.quote(self.random_range_ratio)}", f"export TP={shlex.quote(str(self.tp))}", f"export CONC={shlex.quote(self.concurrency)}", @@ -194,15 +196,47 @@ def build_server_cmd(self): "export VLLM_ROCM_USE_AITER_FUSED_MOE_A16W4=1", f"export PORT={shlex.quote(str(self.port_no))}", ] + # Per-model env overrides win over the defaults above (appended last). + for k, v in self.server_env.items(): + env_lines.append(f"export {k}={shlex.quote(str(v))}") env_script = "\n".join(env_lines) + "\n" # printf the script body verbatim; shlex.quote protects the outer bash layer. self.orch.exec("bash -c " + shlex.quote(f"printf '%s' {shlex.quote(env_script)} > /tmp/server_env_script.sh")) self.orch.exec(f"mkdir -p {shlex.quote(self.out_dir)}") + def _server_argv(self): + """The `vllm serve` arg list for this cell. + + Built in Python (mirrors run_client) so a run is self-contained -- no + external `.sh` to clone/stage. --kv-cache-dtype fp8 is set + unconditionally (the model is FP8-KV); per-model extras come from + roles.server.extra_serve_args. + """ + argv = [ + "vllm", + "serve", + self.model_id, + "--tensor-parallel-size", + str(self.tp), + "--max-model-len", + self._derive_max_model_len(), + "--port", + str(self.port_no), + "--kv-cache-dtype", + "fp8", + ] + argv.extend(self.extra_serve_args) + return argv + def start_server(self): + # Each token shlex.quoted: a model id/path with a space or $ would + # otherwise break the inner bash layer silently (same quoting as the + # client). The env script (HF token, AITER flags, cache pin) is sourced + # first; nohup backgrounds the server into its fixed log. + serve_cmd = " ".join(shlex.quote(str(a)) for a in self._server_argv()) inner = ( - f"cd {shlex.quote(self.scripts_dir)} && source /tmp/server_env_script.sh && " - f"nohup /bin/bash {shlex.quote(self.server_script)} > {shlex.quote(self.server_log)} 2>&1 &" + f"source /tmp/server_env_script.sh && " + f"nohup {serve_cmd} > {shlex.quote(self.server_log)} 2>&1 &" ) out = self.orch.exec("bash -c " + shlex.quote(inner)) for host, output in out.items(): @@ -286,6 +320,8 @@ def run_client(self): self.random_prefix_len, "--percentile-metrics", self.percentile_metrics, + "--metric-percentiles", + self.metric_percentiles, "--ignore-eos", "--save-result", "--result-dir", @@ -293,6 +329,20 @@ def run_client(self): "--result-filename", "results", ] + # Goodput SLO gate (optional). Stock computes request_goodput (good-req/s) + # only when --goodput is passed; a request is good iff it meets EVERY named + # SLO. Omit the flag entirely when no per-cell SLO is set (passing + # ttft:None would be a launch failure). + if self.goodput_slo: + args.append("--goodput") + for metric, key in (("ttft", "ttft_ms"), ("tpot", "tpot_ms"), ("e2el", "e2el_ms")): + val = ( + self.goodput_slo.get(key) + if hasattr(self.goodput_slo, "get") + else getattr(self.goodput_slo, key, None) + ) + if val is not None: + args.append(f"{metric}:{val}") bench_cmd = " ".join(shlex.quote(str(a)) for a in args) client_cmd = f"source /tmp/server_env_script.sh && {bench_cmd} > {shlex.quote(self.client_log)} 2>&1 &" self.orch.exec("bash -c " + shlex.quote(client_cmd)) @@ -325,15 +375,27 @@ def wait_client_complete(self): raise RuntimeError("client did not complete before poll cap") def parse_results(self): - """Return {host: {metric: str_value}} parsed from the client log.""" - out = self.orch.exec(f"tail -2000 {shlex.quote(self.client_log)}") + """Return {host: {client.METRIC: value}} parsed from the stock `results` artifact. + + Fetches the extensionless JSON `results` file `vllm bench serve` writes to + `--result-dir` (NOT the console log; NOT `results.json`) and delegates the + namespacing + derived-metric math to the pure + `cvs.lib.inference.utils.vllm_parsing.to_client_metrics`. Raises if the artifact is + missing/empty/unparseable -- the test wraps the job in try/except ... raise, + so this hard-fails the cell rather than recording an empty (silently-green) + row. The fetch lives here because artifact layout is job-specific; the + transform lives in inference.utils so distributed/disagg/InferenceMax can reuse it. + """ + artifact = f"{self.out_dir}/results" + out = self.orch.exec(f"cat {shlex.quote(artifact)}") results = {} for host, text in out.items(): - text = text or "" - m = {} - for key, pat in _METRIC_RES: - hit = pat.search(text) - if hit: - m[key] = hit.group(1) - results[host] = m + text = (text or "").strip() + if not text: + raise RuntimeError(f"empty/missing results artifact on {host}: {artifact}") + try: + raw = json.loads(text) + except (json.JSONDecodeError, ValueError) as e: + raise RuntimeError(f"unparseable results artifact on {host}: {artifact}: {e}") from e + results[host] = to_client_metrics(raw, tp=self.tp, isl=self.isl) return results From 67a67c2022b3abd367dffc6fc9483dddc586c8e1 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Thu, 18 Jun 2026 21:07:35 -0400 Subject: [PATCH 05/46] feat(suite): per-metric result rows, Value/Unit columns, sweep selector pytest_generate_tests now drives parametrization from the named-combo + runs[] selector instead of the cartesian. test_vllm_inference only runs the benchmark and stashes results; the verdict moves into a new test_metric (one pytest test = one HTML row per metric per cell), with inline Value/Unit columns added via pytest_html hooks in conftest. test_setup_sshd gates its 2224 probe on len(orch.hosts) > 1, mirroring the single-node orchestrator guard: single-node runs skip the in-container sshd (it exists only for inter-node MPI) and must not probe for it. Import paths follow the dtni -> utils / inference.utils moves. --- cvs/tests/inference/vllm/_shared.py | 24 +++-- cvs/tests/inference/vllm/conftest.py | 35 ++++++- cvs/tests/inference/vllm/vllm_single.py | 130 +++++++++++++++++++----- 3 files changed, 153 insertions(+), 36 deletions(-) diff --git a/cvs/tests/inference/vllm/_shared.py b/cvs/tests/inference/vllm/_shared.py index 8e509054..c8d1d062 100644 --- a/cvs/tests/inference/vllm/_shared.py +++ b/cvs/tests/inference/vllm/_shared.py @@ -2,7 +2,7 @@ Copyright 2025 Advanced Micro Devices, Inc. All rights reserved. -Shared test helpers for the dtni vllm_single suite. +Shared test helpers for the vllm_single suite. `test_print_results_table` is exported via `from ._shared import *` so each framework-specific suite file picks it up as a sibling test that pytest @@ -18,6 +18,12 @@ __all__ = ["test_print_results_table"] +def _cell(m, key): + """Table cell: a missing OR present-but-None metric renders as '-'.""" + v = m.get(key) + return "-" if v is None else v + + def test_print_results_table(inf_res_dict): if not inf_res_dict: log.info("inf_res_dict empty, nothing to print") @@ -33,8 +39,11 @@ def test_print_results_table(inf_res_dict): "Req/s", "Total tok/s", "Mean TTFT (ms)", + "P95 TTFT (ms)", "Mean TPOT (ms)", + "P95 TPOT (ms)", "P99 ITL (ms)", + "Goodput (req/s)", ] rows = [] for key, host_dict in inf_res_dict.items(): @@ -49,11 +58,14 @@ def test_print_results_table(inf_res_dict): policy, conc, host, - m.get("request_throughput_per_sec", "-"), - m.get("total_throughput_per_sec", "-"), - m.get("mean_ttft_ms", "-"), - m.get("mean_tpot_ms", "-"), - m.get("p99_itl_ms", "-"), + _cell(m, "client.request_throughput"), + _cell(m, "client.total_token_throughput"), + _cell(m, "client.mean_ttft_ms"), + _cell(m, "client.p95_ttft_ms"), + _cell(m, "client.mean_tpot_ms"), + _cell(m, "client.p95_tpot_ms"), + _cell(m, "client.p99_itl_ms"), + _cell(m, "client.goodput"), ] ) log.info("\n" + tabulate(rows, headers=headers, tablefmt="github")) diff --git a/cvs/tests/inference/vllm/conftest.py b/cvs/tests/inference/vllm/conftest.py index a6b3d06e..75b790d3 100644 --- a/cvs/tests/inference/vllm/conftest.py +++ b/cvs/tests/inference/vllm/conftest.py @@ -10,7 +10,7 @@ from cvs.core.orchestrators.factory import OrchestratorConfig, OrchestratorFactory from cvs.lib import globals -from cvs.lib.dtni.config_loader import load_variant +from cvs.lib.inference.utils.inferencing_config_loader import load_variant from cvs.lib.utils_lib import resolve_cluster_config_placeholders log = globals.log @@ -137,8 +137,9 @@ def pytest_collection_modifyitems(items): "test_setup_sshd": 1, "test_model_fetch": 2, "test_vllm_inference": 3, - "test_print_results_table": 4, - "test_teardown": 5, + "test_metric": 4, + "test_print_results_table": 5, + "test_teardown": 6, } items.sort(key=lambda it: rank.get(it.originalname or it.name.split("[")[0], 99)) @@ -170,3 +171,31 @@ def pytest_runtest_makereport(item, call): extras = getattr(report, "extras", []) extras.append(pytest_html.extras.html(html)) report.extras = extras + + +def pytest_html_results_table_header(cells): + """Add Value + Unit columns just before the trailing Links column. + + Populated for test_metric rows; blank for lifecycle/inference rows (they + record no metric_value user-property). Scoped to this suite's conftest, so + other suites' result tables are unaffected. + """ + cells.insert(-1, "Value") + cells.insert(-1, "Unit") + + +def pytest_html_results_table_row(report, cells): + props = dict(report.user_properties) + has = "metric_value" in props + val = props.get("metric_value") + unit = props.get("metric_unit", "") if has else "" + if not has: + shown = "" + elif val is None: + shown = "-" + elif isinstance(val, float): + shown = f"{val:.3f}" + else: + shown = str(val) + cells.insert(-1, f"{shown}") + cells.insert(-1, f"{unit}") diff --git a/cvs/tests/inference/vllm/vllm_single.py b/cvs/tests/inference/vllm/vllm_single.py index 01768b04..22a32235 100644 --- a/cvs/tests/inference/vllm/vllm_single.py +++ b/cvs/tests/inference/vllm/vllm_single.py @@ -13,13 +13,15 @@ import pytest from cvs.lib import globals -from cvs.lib.dtni.verdict import evaluate_all +from cvs.lib.inference.utils.inferencing_config_loader import GoodputSlo +from cvs.lib.utils.verdict import evaluate_all +from cvs.lib.inference.utils.vllm_parsing import CLIENT_METRICS as _METRICS, CLIENT_METRIC_UNITS as _METRIC_UNITS from cvs.lib.inference.vllm_orch import VllmJob import importlib.util as _ilu import pathlib as _pl -_spec = _ilu.spec_from_file_location("_dtni_vllm_shared", _pl.Path(__file__).with_name("_shared.py")) +_spec = _ilu.spec_from_file_location("_vllm_shared", _pl.Path(__file__).with_name("_shared.py")) _mod = _ilu.module_from_spec(_spec) _spec.loader.exec_module(_mod) test_print_results_table = _mod.test_print_results_table # exported as a sibling test # noqa: F841 @@ -34,14 +36,19 @@ _FETCH_PRESENCE_RETRIES = 5 + def pytest_generate_tests(metafunc): - """Parametrize test_vllm_inference over sequence_combinations × concurrency_levels. + """Parametrize test_vllm_inference from the sweep's named-combo + runs selector. Lives in the suite module (not conftest) because it parametrizes fixtures only test_vllm_inference consumes -- co-locating the parametrization with its sole consumer. It runs at collection time, before fixtures exist, so it reads the raw config_file JSON directly (it cannot use the variant_config fixture / the typed loader). + + The sweep lists `sequence_combinations` (each with a `name`) once and a + `runs` array of `{combo, concurrency}` pairs; one case is emitted per run. + No NxM cartesian -- exactly the cells `runs` enumerates. """ config_file = metafunc.config.getoption("config_file") if not config_file or not os.path.isfile(config_file): @@ -50,15 +57,46 @@ def pytest_generate_tests(metafunc): raw = json.load(fp) sweep = raw.get("sweep", {}) combos = sweep.get("sequence_combinations", []) - concs = sweep.get("concurrency_levels", []) + runs = sweep.get("runs", []) + # Validate each raw goodput_slo dict through the same _Forbid model the + # typed loader uses. pytest_generate_tests bypasses load_variant (it reads + # raw JSON at collection time), so without this a typo'd SLO key would be + # silently dropped and a wrong goodput gate would run on hardware. + for combo in combos: + if combo.get("goodput_slo") is not None: + GoodputSlo(**combo["goodput_slo"]) + # Build the name->combo map and mirror the typed Sweep validator here (this + # path reads raw JSON before load_variant runs): a duplicate combo name or a + # run referencing an unknown combo must fail collection, not silently drop. + by_name = {} + for combo in combos: + nm = combo["name"] + if nm in by_name: + raise ValueError(f"duplicate sequence_combination name: {nm!r}") + by_name[nm] = combo cases = [] ids = [] - for combo in combos: - default_name = "isl" + combo["isl"] + "_osl" + combo["osl"] - for c in concs: - cases.append((combo, c)) - ids.append(combo.get("name", default_name) + "-conc" + str(c)) - if "seq_combo" in metafunc.fixturenames and "concurrency" in metafunc.fixturenames and cases: + for run in runs: + combo_name = run["combo"] + if combo_name not in by_name: + raise ValueError( + f"run.combo {combo_name!r} names no sequence_combination " + f"(known: {sorted(by_name)})" + ) + combo = by_name[combo_name] + conc = run["concurrency"] + cases.append((combo, conc)) + ids.append(combo_name + "-conc" + str(conc)) + if "metric" in metafunc.fixturenames: + if cases: + metric_cases = [] + metric_ids = [] + for (combo, c), cid in zip(cases, ids): + for short, _unit in _METRICS: + metric_cases.append((combo, c, short)) + metric_ids.append(cid + "-" + short) + metafunc.parametrize("seq_combo,concurrency,metric", metric_cases, ids=metric_ids) + elif "seq_combo" in metafunc.fixturenames and "concurrency" in metafunc.fixturenames and cases: metafunc.parametrize("seq_combo,concurrency", cases, ids=ids) @@ -93,7 +131,7 @@ def test_launch_container(orch, variant_config, lifecycle, request): def test_setup_sshd(orch, lifecycle, request): - """Stage 2: start sshd in the container. Asserts the daemon is reachable on 2224.""" + """Stage 2: start sshd in the container (multinode only; single-node skips it).""" if lifecycle.failed: pytest.skip("a prior lifecycle stage failed") t = time.monotonic() @@ -102,10 +140,13 @@ def test_setup_sshd(orch, lifecycle, request): if not ok: lifecycle.failed = True pytest.fail("setup_sshd() returned False") - probe = orch.exec("bash -c 'ss -ltn 2>/dev/null | grep -q :2224 && echo OK || echo NO'") - if not any("OK" in (v or "") for v in (probe or {}).values()): - lifecycle.failed = True - pytest.fail("sshd not listening on 2224 after setup_sshd()") + # Single-node runs skip starting the in-container sshd (it exists only for + # inter-node MPI), so only probe 2224 when there is more than one host. + if len(orch.hosts) > 1: + probe = orch.exec("bash -c 'ss -ltn 2>/dev/null | grep -q :2224 && echo OK || echo NO'") + if not any("OK" in (v or "") for v in (probe or {}).values()): + lifecycle.failed = True + pytest.fail("sshd not listening on 2224 after setup_sshd()") def test_model_fetch(orch, variant_config, lifecycle, request): @@ -184,6 +225,7 @@ def test_vllm_inference(orch, variant_config, hf_token, seq_combo, concurrency, osl=osl, concurrency=concurrency, num_prompts=_num_prompts_for(osl, concurrency), + goodput_slo=seq_combo.get("goodput_slo"), client_poll_count=int(variant_config.params.client_poll_count), ) @@ -214,22 +256,56 @@ def test_vllm_inference(orch, variant_config, hf_token, seq_combo, concurrency, concurrency, ) inf_res_dict[key] = results + # Verdict is no longer asserted here: each metric is its own test (test_metric, + # one HTML row per metric per cell). This test only runs the benchmark and + # records the cell's results into the module-scoped inf_res_dict. + + +def test_metric(seq_combo, concurrency, metric, inf_res_dict, variant_config, lifecycle, request): + """One pytest test (= one HTML row) per perf metric per cell. + + The benchmark already ran once in test_vllm_inference and stashed its results + in the module-scoped inf_res_dict; this reads a single cached metric and + surfaces it as its own pass/fail row. The value is rendered inline via the + Value/Unit table columns (pytest_html_results_table_row in conftest). No GPU + work. Skips cleanly when the cell's inference failed/skipped so a missing cell + never reports a false green. + + Verdict: when enforce_thresholds is true AND a spec exists for this cell+metric + the value is asserted via the shared evaluate_all; otherwise the row is a + record-only PASS that simply displays the number. evaluate_all is handed the + full per-cell actuals (not just this one metric) so a min_ratio spec can still + resolve its reference metric. + """ + if lifecycle.failed: + pytest.skip("a prior lifecycle stage failed") + isl = seq_combo["isl"] + osl = seq_combo["osl"] + key = ( + variant_config.model.id, + variant_config.gpu_arch, + isl, + osl, + seq_combo.get("name", "default"), + concurrency, + ) + if key not in inf_res_dict: + pytest.skip(f"no recorded results for cell {key!r} (inference did not run)") + host_dict = inf_res_dict[key] + _host, actuals = next(iter(host_dict.items())) + full = "client." + metric + value = actuals.get(full) + unit = _METRIC_UNITS.get(metric, "-") + request.node.user_properties.append(("metric_value", value)) + request.node.user_properties.append(("metric_unit", unit)) - # Per-cell thresholds: thresholds layout is `{"ISL=...,OSL=...,TP=...,CONC=...": {metric: spec}}`. - # The key is built by VariantConfig.cell_key (the same builder the loader uses for its - # coverage check). When enforce_thresholds is true a missing cell is a hard error -- never - # a silent skip that would report a green PASS with no assertions. When it is false the - # config is a record-only scaffold (un-calibrated thresholds): capture the metrics and - # skip the verdict. if not variant_config.enforce_thresholds: - log.info("enforce_thresholds=false; recorded metrics for cell, skipping verdict") return cell = variant_config.cell_key(isl, osl, concurrency) - cell_thresholds = variant_config.thresholds.get(cell) - if not cell_thresholds: - raise AssertionError(f"no thresholds for cell {cell!r}; threshold file is out of sync with the sweep") - for host, actuals in results.items(): - evaluate_all(actuals, cell_thresholds) + spec = (variant_config.thresholds.get(cell) or {}).get(full) + if spec is None: + return + evaluate_all(actuals, {full: spec}) def test_teardown(orch, lifecycle, request): From 12829bf9bb9d446489dca8a50b08348578063769 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Thu, 18 Jun 2026 21:07:35 -0400 Subject: [PATCH 06/46] chore(config): rename vllm_single config pair, adopt selector, drop cluster file Rename the config/threshold pair to {model}_{precision}_{config|threshold} .json and convert the sweep to the named-combo + runs[] selector. Pin the image to rocm/vllm-dev:nightly (the previously pinned :nightly-sshd tag does not exist on Docker Hub, and single-node runs skip in-container sshd). Delete cvs/input/cluster_file/mi300x_vllm_single.json: a cluster file only needs node IP + user/key/orchestrator; the variant config supplies the container block, so the bespoke per-suite cluster file is redundant. --- .../cluster_file/mi300x_vllm_single.json | 33 ------------------- ...onfig.json => llama31_70b_fp8_config.json} | 32 +++++++++--------- ...ld.json => llama31_70b_fp8_threshold.json} | 20 +++++------ 3 files changed, 27 insertions(+), 58 deletions(-) delete mode 100644 cvs/input/cluster_file/mi300x_vllm_single.json rename cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/{w1_llama31_70b_fp8kv_config.json => llama31_70b_fp8_config.json} (72%) rename cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/{w1_llama31_70b_fp8kv_threshold.json => llama31_70b_fp8_threshold.json} (65%) diff --git a/cvs/input/cluster_file/mi300x_vllm_single.json b/cvs/input/cluster_file/mi300x_vllm_single.json deleted file mode 100644 index e2096212..00000000 --- a/cvs/input/cluster_file/mi300x_vllm_single.json +++ /dev/null @@ -1,33 +0,0 @@ -{ - "_comment": "Single-node MI300X cluster for the vllm_single PoC verification. Container backend with launch:true so CVS owns the container lifecycle; the actual image+name come from the variant config.json (container block), with this file providing only the cluster portion (node/user/key). Replace every with your node/deployment specifics before running.", - - "orchestrator": "container", - "username": "{user-id}", - "priv_key_file": "/data/{user-id}/.ssh/id_ed25519", - - "head_node_dict": { - "mgmt_ip": "" - }, - "env_vars": {}, - "node_dict": { - "": { - "bmc_ip": "NA", - "vpc_ip": "" - } - }, - - "_container_comment": "Stub container block; the testsuite config (variant config.json) overrides image/name/runtime.args via OrchestratorConfig.from_configs.", - "container": { - "lifetime": "per_run", - "name": "", - "image": "", - "runtime": { - "name": "docker", - "args": { - "network": "host", - "ipc": "host", - "privileged": true - } - } - } -} diff --git a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/w1_llama31_70b_fp8kv_config.json b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json similarity index 72% rename from cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/w1_llama31_70b_fp8kv_config.json rename to cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json index 1fa4790e..43b2e8c0 100644 --- a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/w1_llama31_70b_fp8kv_config.json +++ b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json @@ -5,10 +5,9 @@ "enforce_thresholds": false, "paths": { "shared_fs": "/home/{user-id}", - "models_dir": "/it-share/models", + "models_dir": "{shared_fs}/models", "log_dir": "{shared_fs}/LOGS", - "benchmark_scripts_dir": "{shared_fs}/benchmark_server_scripts", - "hf_token_file": "{shared_fs}/.hf_token" + "hf_token_file": "/data/atnair/.hf_token" }, "model": { "id": "amd/Llama-3.1-70B-Instruct-FP8-KV", @@ -16,13 +15,13 @@ "precision": "fp8" }, "image": { - "tag": "rocm/vllm-dev:nightly-sshd", + "tag": "rocm/vllm-dev:nightly", "remote": 1 }, "container": { "lifetime": "per_run", "name": "w1_llama31_70b_fp8kv_perf_inference_rocm", - "image": "rocm/vllm-dev:nightly-sshd", + "image": "rocm/vllm-dev:nightly", "runtime": { "name": "docker", "args": { @@ -37,9 +36,7 @@ } }, "roles": { - "server": { - "server_script": "llama31-70b-fp8kv_mi300x_vllm.sh" - } + "server": {} }, "params": { "backend": "vllm", @@ -49,28 +46,33 @@ "burstiness": "1.0", "seed": "0", "request_rate": "inf", - "max_model_length": "2304", "random_range_ratio": "0.8", "random_prefix_len": "0", "tensor_parallelism": "8", "tokenizer_mode": "auto", "percentile_metrics": "ttft,tpot,itl,e2el", - "metric_percentiles": "99", + "metric_percentiles": "50,90,95,99", "num_prompts": "3200", "client_poll_count": "90" }, "sweep": { "sequence_combinations": [ { + "name": "w1_isl=128_osl=2048", "isl": "128", "osl": "2048", - "name": "throughput" + "goodput_slo": { + "ttft_ms": 1000000000.0, + "tpot_ms": 1000000000.0, + "e2el_ms": 1000000000.0 + } } ], - "concurrency_levels": [ - 64, - 128, - 256 + "runs": [ + { + "combo": "w1_isl=128_osl=2048", + "concurrency": 16 + } ] } } diff --git a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/w1_llama31_70b_fp8kv_threshold.json b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json similarity index 65% rename from cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/w1_llama31_70b_fp8kv_threshold.json rename to cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json index 96d2674c..9b1aff0f 100644 --- a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/w1_llama31_70b_fp8kv_threshold.json +++ b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json @@ -1,43 +1,43 @@ { - "_comment": "PLACEHOLDER thresholds for W1 (Llama 3.1 70B FP8-KV, in=128/out=2048, TP=8). AMD's published source (performance-results.html) gives throughput-vs-latency curves, not tabulated per-cell numbers, so these are not yet calibrated. config.json sets enforce_thresholds=false: the suite records metrics and skips pass/fail. Replace min_tok_s/max_ms with real numbers and flip enforce_thresholds=true to make this cell assert.", + "_comment": "PLACEHOLDER thresholds for W1 (Llama 3.1 70B FP8-KV, in=128/out=2048, TP=8). AMD's published source (performance-results.html) gives throughput-vs-latency curves, not tabulated per-cell numbers, so these are not yet calibrated. config.json sets enforce_thresholds=false: the suite records metrics and skips pass/fail. Metric keys are namespaced client.* (from the stock results artifact). Replace min_tok_s/max_ms with real numbers and flip enforce_thresholds=true to make this cell assert.", "ISL=128,OSL=2048,TP=8,CONC=64": { - "total_throughput_per_sec": { + "client.total_token_throughput": { "kind": "min_tok_s", "value": 0 }, - "mean_ttft_ms": { + "client.mean_ttft_ms": { "kind": "max_ms", "value": 1000000 }, - "mean_tpot_ms": { + "client.mean_tpot_ms": { "kind": "max_ms", "value": 1000000 } }, "ISL=128,OSL=2048,TP=8,CONC=128": { - "total_throughput_per_sec": { + "client.total_token_throughput": { "kind": "min_tok_s", "value": 0 }, - "mean_ttft_ms": { + "client.mean_ttft_ms": { "kind": "max_ms", "value": 1000000 }, - "mean_tpot_ms": { + "client.mean_tpot_ms": { "kind": "max_ms", "value": 1000000 } }, "ISL=128,OSL=2048,TP=8,CONC=256": { - "total_throughput_per_sec": { + "client.total_token_throughput": { "kind": "min_tok_s", "value": 0 }, - "mean_ttft_ms": { + "client.mean_ttft_ms": { "kind": "max_ms", "value": 1000000 }, - "mean_tpot_ms": { + "client.mean_tpot_ms": { "kind": "max_ms", "value": 1000000 } From 18bf04b0e2855e3460a5fa8e56d96b16369ceb31 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Thu, 18 Jun 2026 21:07:35 -0400 Subject: [PATCH 07/46] test(inference): unit tests for parser, sweep selector, and verdict guards Cover to_client_metrics purity + derived metrics, the named-combo/runs sweep selector (expansion, unknown-combo and duplicate-name rejection), the run_client goodput/metric-percentiles flags, table-cell rendering, and the verdict None-guards. Adds JSON fixtures for the stock results artifact. --- cvs/lib/inference/unittests/__init__.py | 0 .../fixtures/vllm_results_sample.json | 1 + .../fixtures/vllm_results_widened.json | 52 ++ .../test_inferencing_config_loader.py | 123 +++++ .../unittests/test_vllm_orch_parse.py | 518 ++++++++++++++++++ 5 files changed, 694 insertions(+) create mode 100644 cvs/lib/inference/unittests/__init__.py create mode 100644 cvs/lib/inference/unittests/fixtures/vllm_results_sample.json create mode 100644 cvs/lib/inference/unittests/fixtures/vllm_results_widened.json create mode 100644 cvs/lib/inference/unittests/test_inferencing_config_loader.py create mode 100644 cvs/lib/inference/unittests/test_vllm_orch_parse.py diff --git a/cvs/lib/inference/unittests/__init__.py b/cvs/lib/inference/unittests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/cvs/lib/inference/unittests/fixtures/vllm_results_sample.json b/cvs/lib/inference/unittests/fixtures/vllm_results_sample.json new file mode 100644 index 00000000..84230482 --- /dev/null +++ b/cvs/lib/inference/unittests/fixtures/vllm_results_sample.json @@ -0,0 +1 @@ +{"date": "20260616-195845", "endpoint_type": "vllm", "backend": "vllm", "label": null, "model_id": "amd/Llama-3.1-70B-Instruct-FP8-KV", "tokenizer_id": "amd/Llama-3.1-70B-Instruct-FP8-KV", "num_prompts": 12800, "request_rate": "inf", "burstiness": 1.0, "max_concurrency": 256, "duration": 2689.1403915379196, "completed": 12800, "failed": 0, "total_input_tokens": 1638400, "total_output_tokens": 26214400, "request_throughput": 4.7598853671896535, "request_goodput": null, "output_throughput": 9748.24523200441, "total_token_throughput": 10357.510559004686, "max_output_tokens_per_s": 10765.0, "max_concurrent_requests": 512, "rtfx": 0.0, "mean_ttft_ms": 668.48989902217, "median_ttft_ms": 639.2352399416268, "std_ttft_ms": 450.33186696146356, "p99_ttft_ms": 2471.584795164014, "mean_tpot_ms": 25.931252678514355, "median_tpot_ms": 25.9539894319472, "std_tpot_ms": 0.21713041382746362, "p99_tpot_ms": 26.324707068246756, "mean_itl_ms": 25.9318070919762, "median_itl_ms": 25.50674183294177, "std_itl_ms": 8.843604314389477, "p99_itl_ms": 32.19962654635305, "mean_e2el_ms": 53749.76413194105, "median_e2el_ms": 53752.437153831124, "std_e2el_ms": 298.81966449031785, "p99_e2el_ms": 55124.09303063527} \ No newline at end of file diff --git a/cvs/lib/inference/unittests/fixtures/vllm_results_widened.json b/cvs/lib/inference/unittests/fixtures/vllm_results_widened.json new file mode 100644 index 00000000..3e32870f --- /dev/null +++ b/cvs/lib/inference/unittests/fixtures/vllm_results_widened.json @@ -0,0 +1,52 @@ +{ + "date": "20260617-163232", + "endpoint_type": "vllm", + "backend": "vllm", + "label": null, + "model_id": "amd/Llama-3.1-70B-Instruct-FP8-KV", + "tokenizer_id": "amd/Llama-3.1-70B-Instruct-FP8-KV", + "num_prompts": 3200, + "request_rate": "inf", + "burstiness": 1.0, + "max_concurrency": 64, + "duration": 564.1147056743503, + "completed": 1791, + "failed": 1409, + "total_input_tokens": 229974, + "total_output_tokens": 2312408, + "request_throughput": 3.174886210170704, + "request_goodput": 3.174886210170704, + "output_throughput": 4099.180497760143, + "total_token_throughput": 4506.852904961594, + "max_output_tokens_per_s": 4590.0, + "max_concurrent_requests": 76, + "rtfx": 0.0, + "mean_ttft_ms": 291.88843878398956, + "median_ttft_ms": 73.2587007805705, + "std_ttft_ms": 1137.8571870825917, + "p50_ttft_ms": 73.2587007805705, + "p90_ttft_ms": 85.64653992652893, + "p95_ttft_ms": 91.81479038670659, + "p99_ttft_ms": 6259.152442589402, + "mean_tpot_ms": 15.032167084785439, + "median_tpot_ms": 15.03020008988302, + "std_tpot_ms": 0.3247818510561047, + "p50_tpot_ms": 15.03020008988302, + "p90_tpot_ms": 15.209271169796184, + "p95_tpot_ms": 15.24944565870449, + "p99_tpot_ms": 15.943741001209947, + "mean_itl_ms": 15.019441688915942, + "median_itl_ms": 14.628257602453232, + "std_itl_ms": 6.1097319902977505, + "p50_itl_ms": 14.628257602453232, + "p90_itl_ms": 15.559395030140879, + "p95_itl_ms": 17.392832040786708, + "p99_itl_ms": 27.511265948414778, + "mean_e2el_ms": 19668.871285726116, + "median_e2el_ms": 19835.17629932612, + "std_e2el_ms": 7829.601121737766, + "p50_e2el_ms": 19835.17629932612, + "p90_e2el_ms": 30145.559770055115, + "p95_e2el_ms": 31765.095902141184, + "p99_e2el_ms": 34034.53387795014 +} diff --git a/cvs/lib/inference/unittests/test_inferencing_config_loader.py b/cvs/lib/inference/unittests/test_inferencing_config_loader.py new file mode 100644 index 00000000..c1ba98b8 --- /dev/null +++ b/cvs/lib/inference/unittests/test_inferencing_config_loader.py @@ -0,0 +1,123 @@ +''' +Copyright 2025 Advanced Micro Devices, Inc. +All rights reserved. + +Unit tests for the sweep selector in +cvs.lib.inference.utils.inferencing_config_loader: named combos + an explicit +`runs` array (replacing the old sequence_combinations x concurrency_levels +cartesian). No hardware. Covers the Sweep validator (unique names, known +run.combo) and VariantConfig.expected_cells (the runs->cell_key expansion). +''' + +import unittest + +from pydantic import ValidationError + +from cvs.lib.inference.utils.inferencing_config_loader import ( + Run, + SeqCombo, + Sweep, + VariantConfig, +) + + +def _combo(name, isl="128", osl="2048"): + return SeqCombo(name=name, isl=isl, osl=osl) + + +def _variant(sweep, tp="8"): + """A minimal VariantConfig carrying just enough to exercise expected_cells. + + remote=0 (the remote guard would otherwise reject it) and + enforce_thresholds=False so the empty threshold dict does not trip the + coverage check -- this test pins the selector expansion, not the gate. + """ + return VariantConfig( + schema_version=1, + framework="vllm_single", + gpu_arch="mi300x", + enforce_thresholds=False, + paths={ + "shared_fs": "/home/x", + "models_dir": "/home/x/models", + "log_dir": "/home/x/LOGS", + "hf_token_file": "/home/x/.hf", + }, + model={"id": "amd/Llama-3.1-70B-Instruct-FP8-KV", "remote": 0, "precision": "fp8"}, + image={"tag": "rocm/vllm-dev:nightly-sshd", "remote": 1}, + container={ + "name": "c", + "image": "rocm/vllm-dev:nightly-sshd", + "runtime": {"name": "docker"}, + }, + params={"tensor_parallelism": tp}, + sweep=sweep, + ) + + +class TestSweepValidator(unittest.TestCase): + def test_valid_runs_selector_constructs(self): + sw = Sweep( + sequence_combinations=[_combo("a"), _combo("b", osl="4096")], + runs=[Run(combo="a", concurrency=16), Run(combo="b", concurrency=32)], + ) + self.assertEqual([r.combo for r in sw.runs], ["a", "b"]) + + def test_unknown_run_combo_raises(self): + with self.assertRaises(ValidationError) as ctx: + Sweep( + sequence_combinations=[_combo("a")], + runs=[Run(combo="typo", concurrency=16)], + ) + self.assertIn("names no sequence_combination", str(ctx.exception)) + + def test_duplicate_combo_names_raise(self): + with self.assertRaises(ValidationError) as ctx: + Sweep( + sequence_combinations=[_combo("a"), _combo("a", osl="4096")], + runs=[Run(combo="a", concurrency=16)], + ) + self.assertIn("duplicate sequence_combination names", str(ctx.exception)) + + def test_concurrency_levels_is_rejected(self): + # The old cartesian key must be gone (extra=forbid): a config still + # carrying concurrency_levels should fail loudly, not silently ignore it. + with self.assertRaises(ValidationError): + Sweep( + sequence_combinations=[_combo("a")], + runs=[Run(combo="a", concurrency=16)], + concurrency_levels=[16], + ) + + +class TestExpectedCells(unittest.TestCase): + def test_runs_expand_to_exactly_their_cells(self): + sw = Sweep( + sequence_combinations=[_combo("a", isl="128", osl="2048"), _combo("b", isl="256", osl="4096")], + runs=[ + Run(combo="a", concurrency=16), + Run(combo="b", concurrency=32), + Run(combo="a", concurrency=64), + ], + ) + vc = _variant(sw) + self.assertEqual( + vc.expected_cells(), + [ + "ISL=128,OSL=2048,TP=8,CONC=16", + "ISL=256,OSL=4096,TP=8,CONC=32", + "ISL=128,OSL=2048,TP=8,CONC=64", + ], + ) + + def test_no_cartesian_blowup(self): + # Two combos + two runs must yield TWO cells, not 2x2=4 (the old bug). + sw = Sweep( + sequence_combinations=[_combo("a"), _combo("b", osl="4096")], + runs=[Run(combo="a", concurrency=16), Run(combo="b", concurrency=16)], + ) + self.assertEqual(len(_variant(sw).expected_cells()), 2) + + +if __name__ == "__main__": + unittest.main() diff --git a/cvs/lib/inference/unittests/test_vllm_orch_parse.py b/cvs/lib/inference/unittests/test_vllm_orch_parse.py new file mode 100644 index 00000000..6b635c3d --- /dev/null +++ b/cvs/lib/inference/unittests/test_vllm_orch_parse.py @@ -0,0 +1,518 @@ +''' +Copyright 2025 Advanced Micro Devices, Inc. +All rights reserved. + +Unit tests for VllmJob.parse_results (stock `results` artifact -> client.* + +derived metrics) and run_client's --goodput / --metric-percentiles flag +construction. No hardware: a fake orch returns committed fixture text. +''' + +import json +import re +import unittest +from pathlib import Path +from types import SimpleNamespace + +from cvs.lib.utils.verdict import ThresholdViolation, evaluate_all +from cvs.lib.inference.vllm_orch import VllmJob + +_HERE = Path(__file__).parent +_FIXTURES = _HERE / "fixtures" +_REPO = _HERE.parents[3] # cvs/lib/inference/unittests -> repo root +_SHARED = _REPO / "cvs/tests/inference/vllm/_shared.py" +_THRESHOLD = ( + _REPO / "cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json" +) + +# isl/tp used to build the job; must match the fixture's run for the derived +# math assertions to be meaningful (real artifact: isl=128, tp=8). +_ISL = 128 +_TP = 8 + + +class FakeOrch: + """Minimal stand-in for ContainerOrchestrator: records commands, returns a canned dict.""" + + def __init__(self, exec_return=None): + self.exec_return = exec_return if exec_return is not None else {} + self.commands = [] + + def exec(self, cmd, **kwargs): + self.commands.append(cmd) + return self.exec_return + + +def _fake_variant(goodput_slo_unused=None): + """A SimpleNamespace tree carrying exactly the attributes VllmJob.__init__ reads.""" + params = SimpleNamespace( + tensor_parallelism=str(_TP), + port_no="8888", + random_range_ratio="0.8", + random_prefix_len="0", + burstiness="1.0", + seed="0", + request_rate="inf", + tokenizer_mode="auto", + percentile_metrics="ttft,tpot,itl,e2el", + metric_percentiles="50,90,95,99", + base_url="http://0.0.0.0", + dataset_name="random", + backend="vllm", + ) + return SimpleNamespace( + params=params, + model=SimpleNamespace(id="amd/Llama-3.1-70B-Instruct-FP8-KV"), + roles=SimpleNamespace(server=SimpleNamespace(extra_serve_args=[], env={})), + paths=SimpleNamespace(log_dir="/tmp/logs", models_dir="/tmp/models"), + ) + + +def _make_job(orch, goodput_slo=None): + return VllmJob( + orch=orch, + variant=_fake_variant(), + hf_token="tok", + isl=_ISL, + osl=2048, + concurrency=256, + num_prompts=12800, + goodput_slo=goodput_slo, + ) + + +def _load_fixture(name): + return (_FIXTURES / name).read_text() + + +class TestParseResults(unittest.TestCase): + def setUp(self): + self.real = json.loads(_load_fixture("vllm_results_sample.json")) + self.widened = json.loads(_load_fixture("vllm_results_widened.json")) + + def _parse(self, fixture_name): + orch = FakeOrch({"fakehost": _load_fixture(fixture_name)}) + job = _make_job(orch) + return job.parse_results()["fakehost"] + + def test_all_stock_scalars_namespaced_and_numeric(self): + m = self._parse("vllm_results_widened.json") + # Every stock scalar appears 1:1 under client.* with its value preserved. + for k, v in self.widened.items(): + ck = f"client.{k}" + self.assertIn(ck, m, f"missing {ck}") + self.assertEqual(m[ck], v) + # Spot-check a few are numeric (not the old scrape's strings). + for ck in ("client.total_token_throughput", "client.mean_ttft_ms", "client.p99_itl_ms"): + self.assertIsInstance(m[ck], (int, float)) + + def test_derived_metrics_exact(self): + m = self._parse("vllm_results_widened.json") + w = self.widened + self.assertAlmostEqual(m["client.per_gpu_throughput"], w["total_token_throughput"] / _TP) + self.assertAlmostEqual(m["client.normalized_ttft_ms_per_tok"], w["mean_ttft_ms"] / _ISL) + self.assertAlmostEqual(m["client.decode_latency_ratio"], w["p99_itl_ms"] / w["p50_itl_ms"]) + self.assertAlmostEqual(m["client.decode_throughput_p50"], 1000.0 / w["median_tpot_ms"]) + self.assertAlmostEqual(m["client.success_rate"], w["completed"] / (w["completed"] + w["failed"])) + + def test_goodput_passthrough_null_and_value(self): + # Real artifact: request_goodput is null (ran without --goodput). + m_null = self._parse("vllm_results_sample.json") + self.assertIsNone(m_null["client.goodput"]) + # Widened fixture: non-null goodput passed straight through. + m_val = self._parse("vllm_results_widened.json") + self.assertEqual(m_val["client.goodput"], self.widened["request_goodput"]) + + def test_decode_latency_ratio_none_when_p50_absent(self): + # The real artifact has no p50_itl_ms (it ran at metric_percentiles=99), + # so the ratio must degrade to None, not raise. + m = self._parse("vllm_results_sample.json") + self.assertIsNone(m["client.decode_latency_ratio"]) + + def test_missing_artifact_raises(self): + orch = FakeOrch({"fakehost": ""}) + job = _make_job(orch) + with self.assertRaises(RuntimeError): + job.parse_results() + + def test_unparseable_artifact_raises(self): + orch = FakeOrch({"fakehost": "not json {{{"}) + job = _make_job(orch) + with self.assertRaises(RuntimeError): + job.parse_results() + + +class TestRunClientFlags(unittest.TestCase): + def _client_cmd(self, goodput_slo): + orch = FakeOrch() + job = _make_job(orch, goodput_slo=goodput_slo) + job.run_client() + # run_client issues exactly one exec: bash -c ''. + self.assertEqual(len(orch.commands), 1) + return orch.commands[0] + + def test_metric_percentiles_flag_present(self): + cmd = self._client_cmd(None) + self.assertIn("--metric-percentiles", cmd) + self.assertIn("50,90,95,99", cmd) + + def test_goodput_flag_omitted_when_none(self): + cmd = self._client_cmd(None) + self.assertNotIn("--goodput", cmd) + + def test_goodput_flag_built_from_slo_dict(self): + slo = {"ttft_ms": 500.0, "tpot_ms": 50.0, "e2el_ms": 60000.0} + cmd = self._client_cmd(slo) + self.assertIn("--goodput", cmd) + for tok in ("ttft:500.0", "tpot:50.0", "e2el:60000.0"): + self.assertIn(tok, cmd) + + def test_goodput_flag_built_from_slo_object(self): + # config_loader.GoodputSlo is a pydantic object (attr access), not a dict. + slo = SimpleNamespace(ttft_ms=500.0, tpot_ms=50.0, e2el_ms=60000.0) + cmd = self._client_cmd(slo) + self.assertIn("--goodput", cmd) + self.assertIn("ttft:500.0", cmd) + + +class TestKeyConsistency(unittest.TestCase): + """Mechanical guard (verification #5): every key the table reads and every + threshold cell key must be a key parse_results actually emits. Catches a + silent `-` column or a silent threshold skip WITHOUT a hardware run.""" + + def _producer_keys(self): + orch = FakeOrch({"fakehost": _load_fixture("vllm_results_widened.json")}) + return set(_make_job(orch).parse_results()["fakehost"].keys()) + + def test_table_keys_are_produced(self): + produced = self._producer_keys() + shared_src = _SHARED.read_text() + table_keys = set(re.findall(r'_cell\(m,\s*"(client\.[^"]+)"', shared_src)) + self.assertTrue(table_keys, "no client.* table keys found in _shared.py") + missing = table_keys - produced + self.assertEqual(missing, set(), f"table reads keys parse_results never emits: {missing}") + + def test_threshold_keys_are_produced(self): + produced = self._producer_keys() + thr = json.loads(_THRESHOLD.read_text()) + threshold_metric_keys = set() + for cell, metrics in thr.items(): + if cell.startswith("_"): + continue + threshold_metric_keys.update(metrics.keys()) + self.assertTrue(threshold_metric_keys, "no threshold metric keys found") + missing = threshold_metric_keys - produced + self.assertEqual(missing, set(), f"threshold asserts keys parse_results never emits: {missing}") + + +class TestVerdictNoneGuard(unittest.TestCase): + """Regression (review fix #1): parse_results now emits metrics that are + legitimately None (derived ratio with no p50, goodput with no SLO run). + If a threshold targets one, evaluate_all must raise a clean + ThresholdViolation -- NOT a float(None) TypeError.""" + + def test_none_actual_raises_threshold_violation_not_typeerror(self): + actuals = {"client.goodput": None} + thresholds = {"client.goodput": {"kind": "min", "value": 1.0}} + with self.assertRaises(ThresholdViolation) as ctx: + evaluate_all(actuals, thresholds) + self.assertIn("value is None", str(ctx.exception)) + + def test_none_actual_does_not_mask_other_real_violations(self): + actuals = {"client.goodput": None, "client.total_token_throughput": 5.0} + thresholds = { + "client.goodput": {"kind": "min", "value": 1.0}, + "client.total_token_throughput": {"kind": "min", "value": 10.0}, + } + with self.assertRaises(ThresholdViolation) as ctx: + evaluate_all(actuals, thresholds) + msg = str(ctx.exception) + self.assertIn("value is None", msg) + self.assertIn("client.total_token_throughput", msg) + + def test_non_none_actual_still_evaluated_normally(self): + # A satisfied threshold must still pass (guard is None-specific). + actuals = {"client.goodput": 5.0} + thresholds = {"client.goodput": {"kind": "min", "value": 1.0}} + evaluate_all(actuals, thresholds) # no raise + + +class TestTableCellRendering(unittest.TestCase): + """Regression (code-review F1): a metric that is present-but-None (goodput + with no SLO run, a derived ratio with no p50) must render as "-" in the + results table, not the literal "None" (m.get returns None when the key + exists).""" + + def _cell(self): + import importlib.util + import sys as _sys + from types import ModuleType + + # _shared.py imports tabulate at module scope; the _cell helper does not + # need it. Stub it so the module loads in the bare unittest interpreter. + _sys.modules.setdefault("tabulate", ModuleType("tabulate")) + if not hasattr(_sys.modules["tabulate"], "tabulate"): + _sys.modules["tabulate"].tabulate = lambda *a, **k: "" + spec = importlib.util.spec_from_file_location("_shared_under_test", str(_SHARED)) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod._cell + + def test_present_but_none_renders_dash(self): + cell = self._cell() + self.assertEqual(cell({"client.goodput": None}, "client.goodput"), "-") + + def test_absent_key_renders_dash(self): + cell = self._cell() + self.assertEqual(cell({}, "client.goodput"), "-") + + def test_real_value_passes_through(self): + cell = self._cell() + self.assertEqual(cell({"client.goodput": 4.76}, "client.goodput"), 4.76) + + def test_zero_is_not_dashed(self): + # 0.0 is a real measurement, must NOT become '-'. + cell = self._cell() + self.assertEqual(cell({"client.request_throughput": 0.0}, "client.request_throughput"), 0.0) + + +class TestVerdictMinRatioReferenceNone(unittest.TestCase): + """Regression (code-review F2): min_ratio dereferences a SECOND (reference) + metric; if that reference is a None-valued derived metric, evaluate_all must + raise a clean ThresholdViolation, not float(None) TypeError.""" + + def test_min_ratio_none_reference_raises_violation_not_typeerror(self): + actuals = {"client.per_gpu_throughput": 1000.0, "client.decode_latency_ratio": None} + thresholds = { + "client.per_gpu_throughput": { + "kind": "min_ratio", + "reference": "client.decode_latency_ratio", + "value": 0.5, + } + } + with self.assertRaises(ThresholdViolation) as ctx: + evaluate_all(actuals, thresholds) + self.assertIn("is None", str(ctx.exception)) + + +if __name__ == "__main__": + unittest.main() + + +class TestDerivedMaxModelLen(unittest.TestCase): + """MAX_MODEL_LEN is derived per cell from isl/osl/random_range_ratio/random_prefix_len, + not read from config. Worst-case sequence = (isl+osl)*(1+r) + prefix; +pad for rounding.""" + + def _env_cmd(self, job): + orch = FakeOrch() + job.orch = orch + job.build_server_cmd() + # build_server_cmd issues: printf the env script, then mkdir the out-dir. + # The env script (with MAX_MODEL_LEN) is in the first exec. + return orch.commands[0] + + def test_derived_value_for_default_cell(self): + # isl=128, osl=2048, r=0.8, prefix=0 -> ceil(2176*1.8)=3917, +0 +8 = 3925. + job = _make_job(FakeOrch()) + self.assertEqual(job._derive_max_model_len(), "3925") + + def test_low_ratio_shrinks_window(self): + # Dropping the ratio must shrink the derived window automatically. + job = _make_job(FakeOrch()) + job.random_range_ratio = "0.1" + # ceil(2176*1.1)=2394, +0 +8 = 2402. + self.assertEqual(job._derive_max_model_len(), "2402") + + def test_zero_ratio_is_fixed_length_plus_pad(self): + job = _make_job(FakeOrch()) + job.random_range_ratio = "0.0" + # (128+2048) + 0 + 8 = 2184. + self.assertEqual(job._derive_max_model_len(), "2184") + + def test_prefix_len_added(self): + job = _make_job(FakeOrch()) + job.random_range_ratio = "0.0" + job.random_prefix_len = "64" + # 2176 + 64 + 8 = 2248. + self.assertEqual(job._derive_max_model_len(), "2248") + + def test_env_script_carries_derived_value(self): + job = _make_job(FakeOrch()) + cmd = self._env_cmd(job) + self.assertIn("MAX_MODEL_LEN=3925", cmd) + + +import importlib.util as _ilu_t + +_VS_PATH = _REPO / "cvs/tests/inference/vllm/vllm_single.py" + + +def _load_vllm_single(): + """Import the suite module standalone to reach _METRICS and test_metric. + + The module's only collection-time work is a top-level importlib exec of the + sibling _shared.py; everything else is function/constant defs, so importing it + outside pytest is safe and hardware-free. + """ + spec = _ilu_t.spec_from_file_location("_vllm_single_under_test", str(_VS_PATH)) + mod = _ilu_t.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +class _FakeNode: + def __init__(self): + self.user_properties = [] + + +class _FakeRequest: + def __init__(self): + self.node = _FakeNode() + + +class _FakeLifecycle: + def __init__(self, failed=False): + self.failed = failed + + +def _fake_variant_config(enforce=False, thresholds=None): + """Stand-in carrying exactly what test_metric reads: model.id, gpu_arch, + enforce_thresholds, thresholds, and the real cell_key builder.""" + params = SimpleNamespace(tensor_parallelism=str(_TP)) + vc = SimpleNamespace( + model=SimpleNamespace(id="amd/Llama-3.1-70B-Instruct-FP8-KV"), + gpu_arch="mi300x", + params=params, + enforce_thresholds=enforce, + thresholds=thresholds or {}, + ) + vc.cell_key = lambda isl, osl, conc: f"ISL={isl},OSL={osl},TP={params.tensor_parallelism},CONC={conc}" + return vc + + +class TestMetricTests(unittest.TestCase): + """test_metric is the per-metric pytest row. These exercise its three paths + (cell-missing skip, record-only PASS, enforced violation) with fakes -- no + hardware, no real pytest collection.""" + + def setUp(self): + self.vs = _load_vllm_single() + # A realistic per-cell actuals dict, as test_vllm_inference would stash. + orch = FakeOrch({"fakehost": _load_fixture("vllm_results_widened.json")}) + self.actuals = _make_job(orch).parse_results()["fakehost"] + self.seq_combo = {"isl": "128", "osl": "2048", "name": "throughput"} + self.conc = 64 + self.key = ("amd/Llama-3.1-70B-Instruct-FP8-KV", "mi300x", "128", "2048", "throughput", 64) + + def test_every_metric_key_is_produced(self): + """Every _METRICS short name must resolve to a client.* key parse_results + emits -- otherwise its row would silently show '-'.""" + produced = set(self.actuals.keys()) + missing = [short for short, _u in self.vs._METRICS if ("client." + short) not in produced] + self.assertEqual(missing, [], f"_METRICS names with no producer: {missing}") + + def test_skips_when_cell_absent(self): + import pytest as _pt + with self.assertRaises(_pt.skip.Exception): + self.vs.test_metric( + self.seq_combo, self.conc, "p99_e2el_ms", + {}, _fake_variant_config(), _FakeLifecycle(), _FakeRequest(), + ) + + def test_skips_when_lifecycle_failed(self): + import pytest as _pt + with self.assertRaises(_pt.skip.Exception): + self.vs.test_metric( + self.seq_combo, self.conc, "p99_e2el_ms", + {self.key: {"fakehost": self.actuals}}, + _fake_variant_config(), _FakeLifecycle(failed=True), _FakeRequest(), + ) + + def test_record_only_records_value_and_unit(self): + req = _FakeRequest() + self.vs.test_metric( + self.seq_combo, self.conc, "p99_e2el_ms", + {self.key: {"fakehost": self.actuals}}, + _fake_variant_config(enforce=False), _FakeLifecycle(), req, + ) + props = dict(req.node.user_properties) + self.assertEqual(props["metric_value"], self.actuals["client.p99_e2el_ms"]) + self.assertEqual(props["metric_unit"], "ms") + + def test_enforce_raises_on_violation(self): + # mean_ttft_ms is ~hundreds; a max_ms of 1.0 must trip evaluate_all. + cell = f"ISL=128,OSL=2048,TP={_TP},CONC=64" + thr = {cell: {"client.mean_ttft_ms": {"kind": "max_ms", "value": 1.0}}} + with self.assertRaises(ThresholdViolation): + self.vs.test_metric( + self.seq_combo, self.conc, "mean_ttft_ms", + {self.key: {"fakehost": self.actuals}}, + _fake_variant_config(enforce=True, thresholds=thr), _FakeLifecycle(), _FakeRequest(), + ) + + def test_enforce_no_spec_is_record_only(self): + # enforce=true but no threshold for this metric -> record-only, no raise. + req = _FakeRequest() + self.vs.test_metric( + self.seq_combo, self.conc, "p95_tpot_ms", + {self.key: {"fakehost": self.actuals}}, + _fake_variant_config(enforce=True, thresholds={}), _FakeLifecycle(), req, + ) + self.assertEqual(dict(req.node.user_properties)["metric_unit"], "ms") + + +from cvs.lib.inference.utils.vllm_parsing import _safe_div, to_client_metrics + + +class TestToClientMetricsPure(unittest.TestCase): + """Direct tests of the pure transform -- no FakeOrch, no VllmJob. + + parse_results already covers the wiring; these pin the vocabulary + math in + isolation so distributed/disagg/InferenceMax reuse rests on a tested seam. + """ + + def setUp(self): + self.raw = json.loads(_load_fixture("vllm_results_widened.json")) + + def test_namespaces_every_stock_scalar(self): + m = to_client_metrics(self.raw, tp=_TP, isl=_ISL) + for k, v in self.raw.items(): + self.assertEqual(m[f"client.{k}"], v) + + def test_goodput_alias(self): + m = to_client_metrics(self.raw, tp=_TP, isl=_ISL) + self.assertEqual(m["client.goodput"], self.raw["request_goodput"]) + + def test_derived_metrics_exact(self): + m = to_client_metrics(self.raw, tp=_TP, isl=_ISL) + w = self.raw + self.assertAlmostEqual(m["client.per_gpu_throughput"], w["total_token_throughput"] / _TP) + self.assertAlmostEqual(m["client.normalized_ttft_ms_per_tok"], w["mean_ttft_ms"] / _ISL) + self.assertAlmostEqual(m["client.decode_latency_ratio"], w["p99_itl_ms"] / w["p50_itl_ms"]) + self.assertAlmostEqual(m["client.decode_throughput_p50"], 1000.0 / w["median_tpot_ms"]) + self.assertAlmostEqual(m["client.success_rate"], w["completed"] / (w["completed"] + w["failed"])) + + def test_pure_no_mutation_of_input(self): + snapshot = dict(self.raw) + to_client_metrics(self.raw, tp=_TP, isl=_ISL) + self.assertEqual(self.raw, snapshot) + + def test_missing_inputs_degrade_to_none_not_raise(self): + m = to_client_metrics({}, tp=_TP, isl=_ISL) + for d in ("per_gpu_throughput", "normalized_ttft_ms_per_tok", + "decode_latency_ratio", "decode_throughput_p50", "success_rate"): + self.assertIsNone(m[f"client.{d}"]) + + +class TestSafeDivPure(unittest.TestCase): + def test_normal(self): + self.assertEqual(_safe_div(10, 2), 5.0) + + def test_zero_divisor_is_none(self): + self.assertIsNone(_safe_div(1, 0)) + + def test_none_operands_are_none(self): + self.assertIsNone(_safe_div(None, 2)) + self.assertIsNone(_safe_div(2, None)) + + def test_non_numeric_is_none(self): + self.assertIsNone(_safe_div("x", 2)) From 3a9723e47e9dd8b65e84b8a395baf1a52e93a667 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Thu, 18 Jun 2026 21:53:23 -0400 Subject: [PATCH 08/46] docs: suite-authoring guide + AGENTS.md for shared and inference helpers Add a human reference guide (plans/building-a-cvs-test-suite.md) that walks the six-layer suite architecture using vllm_single as the worked example: the generic <-> framework config seam, the named-combo + runs[] sweep selector, the self-contained Python-built server cmd, lifecycle-as-tests, and a checklist for authoring a new inference or training suite. Add per-package AGENTS.md docs naming the public entry points, the seam, and the non-obvious gotchas: - cvs/lib/utils: substitute_config / BaseVariantConfig / evaluate_all, the 3-pass placeholder order, sibling-glob threshold discovery, parent-first validator ordering. - cvs/lib/inference/utils: load_variant / to_client_metrics / CLIENT_METRICS, the cell_key single-source-of-truth, the coverage check that prevents a silent green, and the validators mirrored in pytest_generate_tests. --- cvs/lib/inference/utils/AGENTS.md | 96 +++++++++++++ cvs/lib/utils/AGENTS.md | 74 ++++++++++ plans/building-a-cvs-test-suite.md | 222 +++++++++++++++++++++++++++++ 3 files changed, 392 insertions(+) create mode 100644 cvs/lib/inference/utils/AGENTS.md create mode 100644 cvs/lib/utils/AGENTS.md create mode 100644 plans/building-a-cvs-test-suite.md diff --git a/cvs/lib/inference/utils/AGENTS.md b/cvs/lib/inference/utils/AGENTS.md new file mode 100644 index 00000000..5770f546 --- /dev/null +++ b/cvs/lib/inference/utils/AGENTS.md @@ -0,0 +1,96 @@ +# cvs/lib/inference/utils — inference-specific helpers + +The inference half of the config/parsing machinery. Generic machinery +(`BaseVariantConfig`, placeholder substitution, `substitute_config`, +`evaluate_all`) lives one level up in `cvs/lib/utils/` — import from there, don't +duplicate it. This dir holds what's specific to *serving* workloads: the sweep +schema and the vLLM client-metric vocabulary. + +## What's here + +- `inferencing_config_loader.py` — the inference config schema: the named-combo + sweep selector (`SeqCombo`/`Run`/`Sweep`), `GoodputSlo`, `Params` (vLLM bench + flags), the `server` role, and `VariantConfig(BaseVariantConfig)` with + `cell_key`/`expected_cells` + the threshold-coverage check. +- `vllm_parsing.py` — pure parsers for vLLM benchmark artifacts: + `to_client_metrics()`, the `client.*` namespace, and `CLIENT_METRICS` (the + display surface). + +## Public entry points + +- `load_variant(config_path, cluster_dict) -> VariantConfig` + The inference loader. Delegates file-read + substitution to + `substitute_config`, attaches thresholds, builds + validates the typed + `VariantConfig`. This is what the suite's `variant_config` fixture calls. +- `to_client_metrics(raw, *, tp, isl) -> {client.*: value}` + Maps a stock `vllm bench serve` results dict to the namespaced metric dict. + **Pure** — no I/O, no orchestration. Caller fetches + json-loads the artifact. +- `CLIENT_METRICS` / `CLIENT_METRIC_UNITS` + The ordered `(short_name, unit)` list that becomes one HTML row per metric. The + single definition every vLLM flavour (single-node, distributed, disagg, + InferenceMax) shares — don't re-list rows per suite. +- `GoodputSlo`, `SeqCombo`, `Run`, `Sweep`, `Params`, `Roles`, `VariantConfig` — + the typed schema. + +## The sweep selector (the headline design point) + +The old schema expanded `sequence_combinations × concurrency_levels` into an NxM +cartesian. This replaces it with **named combos + an explicit `runs[]` list**: + +```json +"sweep": { + "sequence_combinations": [ {"name": "w1_isl=128_osl=2048", "isl":"128", "osl":"2048", ...} ], + "runs": [ {"combo": "w1_isl=128_osl=2048", "concurrency": 16} ] +} +``` + +Each `run` is one `(combo, concurrency)` cell — you enumerate exactly the cells +you want, no explosion. `Sweep`'s `@model_validator` rejects duplicate combo +names and any `run.combo` that names no combo, **at load time**. + +## Gotchas worth not re-discovering + +- **`cell_key` is the single source of truth** for a cell's threshold key + (`ISL=…,OSL=…,TP=…,CONC=…`). The loader's coverage check and the test's verdict + lookup both call it, so they can't drift on whitespace/ordering. If you change + the key format, everything keyed on it (threshold.json, `expected_cells`) moves + together — change it in one place. +- **`_check_thresholds_cover_sweep` is what prevents a silent green.** Without it, + a mistyped threshold key makes the test skip its verdict and report PASS with + zero assertions. When `enforce_thresholds=false` the same mismatch is a + *warning* (record-only mode), not an error — that's intentional for + un-calibrated configs, not a bug to "fix". +- **`pytest_generate_tests` reads raw JSON and bypasses this loader** (it runs at + collection time, before fixtures exist). So the validators here are + **mirrored** by hand in the suite's `pytest_generate_tests` (dup-name check, + unknown-combo check, `GoodputSlo(**…)` validation). If you add a validator to + `Sweep`, mirror it there too or collection won't enforce it. +- **`GoodputSlo` is an INPUT, not a threshold.** It's passed to + `vllm bench serve --goodput`; it lives in the sweep, not threshold.json. + Per-combo because e2el scales with osl. `_Forbid`, so a typo'd SLO key fails + load rather than silently dropping the gate. +- **`to_client_metrics` is deliberately I/O-free.** Artifact layout is + job-specific (single-node `cat`, disagg prefill+decode, distributed rank-0); + the *fetch* lives in the orchestrator (`vllm_orch.parse_results`), the + *transform* lives here so every job reuses it. Keep it that way — don't add + file reads here. +- **Derived metrics degrade to `None`, never crash.** `_safe_div` returns `None` + on missing/None/zero-divisor. `evaluate_all` then reports `None` as a loud + violation (not a `float(None)` TypeError). A metric the run couldn't produce + shows `-` in the table, not a stack trace. +- **`client.goodput` is an alias** for stock's `request_goodput` (the name the + table + threshold file use). Stock leaves `request_goodput` null unless + `--goodput` was passed. +- **`Params` is the only framework-flavoured schema class** (it's the `vllm bench + serve` flag set). `Sweep`/`SeqCombo`/`GoodputSlo`/`Roles`/`cell_key` are + serving-generic. When a second serving framework lands, subclass `Params`; the + rest is reusable as-is. + +## When extending + +- New `client.*` metric → add the derivation in `to_client_metrics` (guard + divisors with `_safe_div`) AND a `(name, unit)` entry in `CLIENT_METRICS`. +- New combo field → add to `SeqCombo` (`_Forbid`, so also update any raw-JSON + reader like `pytest_generate_tests`). +- Second framework → subclass `Params`; reuse everything else here and + `BaseVariantConfig`/`substitute_config` from `cvs/lib/utils/`. diff --git a/cvs/lib/utils/AGENTS.md b/cvs/lib/utils/AGENTS.md new file mode 100644 index 00000000..a1633ff8 --- /dev/null +++ b/cvs/lib/utils/AGENTS.md @@ -0,0 +1,74 @@ +# cvs/lib/utils — shared, framework-agnostic config machinery + +Pure functions and schema any CVS suite (inference, training, …) can call. No +suite-specific knowledge lives here. If something only an inference suite needs +crept in, it belongs in `cvs/lib/inference/utils/` instead — keep this boundary. + +## What's here + +- `config_loader.py` — the generic half of variant-config loading: the + `paths`/`model`/`image`/`container` schema, `BaseVariantConfig`, the 3-pass + placeholder substitution engine, and `substitute_config()`. +- `verdict.py` — `evaluate_all(actuals, thresholds)`: a tiny, framework-neutral + threshold checker. + +## Public entry points + +- `substitute_config(config_path, cluster_dict) -> (raw_dict, thresholds)` + Reads a variant `*_config.json` + its sibling `*threshold.json`, resolves all + placeholders, strips threshold comment keys. Returns the substituted-but- + **unvalidated** dict plus the parsed thresholds. A per-framework loader calls + this, then builds its own typed `VariantConfig(**raw)`. +- `BaseVariantConfig` — subclass it per framework. Carries the shared fields + (`schema_version`, `enforce_thresholds`, `paths`, `model`, `image`, + `container`, `thresholds`) and the `model.remote==1` NotImplementedError guard. +- `evaluate_all(actuals, thresholds)` — raises `ThresholdViolation` listing every + failing metric. Threshold kinds: `min`, `max_ms`, `within` (±tolerance_pct), + `min_tok_s`, `min_ratio` (compares against another metric named in + `reference`). + +## The seam (why this is split out) + +`config_loader.py` used to be one monolith mixing generic machinery with +inference-only schema (`Sweep`/`SeqCombo`/`cell_key`). The generic half stayed +here; the inference half moved to +`cvs.lib.inference.utils.inferencing_config_loader`, which imports +`BaseVariantConfig`, `_Forbid`, and `substitute_config` from here. A second +framework (training) does the same: subclass `BaseVariantConfig`, reuse +`substitute_config` and `evaluate_all`, add its own schema in its own dir. + +## Gotchas worth not re-discovering + +- **Placeholder resolution is 3 ordered passes** (`substitute_config`): + (1) cluster placeholders like `{user-id}` everywhere, (2) self-reference within + the `paths` block (`{shared_fs}` → expanded inside other `paths.*`), + (3) cross-block `{paths.models_dir}` → anywhere. An unknown `{token}` is left + **verbatim**, not errored — a typo'd placeholder surfaces as a literal brace in + a path, not a load failure. +- **Threshold file is found by sibling glob**, not by name: exactly one + `*threshold.json` next to the config. Zero → `FileNotFoundError`; more than one + → `ValueError` (ambiguous). So config and threshold can share a descriptive + prefix (`llama31_70b_fp8_config.json` / `…_threshold.json`) but don't have to. +- **Threshold comment keys** (anything starting with `_`, e.g. `_comment`) are + stripped before the coverage check, so you can document a threshold file inline. +- **`_Forbid` vs `_Allow`**: most schema classes forbid extra keys (a typo fails + load loudly). `RuntimeSpec` is `_Allow` (orchestrator runtime args are + open-ended). Don't loosen `_Forbid` to silence a "valid" key — add the field. +- **`BaseVariantConfig` validators run parent-first.** The remote-not-implemented + check is intentionally first so a remote config fails fast before any + subclass's (meaningless-for-a-rejected-config) coverage check runs. If you add + a base validator, mind the ordering contract. +- **`container.model_dump()` is the orchestrator contract.** The loaded + `container` field serialises to the exact `{lifetime, name, image, runtime: + {name, args}}` shape `OrchestratorConfig.from_configs` consumes. Don't reshape + it here. +- `model.remote==1` raises `NotImplementedError` (remote model download is + unported PoC scope). The schema accepts it; the validator rejects it. + +## When extending + +- New generic threshold kind → add a branch in `verdict._check_one` and document + it above. Keep it metric-name-agnostic. +- New shared config field every suite needs → add to `BaseVariantConfig`. +- Anything inference/training-specific → NOT here. See the sibling + `cvs/lib/inference/utils/AGENTS.md`. diff --git a/plans/building-a-cvs-test-suite.md b/plans/building-a-cvs-test-suite.md new file mode 100644 index 00000000..764819be --- /dev/null +++ b/plans/building-a-cvs-test-suite.md @@ -0,0 +1,222 @@ +# Building a CVS test suite — a reference guide + +This guide documents the base infrastructure for writing a new **inference** or +**training** test suite in CVS, using the `vllm_single` suite (PR against +`dev/dtni`) as the worked reference implementation. If you are adding a serving +framework, a training benchmark, or any new workload, base your structure on the +patterns here rather than on the older `InferenceBaseJob`/`if_dict` style. + +> **Status note.** CVS is experimental. Flags and config keys change, and the +> built-in pass/fail is not a trustworthy oracle on its own — always verify a +> run's artifacts independently. This guide describes the structure, not a frozen +> API. + +--- + +## 1. The layered architecture + +A suite is built from six layers. Each has one job; the seams between them are +the reusable surface a second suite plugs into. + +``` + cluster file (node IP + user/key/orchestrator) <- per-environment, OUT of repo + | + variant config (*_config.json + *threshold.json) <- per-workload, IN repo + | + config loader ── cvs/lib/utils/config_loader.py (generic: schema, substitution) + | └ cvs/lib/inference/utils/inferencing_config_loader.py (framework schema + sweep) + | + orchestrator ── cvs/core/orchestrators (ContainerOrchestrator; provided) + | + job / driver ── cvs/lib/inference/vllm_orch.py (launch server, run client, fetch artifact) + | + suite / pytest ── cvs/tests/inference/vllm/ (lifecycle-as-tests, parametrization, HTML) + | + parsing + verdict ── vllm_parsing.to_client_metrics + cvs/lib/utils/verdict.evaluate_all +``` + +The **generic ↔ framework seam** is the key idea: anything every workload shares +lives in `cvs/lib/utils/` (`BaseVariantConfig`, placeholder substitution, +`substitute_config`, `evaluate_all`); anything specific to *your* workload lives +in your own `cvs/lib//utils/` module that subclasses/reuses it. The +config loader literally documents this as a "generalization seam," and this PR +executes the split. + +--- + +## 2. The two config files (per workload) + +A workload is described by a pair of JSON files sitting in the same directory: + +- `*_config.json` — the variant: paths, model, image, container, params, sweep. +- `*threshold.json` — the per-cell pass/fail thresholds. + +The loader finds the threshold by **sibling glob** (exactly one `*threshold.json` +next to the config), so the two share a descriptive prefix +(`llama31_70b_fp8_config.json` / `llama31_70b_fp8_threshold.json`). + +### Placeholders + +Config values use `{...}` placeholders resolved in three passes: +`{user-id}` (from the cluster file) → `{shared_fs}` (self-reference within +`paths`) → `{paths.models_dir}` (cross-block). An unknown placeholder is left +literal — there is no error, so check a stray brace in a resolved path if +something doesn't mount. + +### enforce_thresholds + +`enforce_thresholds: false` makes the suite **record-only**: it captures every +metric and skips assertions, and a threshold/sweep mismatch warns instead of +failing the load. Use it for un-calibrated workloads (e.g. throughput +characterization where the published numbers are curves, not tabulated cells). +Flip to `true` once you have real numbers — the coverage check then guarantees +every sweep cell has a threshold entry, so a green run can't have silently +skipped its verdict. + +--- + +## 3. The sweep selector (named combos + runs) + +The sweep enumerates exactly the `(sequence-shape, concurrency)` cells to run. +It replaces the old `sequence_combinations × concurrency_levels` cartesian: + +```json +"sweep": { + "sequence_combinations": [ + { "name": "w1_isl=128_osl=2048", "isl": "128", "osl": "2048", + "goodput_slo": { "ttft_ms": 1e9, "tpot_ms": 1e9, "e2el_ms": 1e9 } } + ], + "runs": [ + { "combo": "w1_isl=128_osl=2048", "concurrency": 16 } + ] +} +``` + +- Combos are named once; `runs` cherry-picks `(combo, concurrency)` pairs. No NxM + explosion — you list precisely the cells you want. +- Load-time validation rejects duplicate combo names and any `run.combo` that + names no combo. (This is mirrored in the suite's `pytest_generate_tests`, which + reads raw JSON at collection time before the typed loader runs.) +- `goodput_slo` is an **input** to the run (passed to `vllm bench serve + --goodput`), not a threshold — that's why it lives in the sweep. + +Each cell's threshold key is produced by `VariantConfig.cell_key()` +(`ISL=…,OSL=…,TP=…,CONC=…`). It is the single source of truth shared by the +coverage check and the verdict lookup, so threshold.json keys must match it +exactly. + +--- + +## 4. The job/driver (self-contained, no external .sh) + +`vllm_orch.VllmJob` is the reference driver. It talks only to an injected +orchestrator (`orch.exec`, which routes into the running container) and a typed +`VariantConfig`. Lifecycle highlights to copy: + +- **The server command is built in Python** (`_server_argv`), not cloned from an + external `.sh` repo. A run is self-contained. Per-model quirks come from + `roles.server.extra_serve_args` / `roles.server.env` in config. +- **`/tmp/server_env_script.sh`** is written by `build_server_cmd` and **sourced + by both server and client**, so the two share one environment (HF token, cache + pin, AITER flags). Each value is `shlex.quote`d. +- **`--max-model-len` is derived per cell** from isl/osl/random_range_ratio so a + sweep change stays self-consistent. +- **Readiness/completion are detected by scanning the whole log**, with narrow + failure markers (don't match bare `error:` — ROCm/vLLM logs benign ones). +- **Results come from the stock `results` artifact**, not console-regex. + `parse_results` fetches the extensionless JSON `vllm bench serve` writes to + `--result-dir` and hands it to the pure `to_client_metrics`. Missing/empty/ + unparseable → hard-fail the cell (never a silently-green empty row). + +The fetch lives in the job (artifact layout is job-specific); the transform lives +in `inference.utils` (so distributed/disagg/InferenceMax reuse it). + +--- + +## 5. The suite (lifecycle-as-tests) + +In `cvs/tests/inference/vllm/`, each lifecycle stage is its own pytest test so it +shows up as a timed, pass/fail row in the HTML report: + +``` +test_launch_container → test_setup_sshd → test_model_fetch + → test_vllm_inference (per cell) → test_metric (per metric per cell) + → test_print_results_table → test_teardown +``` + +Patterns to copy: + +- **`pytest_generate_tests`** (in the suite module, not conftest) parametrizes + from the sweep selector. It runs at collection time and reads raw JSON, so it + re-validates combos by hand (mirroring the typed loader). +- **`test_vllm_inference` runs the benchmark once per cell** and stashes results + in a module-scoped `inf_res_dict`. **`test_metric` reads one cached metric** and + is one HTML row per metric per cell — no GPU work, asserts only when + `enforce_thresholds` is true and a spec exists. +- **`_Lifecycle`** (`conftest`) carries cross-test state: `failed` lets a broken + stage skip the rest instead of cascading; `torn_down` lets explicit teardown + suppress the fixture leak-guard finalizer. +- **The `orch` fixture owns only the teardown safety net** — launch/sshd happen + in tests so they're timed rows. A mid-sweep failure still tears the container + down via the finalizer. +- **`single-node guards`**: `test_setup_sshd` only probes port 2224 when + `len(orch.hosts) > 1` (in-container sshd exists only for inter-node MPI). +- **HTML Value/Unit columns** come from `pytest_html_results_table_header/_row` + hooks scoped to this conftest, populated from `metric_value`/`metric_unit` + user-properties. + +--- + +## 6. Parsing + verdict + +- **`to_client_metrics(raw, *, tp, isl)`** — pure: stock keys namespaced + `client.*` 1:1, plus derived metrics (`per_gpu_throughput`, + `decode_throughput_p50`, `success_rate`, …) via `_safe_div` (degrades to + `None`, never crashes). `CLIENT_METRICS` is the ordered display surface. +- **`evaluate_all(actuals, thresholds)`** — generic, framework-neutral. Kinds: + `min`, `max_ms`, `within`, `min_tok_s`, `min_ratio`. Raises `ThresholdViolation` + listing every failure. A `None` actual is a loud violation, not a TypeError. + +--- + +## 7. To add your own suite — checklist + +1. **Schema.** If serving: reuse `inferencing_config_loader` (subclass `Params` + for a new framework's flags). If a new domain (training): create + `cvs/lib//utils/_config_loader.py`, subclass + `BaseVariantConfig`, reuse `substitute_config` + `evaluate_all` from + `cvs/lib/utils/`. Define your own `cell_key` + coverage check. +2. **Config pair.** Write `*_config.json` + `*threshold.json` under + `cvs/input/config_file////`. Start + `enforce_thresholds: false` until calibrated. +3. **Driver.** Write a `Job` class that takes an `orch` + typed config and owns + launch → run → fetch-artifact → `parse`. Build commands in Python; keep the + pure transform in your `utils`. Hard-fail on missing artifacts. +4. **Suite.** Under `cvs/tests///`: `conftest.py` (fixtures + + lifecycle + HTML hooks), the suite module (`pytest_generate_tests` + + lifecycle-as-tests), `_shared.py` (the results table). Pin test order in + `pytest_collection_modifyitems`. +5. **Metrics.** Define your metric vocabulary + units list once in your `utils`, + the way `CLIENT_METRICS` does — don't re-list rows per suite. +6. **Cluster file.** Keep it minimal and OUT of the repo: node IP + user + key + + orchestrator. The variant config supplies the container block; don't ship a + bespoke per-suite cluster file. +7. **Verify independently.** After a run, read the artifact + the HTML cells + yourself. CVS's PASS is not a trustworthy oracle. + +--- + +## 8. Reference files (this PR) + +| Layer | File | +|---|---| +| generic schema + substitution | `cvs/lib/utils/config_loader.py` | +| generic verdict | `cvs/lib/utils/verdict.py` | +| inference schema + sweep | `cvs/lib/inference/utils/inferencing_config_loader.py` | +| client metric vocabulary | `cvs/lib/inference/utils/vllm_parsing.py` | +| driver | `cvs/lib/inference/vllm_orch.py` | +| suite | `cvs/tests/inference/vllm/{vllm_single,conftest,_shared}.py` | +| config pair | `cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/` | + +Agent-facing summaries (entry points + gotchas) live in each package's +`AGENTS.md`. From 8ddb3e1b27cf73c7f79df52b70627e3f14a8798b Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Thu, 18 Jun 2026 22:03:49 -0400 Subject: [PATCH 09/46] docs: expand suite guide with lib restructure, drop redundant section rules Document the dtni -> utils rename and the shared (cvs/lib/utils) vs domain-specific (cvs/lib/inference/utils) split: what lives where, the rule for placing a new helper, and the directory map. Note training is not yet ported and this guide is the blueprint for that port. Remove the manual --- horizontal rules between sections: heading levels already render their own bottom border, so the extra rules produced a double-underline. Minor prose/format cleanups. --- plans/building-a-cvs-test-suite.md | 123 +++++++++++++++++++---------- 1 file changed, 81 insertions(+), 42 deletions(-) diff --git a/plans/building-a-cvs-test-suite.md b/plans/building-a-cvs-test-suite.md index 764819be..d5b6640e 100644 --- a/plans/building-a-cvs-test-suite.md +++ b/plans/building-a-cvs-test-suite.md @@ -4,16 +4,70 @@ This guide documents the base infrastructure for writing a new **inference** or **training** test suite in CVS, using the `vllm_single` suite (PR against `dev/dtni`) as the worked reference implementation. If you are adding a serving framework, a training benchmark, or any new workload, base your structure on the -patterns here rather than on the older `InferenceBaseJob`/`if_dict` style. +patterns here rather than on the older `InferenceBaseJob` / `if_dict` style. > **Status note.** CVS is experimental. Flags and config keys change, and the > built-in pass/fail is not a trustworthy oracle on its own — always verify a > run's artifacts independently. This guide describes the structure, not a frozen > API. ---- +> **Training is not ported yet.** Only `vllm_single` (inference) exists today. +> The library layout, the generic↔domain seam, and the suite skeleton below are +> deliberately framework-neutral so a training suite drops into the same shape — +> this guide is the blueprint for that port, not a record of it. + +## What changed in the restructure + +This PR reorganizes where shared vs. workload-specific code lives, so the next +suite reuses machinery instead of copy-pasting it. + +**`cvs/lib/dtni` → `cvs/lib/utils` (renamed).** `dtni` was a leftover project +codename; the directory actually holds pure, framework-agnostic helpers. The new +name says what it is: utilities any suite (inference, training, …) can call. + +**`cvs/lib/utils/` — the shared/common layer.** Everything here is generic: no +suite knows about vLLM, Megatron, ISL/OSL, or goodput. It holds: +- `config_loader.py` — the generic config skeleton (`BaseVariantConfig`), the + `paths`/`model`/`image`/`container` schema, the 3-pass placeholder + substitution engine, and `substitute_config()` (read a config + its sibling + threshold file, resolve placeholders). +- `verdict.py` — `evaluate_all(actuals, thresholds)`, a metric-name-agnostic + threshold checker (`min`, `max_ms`, `within`, `min_tok_s`, `min_ratio`). + +**`cvs/lib/inference/utils/` — the domain-specific layer.** Helpers only an +*inference* suite needs live here, one level down from the shared dir. It holds: +- `inferencing_config_loader.py` — the inference config schema: the named-combo + sweep selector, `GoodputSlo`, the vLLM bench `Params`, and + `VariantConfig(BaseVariantConfig)` with the ISL/OSL/TP/CONC `cell_key`. It + *imports* `BaseVariantConfig` + `substitute_config` from `cvs/lib/utils` rather + than re-implementing them. +- `vllm_parsing.py` — the `client.*` metric vocabulary (`to_client_metrics`, + `CLIENT_METRICS`): pure transforms from a vLLM benchmark artifact to a + namespaced metric dict. + +**The rule for where a helper goes.** If every workload could use it (config +plumbing, threshold math) → `cvs/lib/utils/`. If it only makes sense for one +domain (sweep shapes, serving metrics) → `cvs/lib//utils/`. A training +port adds `cvs/lib/training/utils/` the same way inference did — subclass +`BaseVariantConfig`, reuse `substitute_config` and `evaluate_all`, add its own +schema and metric vocabulary. The shared layer never grows a dependency on a +specific framework. -## 1. The layered architecture +``` +cvs/lib/ + utils/ # shared, framework-agnostic (was: dtni) + config_loader.py # BaseVariantConfig, substitute_config, placeholders + verdict.py # evaluate_all (generic threshold kinds) + inference/ + utils/ # inference-only helpers + inferencing_config_loader.py # sweep selector, Params, VariantConfig, cell_key + vllm_parsing.py # client.* metrics, to_client_metrics + vllm_orch.py # the driver (VllmJob) + training/ # (future) same shape: + utils/ # training_config_loader.py, _parsing.py +``` + +## The layered architecture A suite is built from six layers. Each has one job; the seams between them are the reusable surface a second suite plugs into. @@ -23,8 +77,8 @@ the reusable surface a second suite plugs into. | variant config (*_config.json + *threshold.json) <- per-workload, IN repo | - config loader ── cvs/lib/utils/config_loader.py (generic: schema, substitution) - | └ cvs/lib/inference/utils/inferencing_config_loader.py (framework schema + sweep) + config loader ── cvs/lib/utils/config_loader.py (generic: schema, substitution) + | └ cvs/lib/inference/utils/inferencing_config_loader.py (domain schema + sweep) | orchestrator ── cvs/core/orchestrators (ContainerOrchestrator; provided) | @@ -35,18 +89,15 @@ the reusable surface a second suite plugs into. parsing + verdict ── vllm_parsing.to_client_metrics + cvs/lib/utils/verdict.evaluate_all ``` -The **generic ↔ framework seam** is the key idea: anything every workload shares -lives in `cvs/lib/utils/` (`BaseVariantConfig`, placeholder substitution, -`substitute_config`, `evaluate_all`); anything specific to *your* workload lives -in your own `cvs/lib//utils/` module that subclasses/reuses it. The -config loader literally documents this as a "generalization seam," and this PR -executes the split. +The **generic↔domain seam** is the key idea: anything every workload shares lives +in `cvs/lib/utils/`; anything specific to *your* workload lives in your own +`cvs/lib//utils/` module that subclasses/reuses it. The config loader +docstring literally calls this the "generalization seam," and this PR executes +the split. ---- +## The two config files (per workload) -## 2. The two config files (per workload) - -A workload is described by a pair of JSON files sitting in the same directory: +A workload is described by a pair of JSON files in the same directory: - `*_config.json` — the variant: paths, model, image, container, params, sweep. - `*threshold.json` — the per-cell pass/fail thresholds. @@ -60,7 +111,7 @@ next to the config), so the two share a descriptive prefix Config values use `{...}` placeholders resolved in three passes: `{user-id}` (from the cluster file) → `{shared_fs}` (self-reference within `paths`) → `{paths.models_dir}` (cross-block). An unknown placeholder is left -literal — there is no error, so check a stray brace in a resolved path if +literal — there is no error, so check for a stray brace in a resolved path if something doesn't mount. ### enforce_thresholds @@ -73,18 +124,16 @@ Flip to `true` once you have real numbers — the coverage check then guarantees every sweep cell has a threshold entry, so a green run can't have silently skipped its verdict. ---- - -## 3. The sweep selector (named combos + runs) +## The sweep selector (named combos + runs) -The sweep enumerates exactly the `(sequence-shape, concurrency)` cells to run. -It replaces the old `sequence_combinations × concurrency_levels` cartesian: +The sweep enumerates exactly the `(sequence-shape, concurrency)` cells to run. It +replaces the old `sequence_combinations × concurrency_levels` cartesian: ```json "sweep": { "sequence_combinations": [ { "name": "w1_isl=128_osl=2048", "isl": "128", "osl": "2048", - "goodput_slo": { "ttft_ms": 1e9, "tpot_ms": 1e9, "e2el_ms": 1e9 } } + "goodput_slo": { "ttft_ms": 1000000000.0, "tpot_ms": 1000000000.0, "e2el_ms": 1000000000.0 } } ], "runs": [ { "combo": "w1_isl=128_osl=2048", "concurrency": 16 } @@ -105,9 +154,7 @@ Each cell's threshold key is produced by `VariantConfig.cell_key()` coverage check and the verdict lookup, so threshold.json keys must match it exactly. ---- - -## 4. The job/driver (self-contained, no external .sh) +## The job/driver (self-contained, no external .sh) `vllm_orch.VllmJob` is the reference driver. It talks only to an injected orchestrator (`orch.exec`, which routes into the running container) and a typed @@ -129,11 +176,9 @@ orchestrator (`orch.exec`, which routes into the running container) and a typed unparseable → hard-fail the cell (never a silently-green empty row). The fetch lives in the job (artifact layout is job-specific); the transform lives -in `inference.utils` (so distributed/disagg/InferenceMax reuse it). - ---- +in `inference/utils` (so distributed/disagg/InferenceMax reuse it). -## 5. The suite (lifecycle-as-tests) +## The suite (lifecycle-as-tests) In `cvs/tests/inference/vllm/`, each lifecycle stage is its own pytest test so it shows up as a timed, pass/fail row in the HTML report: @@ -159,15 +204,13 @@ Patterns to copy: - **The `orch` fixture owns only the teardown safety net** — launch/sshd happen in tests so they're timed rows. A mid-sweep failure still tears the container down via the finalizer. -- **`single-node guards`**: `test_setup_sshd` only probes port 2224 when +- **Single-node guards**: `test_setup_sshd` only probes port 2224 when `len(orch.hosts) > 1` (in-container sshd exists only for inter-node MPI). -- **HTML Value/Unit columns** come from `pytest_html_results_table_header/_row` - hooks scoped to this conftest, populated from `metric_value`/`metric_unit` - user-properties. - ---- +- **HTML Value/Unit columns** come from `pytest_html_results_table_header` / + `_row` hooks scoped to this conftest, populated from + `metric_value`/`metric_unit` user-properties. -## 6. Parsing + verdict +## Parsing + verdict - **`to_client_metrics(raw, *, tp, isl)`** — pure: stock keys namespaced `client.*` 1:1, plus derived metrics (`per_gpu_throughput`, @@ -177,9 +220,7 @@ Patterns to copy: `min`, `max_ms`, `within`, `min_tok_s`, `min_ratio`. Raises `ThresholdViolation` listing every failure. A `None` actual is a loud violation, not a TypeError. ---- - -## 7. To add your own suite — checklist +## To add your own suite — checklist 1. **Schema.** If serving: reuse `inferencing_config_loader` (subclass `Params` for a new framework's flags). If a new domain (training): create @@ -204,9 +245,7 @@ Patterns to copy: 7. **Verify independently.** After a run, read the artifact + the HTML cells yourself. CVS's PASS is not a trustworthy oracle. ---- - -## 8. Reference files (this PR) +## Reference files (this PR) | Layer | File | |---|---| From 00c9cdc98bdea00295678336f7c41465ebc1720a Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 11:41:16 -0400 Subject: [PATCH 10/46] docs: demote headings so GitHub stops underlining sections --- plans/building-a-cvs-test-suite.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/plans/building-a-cvs-test-suite.md b/plans/building-a-cvs-test-suite.md index d5b6640e..986726c2 100644 --- a/plans/building-a-cvs-test-suite.md +++ b/plans/building-a-cvs-test-suite.md @@ -1,4 +1,4 @@ -# Building a CVS test suite — a reference guide +**Building a CVS test suite — a reference guide** This guide documents the base infrastructure for writing a new **inference** or **training** test suite in CVS, using the `vllm_single` suite (PR against @@ -16,7 +16,7 @@ patterns here rather than on the older `InferenceBaseJob` / `if_dict` style. > deliberately framework-neutral so a training suite drops into the same shape — > this guide is the blueprint for that port, not a record of it. -## What changed in the restructure +### What changed in the restructure This PR reorganizes where shared vs. workload-specific code lives, so the next suite reuses machinery instead of copy-pasting it. @@ -67,7 +67,7 @@ cvs/lib/ utils/ # training_config_loader.py, _parsing.py ``` -## The layered architecture +### The layered architecture A suite is built from six layers. Each has one job; the seams between them are the reusable surface a second suite plugs into. @@ -95,7 +95,7 @@ in `cvs/lib/utils/`; anything specific to *your* workload lives in your own docstring literally calls this the "generalization seam," and this PR executes the split. -## The two config files (per workload) +### The two config files (per workload) A workload is described by a pair of JSON files in the same directory: @@ -106,7 +106,7 @@ The loader finds the threshold by **sibling glob** (exactly one `*threshold.json next to the config), so the two share a descriptive prefix (`llama31_70b_fp8_config.json` / `llama31_70b_fp8_threshold.json`). -### Placeholders +#### Placeholders Config values use `{...}` placeholders resolved in three passes: `{user-id}` (from the cluster file) → `{shared_fs}` (self-reference within @@ -114,7 +114,7 @@ Config values use `{...}` placeholders resolved in three passes: literal — there is no error, so check for a stray brace in a resolved path if something doesn't mount. -### enforce_thresholds +#### enforce_thresholds `enforce_thresholds: false` makes the suite **record-only**: it captures every metric and skips assertions, and a threshold/sweep mismatch warns instead of @@ -124,7 +124,7 @@ Flip to `true` once you have real numbers — the coverage check then guarantees every sweep cell has a threshold entry, so a green run can't have silently skipped its verdict. -## The sweep selector (named combos + runs) +### The sweep selector (named combos + runs) The sweep enumerates exactly the `(sequence-shape, concurrency)` cells to run. It replaces the old `sequence_combinations × concurrency_levels` cartesian: @@ -154,7 +154,7 @@ Each cell's threshold key is produced by `VariantConfig.cell_key()` coverage check and the verdict lookup, so threshold.json keys must match it exactly. -## The job/driver (self-contained, no external .sh) +### The job/driver (self-contained, no external .sh) `vllm_orch.VllmJob` is the reference driver. It talks only to an injected orchestrator (`orch.exec`, which routes into the running container) and a typed @@ -178,7 +178,7 @@ orchestrator (`orch.exec`, which routes into the running container) and a typed The fetch lives in the job (artifact layout is job-specific); the transform lives in `inference/utils` (so distributed/disagg/InferenceMax reuse it). -## The suite (lifecycle-as-tests) +### The suite (lifecycle-as-tests) In `cvs/tests/inference/vllm/`, each lifecycle stage is its own pytest test so it shows up as a timed, pass/fail row in the HTML report: @@ -210,7 +210,7 @@ Patterns to copy: `_row` hooks scoped to this conftest, populated from `metric_value`/`metric_unit` user-properties. -## Parsing + verdict +### Parsing + verdict - **`to_client_metrics(raw, *, tp, isl)`** — pure: stock keys namespaced `client.*` 1:1, plus derived metrics (`per_gpu_throughput`, @@ -220,7 +220,7 @@ Patterns to copy: `min`, `max_ms`, `within`, `min_tok_s`, `min_ratio`. Raises `ThresholdViolation` listing every failure. A `None` actual is a loud violation, not a TypeError. -## To add your own suite — checklist +### To add your own suite — checklist 1. **Schema.** If serving: reuse `inferencing_config_loader` (subclass `Params` for a new framework's flags). If a new domain (training): create @@ -245,7 +245,7 @@ Patterns to copy: 7. **Verify independently.** After a run, read the artifact + the HTML cells yourself. CVS's PASS is not a trustworthy oracle. -## Reference files (this PR) +### Reference files (this PR) | Layer | File | |---|---| From 94d2081e2aeb9d1abbfb78ffc8f7e8a4c5b40693 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 11:48:35 -0400 Subject: [PATCH 11/46] removing old plan --- plans/vllm-single-orch-poc.md | 45 ----------------------------------- 1 file changed, 45 deletions(-) delete mode 100644 plans/vllm-single-orch-poc.md diff --git a/plans/vllm-single-orch-poc.md b/plans/vllm-single-orch-poc.md deleted file mode 100644 index 068bd573..00000000 --- a/plans/vllm-single-orch-poc.md +++ /dev/null @@ -1,45 +0,0 @@ -# PoC: vllm_single refactor (orch + typed configs) - -Branch: `dev/dtni`. Worktree: `/data/atnair/repos/cvs_worktrees/cvs-dtni/`. - -Replace 4 byte-similar `vllm_single` wrappers with one parametrized wrapper, per-variant config + threshold dirs, a typed config loader, and a new orch-driven VllmJob. Container lifecycle moves entirely to `ContainerOrchestrator` (`launch: true`). Existing `cvs/lib/inference/{base,vllm,inference_max}.py` untouched (other suites still use them). - -## Approach - -- Add `cvs/lib/dtni/__init__.py`, `cvs/lib/dtni/verdict.py` (~60 LOC; 5 kinds: `min`, `max_ms`, `within`, `min_tok_s`, `min_ratio`; `evaluate_all(actuals, thresholds)`). -- Add `cvs/lib/dtni/config_loader.py` (~110 LOC). Pydantic models with `extra="forbid"`: top-level `schema_version: Literal[1]`, `framework: Literal["vllm_single"]`, `gpu_arch`, `paths` (5 required keys), `model {id, remote: Literal[0,1], precision}`, `image {tag, remote}`, `container` (passthrough shape matching `cvs/core/orchestrators/factory.OrchestratorConfig` — `enabled`, `launch`, `name`, `runtime {name, args}`; `runtime.args` is `extra="allow"`), `roles.server`, `params`, `benchmark_params`, `sweep`. `load_variant(config_path, cluster_dict) -> VariantConfig` loads `config.json` + sibling `threshold.json`, runs 3-pass placeholder substitution: cluster (`{user-id}`) → self-ref (`{shared_fs}` in `paths.*`) → cross-block (`{paths.models_dir}` in `container.runtime.args.volumes`). `model.remote=1` raises `NotImplementedError` pointing at cvs-dtni-v1 `resource_resolver.py`. `enumerate_variants(root_dir) -> list[Path]` walks `cvs/input/dtni/vllm_single/*/`. -- Add `cvs/lib/inference/vllm_orch.py` (~250 LOC). Standalone `VllmJob` — does NOT inherit `InferenceBaseJob`. Takes `orch: ContainerOrchestrator`. Methods: `build_server_cmd`, `start_server`, `is_ready`, `wait_ready`, `run_client`, `wait_client_complete`, `parse_results`, `stop_server`. All exec via `orch.exec` / `orch.exec_on_head` (orch already routes into the container). Drops: dead distributed branch (`self.port_no`), `random_range_ration` typo, `globals.error_list` indirection, silent-skip in `verify_inference_results`. -- Add `cvs/tests/inference/vllm/conftest.py` (~70 LOC): `cluster_dict`, `variant_config`, `orch` (constructs `OrchestratorConfig` from cluster_dict identity + `variant_config.container.dict()`; calls `orch.setup_containers()`, yields, calls `orch.teardown_containers()`), `hf_token`, `inf_res_dict` (session-scoped table). `pytest_generate_tests` parametrizes `test_vllm_inference` over `(seq_combo × concurrency)` from the loaded variant. -- Add `cvs/tests/inference/vllm/_shared.py` (~30 LOC): `test_print_results_table(inf_res_dict)`. `test_cleanup_stale_containers` + `test_launch_inference_containers` collapse into the `orch` fixture lifecycle. -- Add `cvs/tests/inference/vllm/vllm_single.py` (~60 LOC): `from ._shared import *`. Single `test_vllm_inference(orch, variant_config, hf_token, seq_combo, concurrency, inf_res_dict)` — constructs `VllmJob(orch=orch, ...)`, runs `stop_server → build_server_cmd → start_server → wait_ready → run_client → wait_client_complete → parse_results → evaluate_all(actuals, variant_config.thresholds)`. -- Add 4 per-variant dirs `cvs/input/dtni/vllm_single/_perf/{config.json, threshold.json}` for: `Qwen3-Next-80B-A3B-Instruct`, `Qwen3-235B-A22B-Instruct-2507-FP8`, `DeepSeek-V3.1-Terminus`, `gpt-oss-120b`. Each `config.json` carries the existing per-model slice from `mi355x_vllm_single.json` plus the new `paths`/`model`/`image`/`container` blocks. `threshold.json` ports the model's `result_dict` keys. -- Delete `cvs/tests/inference/vllm/vllm_qwen3_80b_single.py` (480), `vllm_qwen3_235b_single.py` (449), `vllm_deepseek31_685b_single.py` (453), `vllm_gpt_oss_120b_single.py` (451), and `cvs/input/config_file/inference/vllm/mi355x_vllm_single.json` (95). - -## Out of scope / Future additions - -- Accuracy variants (`*_accuracy/`, `test_vllm_accuracy`, lm_eval harness, dataset download). -- `cvs/lib/inference/{base,vllm,inference_max}.py` cleanup. Old VllmJob stays; InferenceMaxJob still inherits InferenceBaseJob. -- Orphan `cvs/lib/inference_lib.py` (`InferenceJobFactory` importing non-existent module) — separate cleanup PR. -- `model.remote=1` implementation. Schema present, raises NotImplementedError; port from cvs-dtni-v1 `resource_resolver.py` later. -- `cvs/lib/docker_lib.py` deletion / `cvs/lib/parallel_ssh_lib.py` deprecation cleanup. -- Switching `cvs/core/*` off the deprecated `parallel_ssh_lib` shim (DeprecationWarning will appear in test output — not blocking). -- Other suites (sglang, inferencemax, pytorch_xdit, megatron, jax) — untouched. -- `cvs migrate-config` tool — the 4 variant configs are hand-written. -- Manifest / sidecar / `cvs export` (v1 W4). -- Topology / sweep semantics rework (v1 W5). -- Dev guide (separate doc, written after PoC lands and validates). - -## Verification - -1. `cd /data/atnair/repos/cvs_worktrees/cvs-dtni && python -m pytest --collect-only cvs/tests/inference/vllm/vllm_single.py --config_file=cvs/input/dtni/vllm_single/Qwen3-Next-80B-A3B-Instruct_perf/config.json --cluster_file=` — expect collection lists `test_vllm_inference[balanced-conc16]` plus `test_print_results_table`. No errors. -2. `cvs list vllm_single` — expect enumeration of `test_vllm_inference[...]` for all 4 variants (uses default-walk of `cvs/input/dtni/vllm_single/` when `--config_file=dummy`). Plus `test_print_results_table`. -3. `python -m pytest cvs/tests/inference/vllm/vllm_single.py --config_file=cvs/input/dtni/vllm_single/Qwen3-Next-80B-A3B-Instruct_perf/config.json --cluster_file=` on hardware — expect exit 0. Cell `balanced-conc16` produces a results dict; `test_print_results_table` prints a row matching (within ±10%) the numbers in `vllm_7.14_run.zip / test_print_results_table_*.html`: `Req/s`, `Total tok/s`, `Mean TTFT (ms)`, `Mean TPOT (ms)`, `P99 ITL (ms)`. -4. During the run, `ssh docker ps` shows the `vllm_inference_rocm` container appear at `orch.setup_containers()` (not earlier from `docker_lib`) and disappear at `orch.teardown_containers()`. Container is launched by orch, not by VllmJob. -5. `python -m pytest cvs/tests/inference/vllm/vllm_single.py --config_file=/config.json --cluster_file=...` where `model.remote=0` and `models_dir/` does NOT exist — expect a clean assertion failure naming the missing path, not a runtime crash deep in VllmJob. -6. `python -c "from cvs.lib.dtni.config_loader import load_variant; load_variant('', {})"` — expect Pydantic `ValidationError` mentioning the unknown field. (Catches the v1 typo class the spec called out.) - -## Open questions - -1. Cluster file path for verification 1–4 — which existing mi355x cluster file should the PoC default to? -2. `test_cleanup_stale_containers` + `test_launch_inference_containers` — accept that they vanish from the HTML report (fixture lifecycle replaces them), or keep 5-line stub tests that re-check orch state to preserve the report shape? -3. New Job class name — `cvs.lib.inference.vllm_orch.VllmJob` (same class name, different module) or `VllmOrchJob` (clearly different during transition)? From 488e9d801ed3ace3744d8c93bfe1f765d0cdb6b6 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 13:03:23 -0400 Subject: [PATCH 12/46] fix(config): re-key W1 threshold to the swept CONC=16 cell The threshold file carried placeholder CONC=64/128/256 entries while the sweep's only run is concurrency 16, so cell_key() matched no threshold. The mismatch was masked by enforce_thresholds=false (warned, not raised) and would have failed load the instant enforcement was flipped on. Re-key to the single CONC=16 cell the runs selector actually enumerates. --- .../llama31_70b_fp8_threshold.json | 32 ++----------------- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json index 9b1aff0f..8e521a95 100644 --- a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json +++ b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json @@ -1,34 +1,6 @@ { - "_comment": "PLACEHOLDER thresholds for W1 (Llama 3.1 70B FP8-KV, in=128/out=2048, TP=8). AMD's published source (performance-results.html) gives throughput-vs-latency curves, not tabulated per-cell numbers, so these are not yet calibrated. config.json sets enforce_thresholds=false: the suite records metrics and skips pass/fail. Metric keys are namespaced client.* (from the stock results artifact). Replace min_tok_s/max_ms with real numbers and flip enforce_thresholds=true to make this cell assert.", - "ISL=128,OSL=2048,TP=8,CONC=64": { - "client.total_token_throughput": { - "kind": "min_tok_s", - "value": 0 - }, - "client.mean_ttft_ms": { - "kind": "max_ms", - "value": 1000000 - }, - "client.mean_tpot_ms": { - "kind": "max_ms", - "value": 1000000 - } - }, - "ISL=128,OSL=2048,TP=8,CONC=128": { - "client.total_token_throughput": { - "kind": "min_tok_s", - "value": 0 - }, - "client.mean_ttft_ms": { - "kind": "max_ms", - "value": 1000000 - }, - "client.mean_tpot_ms": { - "kind": "max_ms", - "value": 1000000 - } - }, - "ISL=128,OSL=2048,TP=8,CONC=256": { + "_comment": "PLACEHOLDER thresholds for W1 (Llama 3.1 70B FP8-KV, in=128/out=2048, TP=8, conc=16) -- the single cell the sweep's runs selector enumerates. AMD's published source (performance-results.html) gives throughput-vs-latency curves, not tabulated per-cell numbers, so these are not yet calibrated. config.json sets enforce_thresholds=false: the suite records metrics and skips pass/fail. Metric keys are namespaced client.* (from the stock results artifact). Replace min_tok_s/max_ms with real numbers and flip enforce_thresholds=true to make this cell assert.", + "ISL=128,OSL=2048,TP=8,CONC=16": { "client.total_token_throughput": { "kind": "min_tok_s", "value": 0 From 11c79482ae5072841fc8d522296d92ea25da41bb Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 13:03:23 -0400 Subject: [PATCH 13/46] fix(inference): drop dead server-env exports from build_server_cmd MODEL/ISL/OSL/MAX_MODEL_LEN/RANDOM_RANGE_RATIO/TP/CONC/PORT were exported into /tmp/server_env_script.sh but read by nothing after the .sh->Python server command refactor -- both _server_argv and run_client pass these as explicit flags. Keep only the env the vllm process actually consumes (HF token, HF cache pin, AITER flags). Also drops the second _derive_max_model_len call that fed the dead MAX_MODEL_LEN export. --- cvs/lib/inference/vllm_orch.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/cvs/lib/inference/vllm_orch.py b/cvs/lib/inference/vllm_orch.py index 85f93686..b38480a5 100644 --- a/cvs/lib/inference/vllm_orch.py +++ b/cvs/lib/inference/vllm_orch.py @@ -181,20 +181,16 @@ def _derive_max_model_len(self): def build_server_cmd(self): """Write the server-env script (sourced by both server and client) and create the per-node out-dir inside the container.""" + # Only the HF cache pin + token and the AITER tuning flags are read by + # the vllm process. The server and client commands are Python-built and + # pass every other value (model, isl/osl, tp, port, max-model-len) as an + # explicit flag, so exporting them here would be dead. env_lines = [ - f"export MODEL={shlex.quote(self.model_id)}", - f"export ISL={shlex.quote(self.isl)}", - f"export OSL={shlex.quote(self.osl)}", - f"export MAX_MODEL_LEN={shlex.quote(self._derive_max_model_len())}", - f"export RANDOM_RANGE_RATIO={shlex.quote(self.random_range_ratio)}", - f"export TP={shlex.quote(str(self.tp))}", - f"export CONC={shlex.quote(self.concurrency)}", f"export HF_TOKEN={shlex.quote(self.hf_token)}", f"export HF_HUB_CACHE={shlex.quote(self.models_dir)}", "export VLLM_USE_AITER_UNIFIED_ATTENTION=1", "export VLLM_ROCM_USE_AITER_MHA=0", "export VLLM_ROCM_USE_AITER_FUSED_MOE_A16W4=1", - f"export PORT={shlex.quote(str(self.port_no))}", ] # Per-model env overrides win over the defaults above (appended last). for k, v in self.server_env.items(): From c51787728826de84788e5ad0f7fa2d5d56964a2f Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 13:03:23 -0400 Subject: [PATCH 14/46] fix(inference): move --kv-cache-dtype out of the driver into config _server_argv hard-coded --kv-cache-dtype fp8, baking a per-model property into the shared orchestrator -- a non-fp8-KV model dropped into the suite would be served wrong with no config recourse (extra_serve_args can only add, so an override would pass the flag twice). Declare it in the W1 config's roles.server.extra_serve_args instead; the driver stays model-agnostic and 'new model = new config' holds. --- .../w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json | 7 ++++++- cvs/lib/inference/utils/inferencing_config_loader.py | 6 +++--- cvs/lib/inference/vllm_orch.py | 8 +++----- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json index 43b2e8c0..de89fcbf 100644 --- a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json +++ b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json @@ -36,7 +36,12 @@ } }, "roles": { - "server": {} + "server": { + "extra_serve_args": [ + "--kv-cache-dtype", + "fp8" + ] + } }, "params": { "backend": "vllm", diff --git a/cvs/lib/inference/utils/inferencing_config_loader.py b/cvs/lib/inference/utils/inferencing_config_loader.py index eb8a35fa..21c160d5 100644 --- a/cvs/lib/inference/utils/inferencing_config_loader.py +++ b/cvs/lib/inference/utils/inferencing_config_loader.py @@ -36,9 +36,9 @@ class RoleServer(_Forbid): # The `vllm serve` command is built in Python (cvs.lib.inference.vllm_orch), # not cloned from a `.sh` script, so a run is self-contained. Per-model # server quirks live here: extra `vllm serve` flags and extra env vars - # merged over the defaults the orchestrator sets. Both default empty -- the - # fp8-kv cell needs neither (its --kv-cache-dtype is set unconditionally in - # code). + # merged over the defaults the orchestrator sets. Both default empty; the + # fp8-kv cell sets its --kv-cache-dtype via extra_serve_args, kept out of + # the generic driver so it stays model-agnostic. extra_serve_args: List[str] = [] env: Dict[str, str] = {} diff --git a/cvs/lib/inference/vllm_orch.py b/cvs/lib/inference/vllm_orch.py index b38480a5..ea49480c 100644 --- a/cvs/lib/inference/vllm_orch.py +++ b/cvs/lib/inference/vllm_orch.py @@ -204,9 +204,9 @@ def _server_argv(self): """The `vllm serve` arg list for this cell. Built in Python (mirrors run_client) so a run is self-contained -- no - external `.sh` to clone/stage. --kv-cache-dtype fp8 is set - unconditionally (the model is FP8-KV); per-model extras come from - roles.server.extra_serve_args. + external `.sh` to clone/stage. Only framework-generic flags are set + here; per-model knobs (e.g. --kv-cache-dtype for an FP8-KV model) come + from roles.server.extra_serve_args so this driver stays model-agnostic. """ argv = [ "vllm", @@ -218,8 +218,6 @@ def _server_argv(self): self._derive_max_model_len(), "--port", str(self.port_no), - "--kv-cache-dtype", - "fp8", ] argv.extend(self.extra_serve_args) return argv From bbde541d421d8b2d4d312f27556aec934133cf90 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 13:03:23 -0400 Subject: [PATCH 15/46] refactor(inference): share the sweep-selector validator across load and collection pytest_generate_tests hand-reimplemented the duplicate-name and unknown-run.combo checks that Sweep._check_runs_reference_known_combos already enforces, with divergent semantics (first-failure raise vs all-at-once). Extract validate_sweep_selector() as the single home and call it from both the typed validator (load time) and the collection-time raw-JSON path so the rule can't drift. --- .../utils/inferencing_config_loader.py | 40 ++++++++++++------- cvs/tests/inference/vllm/vllm_single.py | 27 +++++-------- 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/cvs/lib/inference/utils/inferencing_config_loader.py b/cvs/lib/inference/utils/inferencing_config_loader.py index 21c160d5..78f74777 100644 --- a/cvs/lib/inference/utils/inferencing_config_loader.py +++ b/cvs/lib/inference/utils/inferencing_config_loader.py @@ -74,26 +74,38 @@ class Run(_Forbid): concurrency: int +def validate_sweep_selector(combo_names, run_combo_refs): + """The sweep-selector rule: combo names unique, every run.combo names one. + + The single home for this check, shared by the typed `Sweep` validator (load + time) and `pytest_generate_tests` (collection time, which reads raw JSON + before the loader runs) so the two can never drift. Operates on plain lists + of strings so both call sites can feed it. + + Without it a duplicate name silently shadows a combo and a typo'd + `run.combo` is a silently-dropped cell -- either way the sweep runs a + different matrix than the config reads. + """ + names = list(combo_names) + dupes = sorted({n for n in names if names.count(n) > 1}) + if dupes: + raise ValueError(f"duplicate sequence_combination names: {dupes}") + known = set(names) + unknown = sorted({r for r in run_combo_refs if r not in known}) + if unknown: + raise ValueError(f"run.combo names no sequence_combination: {unknown} (known: {sorted(known)})") + + class Sweep(_Forbid): sequence_combinations: List[SeqCombo] runs: List[Run] @model_validator(mode="after") def _check_runs_reference_known_combos(self): - """Combo names must be unique and every run.combo must name one. - - Without this a duplicate name silently shadows a combo and a typo'd - `run.combo` is a silently-dropped cell -- either way the sweep runs a - different matrix than the config reads. - """ - names = [c.name for c in self.sequence_combinations] - dupes = sorted({n for n in names if names.count(n) > 1}) - if dupes: - raise ValueError(f"duplicate sequence_combination names: {dupes}") - known = set(names) - unknown = sorted({r.combo for r in self.runs if r.combo not in known}) - if unknown: - raise ValueError(f"run.combo names no sequence_combination: {unknown} (known: {sorted(known)})") + validate_sweep_selector( + [c.name for c in self.sequence_combinations], + [r.combo for r in self.runs], + ) return self diff --git a/cvs/tests/inference/vllm/vllm_single.py b/cvs/tests/inference/vllm/vllm_single.py index 22a32235..c82ff1a4 100644 --- a/cvs/tests/inference/vllm/vllm_single.py +++ b/cvs/tests/inference/vllm/vllm_single.py @@ -13,7 +13,7 @@ import pytest from cvs.lib import globals -from cvs.lib.inference.utils.inferencing_config_loader import GoodputSlo +from cvs.lib.inference.utils.inferencing_config_loader import GoodputSlo, validate_sweep_selector from cvs.lib.utils.verdict import evaluate_all from cvs.lib.inference.utils.vllm_parsing import CLIENT_METRICS as _METRICS, CLIENT_METRIC_UNITS as _METRIC_UNITS from cvs.lib.inference.vllm_orch import VllmJob @@ -65,28 +65,19 @@ def pytest_generate_tests(metafunc): for combo in combos: if combo.get("goodput_slo") is not None: GoodputSlo(**combo["goodput_slo"]) - # Build the name->combo map and mirror the typed Sweep validator here (this - # path reads raw JSON before load_variant runs): a duplicate combo name or a - # run referencing an unknown combo must fail collection, not silently drop. - by_name = {} - for combo in combos: - nm = combo["name"] - if nm in by_name: - raise ValueError(f"duplicate sequence_combination name: {nm!r}") - by_name[nm] = combo + # Mirror the typed Sweep validator here (this path reads raw JSON before + # load_variant runs) via the shared rule so the two cannot drift: a + # duplicate combo name or a run referencing an unknown combo must fail + # collection, not silently drop. + validate_sweep_selector([c["name"] for c in combos], [r["combo"] for r in runs]) + by_name = {c["name"]: c for c in combos} cases = [] ids = [] for run in runs: - combo_name = run["combo"] - if combo_name not in by_name: - raise ValueError( - f"run.combo {combo_name!r} names no sequence_combination " - f"(known: {sorted(by_name)})" - ) - combo = by_name[combo_name] + combo = by_name[run["combo"]] conc = run["concurrency"] cases.append((combo, conc)) - ids.append(combo_name + "-conc" + str(conc)) + ids.append(run["combo"] + "-conc" + str(conc)) if "metric" in metafunc.fixturenames: if cases: metric_cases = [] From 022920e1547313605ff0d9573f609e1469c2a339 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 13:03:23 -0400 Subject: [PATCH 16/46] fix(inference): key the per-cell out_dir by isl/osl/conc out_dir was fixed per job, so a multi-cell sweep would overwrite each cell's `results` and client.log, and parse_results could cat a prior cell's stale artifact if the current cell's client failed to write one. Key it by cell. Latent today (the shipped sweep has one cell). --- cvs/lib/inference/vllm_orch.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cvs/lib/inference/vllm_orch.py b/cvs/lib/inference/vllm_orch.py index ea49480c..6fc22a8c 100644 --- a/cvs/lib/inference/vllm_orch.py +++ b/cvs/lib/inference/vllm_orch.py @@ -151,8 +151,11 @@ def __init__( # every run. Same value the model-fetch test polls with `du`. self.models_dir = variant.paths.models_dir - # Single-node: one output directory. - self.out_dir = f"{self.log_dir}/{self.log_subdir}/out-node0" + # Single-node, per-cell output directory. Keyed by the cell (isl/osl/ + # conc) so a multi-cell sweep does not overwrite an earlier cell's + # artifacts -- and so parse_results can never cat a stale `results` from + # a prior cell when the current cell's client failed to write one. + self.out_dir = f"{self.log_dir}/{self.log_subdir}/out-node0/isl{self.isl}_osl{self.osl}_conc{self.concurrency}" self.server_log = f"{self.out_dir}/vllm_serve_server.log" self.client_log = f"{self.out_dir}/client.log" From e63255ed3de10aaaa22118a498d9882ad2f7264a Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 13:03:23 -0400 Subject: [PATCH 17/46] refactor(inference): normalize goodput_slo to dict-only run_client accepted goodput_slo as either a dict (.get) or an object (getattr) via a per-key hasattr branch, but the only production caller passes a raw dict -- the object path existed solely for a unit test, and the dual path meant the typed GoodputSlo's validation never reached the command builder. Consume the dict only and drop the object-form test. --- cvs/lib/inference/unittests/test_vllm_orch_parse.py | 7 ------- cvs/lib/inference/vllm_orch.py | 6 +----- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/cvs/lib/inference/unittests/test_vllm_orch_parse.py b/cvs/lib/inference/unittests/test_vllm_orch_parse.py index 6b635c3d..5f994a95 100644 --- a/cvs/lib/inference/unittests/test_vllm_orch_parse.py +++ b/cvs/lib/inference/unittests/test_vllm_orch_parse.py @@ -166,13 +166,6 @@ def test_goodput_flag_built_from_slo_dict(self): for tok in ("ttft:500.0", "tpot:50.0", "e2el:60000.0"): self.assertIn(tok, cmd) - def test_goodput_flag_built_from_slo_object(self): - # config_loader.GoodputSlo is a pydantic object (attr access), not a dict. - slo = SimpleNamespace(ttft_ms=500.0, tpot_ms=50.0, e2el_ms=60000.0) - cmd = self._client_cmd(slo) - self.assertIn("--goodput", cmd) - self.assertIn("ttft:500.0", cmd) - class TestKeyConsistency(unittest.TestCase): """Mechanical guard (verification #5): every key the table reads and every diff --git a/cvs/lib/inference/vllm_orch.py b/cvs/lib/inference/vllm_orch.py index 6fc22a8c..f2332036 100644 --- a/cvs/lib/inference/vllm_orch.py +++ b/cvs/lib/inference/vllm_orch.py @@ -333,11 +333,7 @@ def run_client(self): if self.goodput_slo: args.append("--goodput") for metric, key in (("ttft", "ttft_ms"), ("tpot", "tpot_ms"), ("e2el", "e2el_ms")): - val = ( - self.goodput_slo.get(key) - if hasattr(self.goodput_slo, "get") - else getattr(self.goodput_slo, key, None) - ) + val = self.goodput_slo.get(key) if val is not None: args.append(f"{metric}:{val}") bench_cmd = " ".join(shlex.quote(str(a)) for a in args) From 6433f8655eddd13fa40e1da11dc8246af0185297 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 13:03:23 -0400 Subject: [PATCH 18/46] test(inference): drop unused _fake_variant parameter goodput_slo_unused was never read (goodput is threaded through _make_job). --- cvs/lib/inference/unittests/test_vllm_orch_parse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cvs/lib/inference/unittests/test_vllm_orch_parse.py b/cvs/lib/inference/unittests/test_vllm_orch_parse.py index 5f994a95..fe273ec7 100644 --- a/cvs/lib/inference/unittests/test_vllm_orch_parse.py +++ b/cvs/lib/inference/unittests/test_vllm_orch_parse.py @@ -42,7 +42,7 @@ def exec(self, cmd, **kwargs): return self.exec_return -def _fake_variant(goodput_slo_unused=None): +def _fake_variant(): """A SimpleNamespace tree carrying exactly the attributes VllmJob.__init__ reads.""" params = SimpleNamespace( tensor_parallelism=str(_TP), From 3485e643b7552c0948083b4ca4dda4b2ffd8b1fd Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 14:31:48 -0400 Subject: [PATCH 19/46] chore(inference): placeholder personal/image refs in example config The committed vllm_single example config carried a personal hf-token path and a concrete image tag (duplicated in image.tag and container.image). Replace all three with so the file is a template a new user must fill in -- it still loads (collection works) and only fails at run time when an unedited value is read, which is the intended signal. --- .../w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json index de89fcbf..7ea4636f 100644 --- a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json +++ b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json @@ -7,7 +7,7 @@ "shared_fs": "/home/{user-id}", "models_dir": "{shared_fs}/models", "log_dir": "{shared_fs}/LOGS", - "hf_token_file": "/data/atnair/.hf_token" + "hf_token_file": "" }, "model": { "id": "amd/Llama-3.1-70B-Instruct-FP8-KV", @@ -15,13 +15,13 @@ "precision": "fp8" }, "image": { - "tag": "rocm/vllm-dev:nightly", + "tag": "", "remote": 1 }, "container": { "lifetime": "per_run", "name": "w1_llama31_70b_fp8kv_perf_inference_rocm", - "image": "rocm/vllm-dev:nightly", + "image": "", "runtime": { "name": "docker", "args": { From cea80da76fa95453a983af334fed6165787c98f4 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 14:32:46 -0400 Subject: [PATCH 20/46] refactor(inference): server serve_args as a {flag: value} map The per-model server knobs were a flat [flag, value, flag, value] list (roles.server.extra_serve_args), which reads poorly. Replace with a roles.server.serve_args {flag: value} map (flag without the leading --): a scalar renders --flag value, True a bare --flag, a list the flag once per element -- so it stays readable while still covering vllm bare/repeatable flags. _server_argv flattens the map via a new _flatten_serve_args helper; the derived flags (tp/max-model-len/port) stay code-built. Also repoints a stale unit test that asserted MAX_MODEL_LEN in the env script (it moved to the --max-model-len flag in an earlier commit) to assert against the server argv instead. --- .../llama31_70b_fp8_config.json | 7 ++-- .../unittests/test_vllm_orch_parse.py | 11 ++++--- .../utils/inferencing_config_loader.py | 14 +++++--- cvs/lib/inference/vllm_orch.py | 33 ++++++++++++++++--- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json index 7ea4636f..11bd6d33 100644 --- a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json +++ b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json @@ -37,10 +37,9 @@ }, "roles": { "server": { - "extra_serve_args": [ - "--kv-cache-dtype", - "fp8" - ] + "serve_args": { + "kv-cache-dtype": "fp8" + } } }, "params": { diff --git a/cvs/lib/inference/unittests/test_vllm_orch_parse.py b/cvs/lib/inference/unittests/test_vllm_orch_parse.py index fe273ec7..5b71bd10 100644 --- a/cvs/lib/inference/unittests/test_vllm_orch_parse.py +++ b/cvs/lib/inference/unittests/test_vllm_orch_parse.py @@ -62,7 +62,7 @@ def _fake_variant(): return SimpleNamespace( params=params, model=SimpleNamespace(id="amd/Llama-3.1-70B-Instruct-FP8-KV"), - roles=SimpleNamespace(server=SimpleNamespace(extra_serve_args=[], env={})), + roles=SimpleNamespace(server=SimpleNamespace(serve_args={}, env={})), paths=SimpleNamespace(log_dir="/tmp/logs", models_dir="/tmp/models"), ) @@ -328,10 +328,13 @@ def test_prefix_len_added(self): # 2176 + 64 + 8 = 2248. self.assertEqual(job._derive_max_model_len(), "2248") - def test_env_script_carries_derived_value(self): + def test_server_argv_carries_derived_value(self): + # The derived max-model-len is passed as the --max-model-len flag on the + # `vllm serve` argv (it is no longer exported into the env script). job = _make_job(FakeOrch()) - cmd = self._env_cmd(job) - self.assertIn("MAX_MODEL_LEN=3925", cmd) + argv = job._server_argv() + self.assertIn("--max-model-len", argv) + self.assertEqual(argv[argv.index("--max-model-len") + 1], "3925") import importlib.util as _ilu_t diff --git a/cvs/lib/inference/utils/inferencing_config_loader.py b/cvs/lib/inference/utils/inferencing_config_loader.py index 78f74777..f9eff039 100644 --- a/cvs/lib/inference/utils/inferencing_config_loader.py +++ b/cvs/lib/inference/utils/inferencing_config_loader.py @@ -21,7 +21,7 @@ from __future__ import annotations import warnings -from typing import Dict, List, Optional +from typing import Any, Dict, List, Optional from pydantic import model_validator from typing_extensions import Literal @@ -35,11 +35,15 @@ class RoleServer(_Forbid): # The `vllm serve` command is built in Python (cvs.lib.inference.vllm_orch), # not cloned from a `.sh` script, so a run is self-contained. Per-model - # server quirks live here: extra `vllm serve` flags and extra env vars + # server quirks live here: extra `vllm serve` flags (serve_args) and env vars # merged over the defaults the orchestrator sets. Both default empty; the - # fp8-kv cell sets its --kv-cache-dtype via extra_serve_args, kept out of - # the generic driver so it stays model-agnostic. - extra_serve_args: List[str] = [] + # fp8-kv cell sets its --kv-cache-dtype via serve_args, kept out of the + # generic driver so it stays model-agnostic. A {flag: value} map (flag + # without the leading --): a scalar renders `--flag value`, True renders a + # bare `--flag`, and a list renders the flag once per element. Cleaner than + # a flat [flag, value, flag, value] list and still covers vllm's bare and + # repeatable flags. + serve_args: Dict[str, Any] = {} env: Dict[str, str] = {} diff --git a/cvs/lib/inference/vllm_orch.py b/cvs/lib/inference/vllm_orch.py index f2332036..f1d60447 100644 --- a/cvs/lib/inference/vllm_orch.py +++ b/cvs/lib/inference/vllm_orch.py @@ -141,7 +141,7 @@ def __init__( # Per-model server quirks from config (both default empty): extra # `vllm serve` flags and extra env vars merged over the orchestrator's # defaults. The server command itself is Python-built (no .sh script). - self.extra_serve_args = list(variant.roles.server.extra_serve_args) + self.serve_args = dict(variant.roles.server.serve_args) self.server_env = dict(variant.roles.server.env) # Pin the HF cache onto the mounted models dir. The container binds # models_dir both at /models and (via the home bind mount) at its own @@ -203,13 +203,36 @@ def build_server_cmd(self): self.orch.exec("bash -c " + shlex.quote(f"printf '%s' {shlex.quote(env_script)} > /tmp/server_env_script.sh")) self.orch.exec(f"mkdir -p {shlex.quote(self.out_dir)}") + @staticmethod + def _flatten_serve_args(mapping): + """A {flag: value} serve-args map -> a flat `vllm serve` arg list. + + Flags are given without the leading `--`. A scalar renders + `--flag `; True renders a bare `--flag` (e.g. --trust-remote-code); + a list renders the flag once per element (a repeatable flag). This keeps + config readable ({"kv-cache-dtype": "fp8"}) while still covering vllm's + bare and repeatable flags, which a flat list could express but not read. + """ + argv = [] + for flag, value in mapping.items(): + opt = f"--{flag}" + if value is True: + argv.append(opt) + elif isinstance(value, (list, tuple)): + for v in value: + argv.extend([opt, str(v)]) + else: + argv.extend([opt, str(value)]) + return argv + def _server_argv(self): """The `vllm serve` arg list for this cell. Built in Python (mirrors run_client) so a run is self-contained -- no - external `.sh` to clone/stage. Only framework-generic flags are set - here; per-model knobs (e.g. --kv-cache-dtype for an FP8-KV model) come - from roles.server.extra_serve_args so this driver stays model-agnostic. + external `.sh` to clone/stage. Only the derived, framework-generic flags + (tp/max-model-len/port, computed per cell) are set here; per-model knobs + (e.g. --kv-cache-dtype for an FP8-KV model) come from roles.server.serve_args + so this driver stays model-agnostic. """ argv = [ "vllm", @@ -222,7 +245,7 @@ def _server_argv(self): "--port", str(self.port_no), ] - argv.extend(self.extra_serve_args) + argv.extend(self._flatten_serve_args(self.serve_args)) return argv def start_server(self): From f217c9db852c15b93ab453e2b55dd2473f73fdde Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 15:17:53 -0400 Subject: [PATCH 21/46] feat(verdict): add unit-agnostic max threshold kind The only ceiling kind was max_ms, whose message hard-codes ms. A count metric like client.failed needs an upper bound without the unit lie; add a plain max with the same comparison and an honest message. --- cvs/lib/utils/verdict.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cvs/lib/utils/verdict.py b/cvs/lib/utils/verdict.py index d6f0a498..3f421e17 100644 --- a/cvs/lib/utils/verdict.py +++ b/cvs/lib/utils/verdict.py @@ -27,6 +27,12 @@ def _check_one(metric, actual_raw, spec): target = _to_float(spec["value"]) if actual > target: return f"{metric}: actual {actual} ms > max {target} ms" + elif kind == "max": + # Unit-agnostic upper bound, for counts like `failed` where `max_ms` + # would be a unit lie. Same comparison, honest message. + target = _to_float(spec["value"]) + if actual > target: + return f"{metric}: actual {actual} > max {target}" elif kind == "within": target = _to_float(spec["value"]) pct = _to_float(spec["tolerance_pct"]) From f9ab9a02271e85e96d274fb67e36b036c6eb4dce Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 15:18:43 -0400 Subject: [PATCH 22/46] feat(inference): enforce a declared gated-metric SLO contract Previously only cell-presence was validated: a cell could exist while a given metric had no spec, and test_metric (spec is None -> return) would silently report a green record-only row even under enforce_thresholds=true. A new perf metric was thus unvalidated by default. Declare GATED_METRICS beside CLIENT_METRICS -- the perf+health subset that must assert (throughput, mean+p99 latency, success_rate/failed) -- and extend _check_thresholds_cover_sweep to require a spec for every gated metric in every present cell, reusing the same enforce-vs-warn path. A new metric is record-only until added to the set; once gated, the loader forces a spec in every cell before the suite can run green. Inputs, totals, and derived diagnostics stay record-only by design. --- .../llama31_70b_fp8_threshold.json | 30 +++++++- .../test_inferencing_config_loader.py | 76 +++++++++++++++++++ .../unittests/test_vllm_orch_parse.py | 13 ++++ .../utils/inferencing_config_loader.py | 22 +++++- cvs/lib/inference/utils/vllm_parsing.py | 35 +++++++++ 5 files changed, 172 insertions(+), 4 deletions(-) diff --git a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json index 8e521a95..0bcb5b47 100644 --- a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json +++ b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json @@ -1,10 +1,14 @@ { - "_comment": "PLACEHOLDER thresholds for W1 (Llama 3.1 70B FP8-KV, in=128/out=2048, TP=8, conc=16) -- the single cell the sweep's runs selector enumerates. AMD's published source (performance-results.html) gives throughput-vs-latency curves, not tabulated per-cell numbers, so these are not yet calibrated. config.json sets enforce_thresholds=false: the suite records metrics and skips pass/fail. Metric keys are namespaced client.* (from the stock results artifact). Replace min_tok_s/max_ms with real numbers and flip enforce_thresholds=true to make this cell assert.", + "_comment": "PLACEHOLDER thresholds for W1 (Llama 3.1 70B FP8-KV, in=128/out=2048, TP=8, conc=16) -- the single cell the sweep's runs selector enumerates. Every gated metric (the SLO contract in vllm_parsing.GATED_METRICS: throughput, mean+p99 latency, success_rate/failed health) has a spec here so the loader's gated-coverage check passes; the values are NOT calibrated (AMD's published source gives throughput-vs-latency curves, not per-cell scalars). config.json sets enforce_thresholds=false: the suite records metrics and skips pass/fail. Replace the placeholder values with real numbers and flip enforce_thresholds=true to assert.", "ISL=128,OSL=2048,TP=8,CONC=16": { "client.total_token_throughput": { "kind": "min_tok_s", "value": 0 }, + "client.output_throughput": { + "kind": "min_tok_s", + "value": 0 + }, "client.mean_ttft_ms": { "kind": "max_ms", "value": 1000000 @@ -12,6 +16,30 @@ "client.mean_tpot_ms": { "kind": "max_ms", "value": 1000000 + }, + "client.p99_ttft_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.p99_tpot_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.p99_itl_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.p99_e2el_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.success_rate": { + "kind": "min", + "value": 0 + }, + "client.failed": { + "kind": "max", + "value": 1000000000 } } } diff --git a/cvs/lib/inference/unittests/test_inferencing_config_loader.py b/cvs/lib/inference/unittests/test_inferencing_config_loader.py index c1ba98b8..9c8e22b8 100644 --- a/cvs/lib/inference/unittests/test_inferencing_config_loader.py +++ b/cvs/lib/inference/unittests/test_inferencing_config_loader.py @@ -19,12 +19,26 @@ Sweep, VariantConfig, ) +from cvs.lib.inference.utils.vllm_parsing import GATED_METRICS def _combo(name, isl="128", osl="2048"): return SeqCombo(name=name, isl=isl, osl=osl) +def _full_gated_specs(): + """A spec for every gated metric -- the minimum that satisfies coverage. + + Values are inert (a 0 floor / huge ceiling) so the set passes without + asserting anything; these tests pin the coverage gate, not the numbers. + """ + out = {} + for m in GATED_METRICS: + kind = "max_ms" if m.endswith("_ms") else "max" if m == "failed" else "min" + out[f"client.{m}"] = {"kind": kind, "value": 0 if kind == "min" else 1e12} + return out + + def _variant(sweep, tp="8"): """A minimal VariantConfig carrying just enough to exercise expected_cells. @@ -119,5 +133,67 @@ def test_no_cartesian_blowup(self): self.assertEqual(len(_variant(sw).expected_cells()), 2) +class TestGatedMetricCoverage(unittest.TestCase): + """The gated-metric axis of _check_thresholds_cover_sweep.""" + + _CELL = "ISL=128,OSL=2048,TP=8,CONC=16" + + def _variant_with(self, thresholds, enforce): + sw = Sweep( + sequence_combinations=[_combo("a")], + runs=[Run(combo="a", concurrency=16)], + ) + vc = _variant(sw) # remote guard ok; enforce/thresholds set below + # _variant builds enforce_thresholds=False with empty thresholds; rebuild + # via construction so the validator runs against the real values. + return VariantConfig( + schema_version=1, + framework="vllm_single", + gpu_arch="mi300x", + enforce_thresholds=enforce, + paths={ + "shared_fs": "/home/x", + "models_dir": "/home/x/models", + "log_dir": "/home/x/LOGS", + "hf_token_file": "/home/x/.hf", + }, + model={"id": "amd/Llama-3.1-70B-Instruct-FP8-KV", "remote": 0, "precision": "fp8"}, + image={"tag": "rocm/vllm-dev:nightly-sshd", "remote": 1}, + container={"name": "c", "image": "rocm/vllm-dev:nightly-sshd", "runtime": {"name": "docker"}}, + params={"tensor_parallelism": "8"}, + sweep=sw, + thresholds=thresholds, + ) + + def test_full_gated_set_constructs(self): + vc = self._variant_with({self._CELL: _full_gated_specs()}, enforce=True) + self.assertEqual(vc.enforce_thresholds, True) + + def test_missing_gated_metric_raises_when_enforced(self): + specs = _full_gated_specs() + del specs["client.p99_ttft_ms"] # drop one gated metric + with self.assertRaises(ValidationError) as ctx: + self._variant_with({self._CELL: specs}, enforce=True) + self.assertIn("missing gated-metric specs", str(ctx.exception)) + self.assertIn("client.p99_ttft_ms", str(ctx.exception)) + + def test_missing_gated_metric_warns_when_record_only(self): + specs = _full_gated_specs() + del specs["client.failed"] + import warnings as _w + with _w.catch_warnings(record=True) as caught: + _w.simplefilter("always") + self._variant_with({self._CELL: specs}, enforce=False) + self.assertTrue(any("missing gated-metric specs" in str(x.message) for x in caught)) + + def test_extra_non_gated_spec_is_allowed(self): + # A spec for a non-gated metric (record-only display extra) must not + # trip coverage -- gating is a floor, not an allow-list. + specs = _full_gated_specs() + specs["client.num_prompts"] = {"kind": "min", "value": 0} + vc = self._variant_with({self._CELL: specs}, enforce=True) + self.assertIn("client.num_prompts", vc.thresholds[self._CELL]) + + if __name__ == "__main__": unittest.main() diff --git a/cvs/lib/inference/unittests/test_vllm_orch_parse.py b/cvs/lib/inference/unittests/test_vllm_orch_parse.py index 5b71bd10..cc2c2394 100644 --- a/cvs/lib/inference/unittests/test_vllm_orch_parse.py +++ b/cvs/lib/inference/unittests/test_vllm_orch_parse.py @@ -406,6 +406,19 @@ def test_every_metric_key_is_produced(self): missing = [short for short, _u in self.vs._METRICS if ("client." + short) not in produced] self.assertEqual(missing, [], f"_METRICS names with no producer: {missing}") + def test_gated_metrics_are_all_produced_and_displayed(self): + """Every GATED_METRICS name must be both produced (parse_results emits + client.) and displayed (in _METRICS) -- a gated metric with no + producer would gate a '-', and one absent from _METRICS would assert a + value that never appears in the report.""" + from cvs.lib.inference.utils.vllm_parsing import GATED_METRICS + produced = set(self.actuals.keys()) + displayed = {short for short, _u in self.vs._METRICS} + no_producer = sorted(m for m in GATED_METRICS if ("client." + m) not in produced) + no_row = sorted(m for m in GATED_METRICS if m not in displayed) + self.assertEqual(no_producer, [], f"gated metrics with no producer: {no_producer}") + self.assertEqual(no_row, [], f"gated metrics not in _METRICS: {no_row}") + def test_skips_when_cell_absent(self): import pytest as _pt with self.assertRaises(_pt.skip.Exception): diff --git a/cvs/lib/inference/utils/inferencing_config_loader.py b/cvs/lib/inference/utils/inferencing_config_loader.py index f9eff039..d5bf6ac1 100644 --- a/cvs/lib/inference/utils/inferencing_config_loader.py +++ b/cvs/lib/inference/utils/inferencing_config_loader.py @@ -27,6 +27,7 @@ from typing_extensions import Literal from cvs.lib.utils.config_loader import BaseVariantConfig, _Forbid, substitute_config +from cvs.lib.inference.utils.vllm_parsing import GATED_METRICS # ---------- pydantic models (inference) ---------- @@ -163,9 +164,13 @@ def expected_cells(self): def _check_thresholds_cover_sweep(self): """Fail at load time if any sweep cell lacks a threshold entry. - Without this, a mistyped/whitespaced threshold key (or a new sweep - entry without a matching threshold) makes the test silently skip its - verdict and report a green PASS with zero assertions. + Two axes are checked. (1) Cell coverage: every sweep cell has a + threshold entry, and no threshold key names a cell the sweep does not + run. (2) Gated-metric coverage: every present cell carries a spec for + every GATED_METRICS member -- the SLO contract. Without (2) a gated + metric with no spec falls through `test_metric`'s `spec is None` + record-only branch and reports a green PASS with zero assertions, even + under enforce_thresholds=true. When `enforce_thresholds` is false the same mismatch is reported as a warning rather than an error -- the config loads as a record-only @@ -180,6 +185,17 @@ def _check_thresholds_cover_sweep(self): problems.append(f"sweep cells with no threshold entry: {missing}") if extra: problems.append(f"threshold keys matching no sweep cell (typo?): {extra}") + # Gated-metric coverage, only for cells that are present (a wholly + # missing cell is already reported above -- do not double-report it). + gated_keys = [f"client.{m}" for m in sorted(GATED_METRICS)] + gated_gaps = {} + for cell in sorted(expected & present): + specs = self.thresholds.get(cell) or {} + absent = [k for k in gated_keys if k not in specs] + if absent: + gated_gaps[cell] = absent + if gated_gaps: + problems.append(f"cells missing gated-metric specs: {gated_gaps}") if problems: msg = "threshold.json does not match the sweep matrix; " + "; ".join(problems) if self.enforce_thresholds: diff --git a/cvs/lib/inference/utils/vllm_parsing.py b/cvs/lib/inference/utils/vllm_parsing.py index 805a6520..37f7094b 100644 --- a/cvs/lib/inference/utils/vllm_parsing.py +++ b/cvs/lib/inference/utils/vllm_parsing.py @@ -123,3 +123,38 @@ def to_client_metrics(raw, *, tp, isl): ("p99_e2el_ms", "ms"), ] CLIENT_METRIC_UNITS = dict(CLIENT_METRICS) + +# The perf+health SLO contract: the subset of CLIENT_METRICS a calibrated run +# must *assert*, not merely display. The loader's coverage check requires a +# threshold spec for every name here in every present cell, so a gated metric +# can never silently fall through to a zero-assertion record-only row. +# +# Membership = "out of range means FAILURE", not "informational". So this is +# deliberately small: throughput (central + per-request), latency central +# (mean) AND tail (p99 -- mean can pass while the tail blows out), and run +# health (success_rate floor, failed ceiling). Inputs (num_prompts), totals +# (total_*_tokens), and derived diagnostics (normalized_ttft_ms_per_tok, +# decode_latency_ratio -- collinear with their raw metric within a cell) stay +# record-only by design. +# +# Closed-world default: a NEW metric added to CLIENT_METRICS is record-only +# until its name is added here. Add a metric to this set the moment it becomes +# a pass/fail criterion -- the loader then forces a spec for it in every cell +# before the suite can run green. Every name must also appear in CLIENT_METRICS +# (a gated metric with no producer would gate a '-'); a unit test pins that. +GATED_METRICS = { + # throughput + "total_token_throughput", + "output_throughput", + # latency -- central + "mean_ttft_ms", + "mean_tpot_ms", + # latency -- tail (independent signal; not collinear with the means) + "p99_ttft_ms", + "p99_tpot_ms", + "p99_itl_ms", + "p99_e2el_ms", + # run health + "success_rate", + "failed", +} From f3324904f1703d1ea90a4fbd96e9981d7939170a Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 15:36:01 -0400 Subject: [PATCH 23/46] fix(inference): single image source on container.image The image was declared twice: top-level image.tag (live -- conftest copied it onto the container block) and container.image (dead -- overwritten by that copy). The duplicate forced a top-level image block whose remote field was unused and whose tag silently shadowed container.image. Make container.image the single source: drop the top-level ImageSpec block from the generic BaseVariantConfig, drop the conftest overwrite so the merged container.image is used as-is, and remove the now-schemaless image block from the example config. --- .../w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json | 4 ---- .../inference/unittests/test_inferencing_config_loader.py | 2 -- cvs/lib/utils/config_loader.py | 8 ++------ cvs/tests/inference/vllm/conftest.py | 1 - 4 files changed, 2 insertions(+), 13 deletions(-) diff --git a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json index 11bd6d33..3ceeac2e 100644 --- a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json +++ b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_config.json @@ -14,10 +14,6 @@ "remote": 0, "precision": "fp8" }, - "image": { - "tag": "", - "remote": 1 - }, "container": { "lifetime": "per_run", "name": "w1_llama31_70b_fp8kv_perf_inference_rocm", diff --git a/cvs/lib/inference/unittests/test_inferencing_config_loader.py b/cvs/lib/inference/unittests/test_inferencing_config_loader.py index 9c8e22b8..8e64770c 100644 --- a/cvs/lib/inference/unittests/test_inferencing_config_loader.py +++ b/cvs/lib/inference/unittests/test_inferencing_config_loader.py @@ -58,7 +58,6 @@ def _variant(sweep, tp="8"): "hf_token_file": "/home/x/.hf", }, model={"id": "amd/Llama-3.1-70B-Instruct-FP8-KV", "remote": 0, "precision": "fp8"}, - image={"tag": "rocm/vllm-dev:nightly-sshd", "remote": 1}, container={ "name": "c", "image": "rocm/vllm-dev:nightly-sshd", @@ -158,7 +157,6 @@ def _variant_with(self, thresholds, enforce): "hf_token_file": "/home/x/.hf", }, model={"id": "amd/Llama-3.1-70B-Instruct-FP8-KV", "remote": 0, "precision": "fp8"}, - image={"tag": "rocm/vllm-dev:nightly-sshd", "remote": 1}, container={"name": "c", "image": "rocm/vllm-dev:nightly-sshd", "runtime": {"name": "docker"}}, params={"tensor_parallelism": "8"}, sweep=sw, diff --git a/cvs/lib/utils/config_loader.py b/cvs/lib/utils/config_loader.py index 3091d100..86ab589e 100644 --- a/cvs/lib/utils/config_loader.py +++ b/cvs/lib/utils/config_loader.py @@ -66,11 +66,6 @@ class ModelSpec(_Forbid): precision: str -class ImageSpec(_Forbid): - tag: str - remote: Literal[0, 1] - - class RuntimeSpec(_Allow): name: str args: Dict[str, Any] = Field(default_factory=dict) @@ -101,7 +96,8 @@ class BaseVariantConfig(_Forbid): enforce_thresholds: bool = True paths: Paths model: ModelSpec - image: ImageSpec + # The container image is declared once, on container.image (ContainerSpec). + # There is no separate top-level image block. container: ContainerSpec thresholds: Dict[str, Dict[str, Any]] = Field(default_factory=dict) diff --git a/cvs/tests/inference/vllm/conftest.py b/cvs/tests/inference/vllm/conftest.py index 75b790d3..01c6dd71 100644 --- a/cvs/tests/inference/vllm/conftest.py +++ b/cvs/tests/inference/vllm/conftest.py @@ -96,7 +96,6 @@ def orch(cluster_dict, variant_config, lifecycle): cluster_dict.get("container", {}), variant_config.container.model_dump(), ) - container_block["image"] = variant_config.image.tag testsuite_config = { "orchestrator": "container", "container": container_block, From a25687bb70aba0ea69d3564df0ae5d7e88f4f11d Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 16:50:05 -0400 Subject: [PATCH 24/46] feat(inference): gate the full latency distribution Expand GATED_METRICS from the mean+p99 subset to every emitted latency quantile (mean/median/p90/p95/p99) for ttft, tpot, itl, and e2el -- itl omits p90 as CLIENT_METRICS has no producer for it. Throughput and success_rate/failed health are unchanged. Inputs, totals, secondary throughputs, and derived diagnostics stay record-only. The example threshold file gains a placeholder spec for each newly gated metric (23 total) so the loader gated-coverage check passes. --- .../llama31_70b_fp8_threshold.json | 56 ++++++++++++++++++- cvs/lib/inference/utils/vllm_parsing.py | 33 +++++++---- 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json index 0bcb5b47..c5b9fb46 100644 --- a/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json +++ b/cvs/input/config_file/inference/vllm_single/w1_llama31_70b_fp8kv/llama31_70b_fp8_threshold.json @@ -1,5 +1,5 @@ { - "_comment": "PLACEHOLDER thresholds for W1 (Llama 3.1 70B FP8-KV, in=128/out=2048, TP=8, conc=16) -- the single cell the sweep's runs selector enumerates. Every gated metric (the SLO contract in vllm_parsing.GATED_METRICS: throughput, mean+p99 latency, success_rate/failed health) has a spec here so the loader's gated-coverage check passes; the values are NOT calibrated (AMD's published source gives throughput-vs-latency curves, not per-cell scalars). config.json sets enforce_thresholds=false: the suite records metrics and skips pass/fail. Replace the placeholder values with real numbers and flip enforce_thresholds=true to assert.", + "_comment": "PLACEHOLDER thresholds for W1 (Llama 3.1 70B FP8-KV, in=128/out=2048, TP=8, conc=16) -- the single cell the sweep enumerates. Every gated metric (vllm_parsing.GATED_METRICS: throughput, the full latency distribution mean/median/p90/p95/p99 per ttft/tpot/itl/e2el family, and success_rate/failed health) has a spec here so the loader gated-coverage check passes. Values are NOT calibrated (AMD publishes throughput-vs-latency curves, not per-cell scalars). config.json sets enforce_thresholds=false: the suite records metrics and skips pass/fail. Replace placeholders with real numbers and flip enforce_thresholds=true to assert.", "ISL=128,OSL=2048,TP=8,CONC=16": { "client.total_token_throughput": { "kind": "min_tok_s", @@ -13,7 +13,15 @@ "kind": "max_ms", "value": 1000000 }, - "client.mean_tpot_ms": { + "client.median_ttft_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.p90_ttft_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.p95_ttft_ms": { "kind": "max_ms", "value": 1000000 }, @@ -21,14 +29,58 @@ "kind": "max_ms", "value": 1000000 }, + "client.mean_tpot_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.median_tpot_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.p90_tpot_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.p95_tpot_ms": { + "kind": "max_ms", + "value": 1000000 + }, "client.p99_tpot_ms": { "kind": "max_ms", "value": 1000000 }, + "client.mean_itl_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.median_itl_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.p95_itl_ms": { + "kind": "max_ms", + "value": 1000000 + }, "client.p99_itl_ms": { "kind": "max_ms", "value": 1000000 }, + "client.mean_e2el_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.median_e2el_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.p90_e2el_ms": { + "kind": "max_ms", + "value": 1000000 + }, + "client.p95_e2el_ms": { + "kind": "max_ms", + "value": 1000000 + }, "client.p99_e2el_ms": { "kind": "max_ms", "value": 1000000 diff --git a/cvs/lib/inference/utils/vllm_parsing.py b/cvs/lib/inference/utils/vllm_parsing.py index 37f7094b..c53c1fba 100644 --- a/cvs/lib/inference/utils/vllm_parsing.py +++ b/cvs/lib/inference/utils/vllm_parsing.py @@ -129,13 +129,13 @@ def to_client_metrics(raw, *, tp, isl): # threshold spec for every name here in every present cell, so a gated metric # can never silently fall through to a zero-assertion record-only row. # -# Membership = "out of range means FAILURE", not "informational". So this is -# deliberately small: throughput (central + per-request), latency central -# (mean) AND tail (p99 -- mean can pass while the tail blows out), and run -# health (success_rate floor, failed ceiling). Inputs (num_prompts), totals -# (total_*_tokens), and derived diagnostics (normalized_ttft_ms_per_tok, -# decode_latency_ratio -- collinear with their raw metric within a cell) stay -# record-only by design. +# Membership = "out of range means FAILURE", not "informational". Gated: +# throughput (total + per-request output), the FULL latency distribution +# (mean/median/p90/p95/p99 for ttft, tpot, itl, e2el -- itl has no p90), and run +# health (success_rate floor, failed ceiling). Record-only by design: inputs +# (num_prompts), totals (total_*_tokens), secondary throughputs (per_gpu_*, +# decode_throughput_p50, max_output_tokens_per_s, request_throughput, goodput), +# and derived diagnostics (normalized_ttft_ms_per_tok, decode_latency_ratio). # # Closed-world default: a NEW metric added to CLIENT_METRICS is record-only # until its name is added here. Add a metric to this set the moment it becomes @@ -146,13 +146,26 @@ def to_client_metrics(raw, *, tp, isl): # throughput "total_token_throughput", "output_throughput", - # latency -- central + # latency -- full distribution per family (mean/median/p90/p95/p99; + # itl has no p90 producer). Every emitted quantile is a pass/fail gate. "mean_ttft_ms", - "mean_tpot_ms", - # latency -- tail (independent signal; not collinear with the means) + "median_ttft_ms", + "p90_ttft_ms", + "p95_ttft_ms", "p99_ttft_ms", + "mean_tpot_ms", + "median_tpot_ms", + "p90_tpot_ms", + "p95_tpot_ms", "p99_tpot_ms", + "mean_itl_ms", + "median_itl_ms", + "p95_itl_ms", "p99_itl_ms", + "mean_e2el_ms", + "median_e2el_ms", + "p90_e2el_ms", + "p95_e2el_ms", "p99_e2el_ms", # run health "success_rate", From c5201814b46fe9510bc99f18dea65a5d28f9c9c7 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 19 Jun 2026 17:02:57 -0400 Subject: [PATCH 25/46] docs: reflect image collapse, serve_args rename, max kind, GATED_METRICS - utils/AGENTS.md: drop top-level image (now container.image); add max verdict kind - inference/utils/AGENTS.md: document GATED_METRICS contract + dual-axis coverage check - building-a-cvs-test-suite.md: container.image, serve_args map, max kind, GATED_METRICS - dtni-dev-guide.md: SUPERSEDED banner pointing to building guide + AGENTS.md --- cvs/lib/inference/utils/AGENTS.md | 28 +++++++++++++++++++++------- cvs/lib/utils/AGENTS.md | 14 ++++++++------ plans/building-a-cvs-test-suite.md | 21 +++++++++++++++------ plans/dtni-dev-guide.md | 11 +++++++++++ 4 files changed, 55 insertions(+), 19 deletions(-) diff --git a/cvs/lib/inference/utils/AGENTS.md b/cvs/lib/inference/utils/AGENTS.md index 5770f546..3d0374bd 100644 --- a/cvs/lib/inference/utils/AGENTS.md +++ b/cvs/lib/inference/utils/AGENTS.md @@ -13,8 +13,8 @@ schema and the vLLM client-metric vocabulary. flags), the `server` role, and `VariantConfig(BaseVariantConfig)` with `cell_key`/`expected_cells` + the threshold-coverage check. - `vllm_parsing.py` — pure parsers for vLLM benchmark artifacts: - `to_client_metrics()`, the `client.*` namespace, and `CLIENT_METRICS` (the - display surface). + `to_client_metrics()`, the `client.*` namespace, `CLIENT_METRICS` (the + display surface), and `GATED_METRICS` (the asserted SLO subset). ## Public entry points @@ -29,6 +29,14 @@ schema and the vLLM client-metric vocabulary. The ordered `(short_name, unit)` list that becomes one HTML row per metric. The single definition every vLLM flavour (single-node, distributed, disagg, InferenceMax) shares — don't re-list rows per suite. +- `GATED_METRICS` + The asserted subset of `CLIENT_METRICS`: the perf+health SLO contract a + calibrated run must *assert*, not merely display. Membership = "out of range + means FAILURE". Today: throughput (total + per-request output), the full + latency distribution (mean/median/p90/p95/p99 per ttft/tpot/itl/e2el — itl has + no p90 producer), and run health (`success_rate` floor, `failed` ceiling). + Closed-world default: a new metric is record-only until its name is added + here, at which point the loader forces a spec for it in every cell. - `GoodputSlo`, `SeqCombo`, `Run`, `Sweep`, `Params`, `Roles`, `VariantConfig` — the typed schema. @@ -55,10 +63,14 @@ names and any `run.combo` that names no combo, **at load time**. lookup both call it, so they can't drift on whitespace/ordering. If you change the key format, everything keyed on it (threshold.json, `expected_cells`) moves together — change it in one place. -- **`_check_thresholds_cover_sweep` is what prevents a silent green.** Without it, - a mistyped threshold key makes the test skip its verdict and report PASS with - zero assertions. When `enforce_thresholds=false` the same mismatch is a - *warning* (record-only mode), not an error — that's intentional for +- **`_check_thresholds_cover_sweep` is what prevents a silent green.** It checks + two axes: (1) cell coverage — every sweep cell has a threshold entry, no key + names a non-existent cell; (2) gated-metric coverage — every present cell + carries a spec for every `GATED_METRICS` member. Without (1) a mistyped key + makes the test skip its verdict; without (2) a gated metric with no spec falls + through `test_metric`'s record-only branch and reports PASS with zero + assertions even under `enforce_thresholds=true`. When `enforce_thresholds=false` + both are *warnings* (record-only mode), not errors — intentional for un-calibrated configs, not a bug to "fix". - **`pytest_generate_tests` reads raw JSON and bypasses this loader** (it runs at collection time, before fixtures exist). So the validators here are @@ -89,7 +101,9 @@ names and any `run.combo` that names no combo, **at load time**. ## When extending - New `client.*` metric → add the derivation in `to_client_metrics` (guard - divisors with `_safe_div`) AND a `(name, unit)` entry in `CLIENT_METRICS`. + divisors with `_safe_div`) AND a `(name, unit)` entry in `CLIENT_METRICS`. If + it's a pass/fail criterion, also add it to `GATED_METRICS` (then every cell's + threshold.json needs a spec for it) — otherwise it stays record-only. - New combo field → add to `SeqCombo` (`_Forbid`, so also update any raw-JSON reader like `pytest_generate_tests`). - Second framework → subclass `Params`; reuse everything else here and diff --git a/cvs/lib/utils/AGENTS.md b/cvs/lib/utils/AGENTS.md index a1633ff8..8217128f 100644 --- a/cvs/lib/utils/AGENTS.md +++ b/cvs/lib/utils/AGENTS.md @@ -7,7 +7,7 @@ crept in, it belongs in `cvs/lib/inference/utils/` instead — keep this boundar ## What's here - `config_loader.py` — the generic half of variant-config loading: the - `paths`/`model`/`image`/`container` schema, `BaseVariantConfig`, the 3-pass + `paths`/`model`/`container` schema, `BaseVariantConfig`, the 3-pass placeholder substitution engine, and `substitute_config()`. - `verdict.py` — `evaluate_all(actuals, thresholds)`: a tiny, framework-neutral threshold checker. @@ -20,12 +20,14 @@ crept in, it belongs in `cvs/lib/inference/utils/` instead — keep this boundar **unvalidated** dict plus the parsed thresholds. A per-framework loader calls this, then builds its own typed `VariantConfig(**raw)`. - `BaseVariantConfig` — subclass it per framework. Carries the shared fields - (`schema_version`, `enforce_thresholds`, `paths`, `model`, `image`, - `container`, `thresholds`) and the `model.remote==1` NotImplementedError guard. + (`schema_version`, `enforce_thresholds`, `paths`, `model`, `container`, + `thresholds`) and the `model.remote==1` NotImplementedError guard. The image + is declared once on `container.image` — there is no separate top-level block. - `evaluate_all(actuals, thresholds)` — raises `ThresholdViolation` listing every - failing metric. Threshold kinds: `min`, `max_ms`, `within` (±tolerance_pct), - `min_tok_s`, `min_ratio` (compares against another metric named in - `reference`). + failing metric. Threshold kinds: `min`, `max` (unit-agnostic ceiling, for + counts like `failed`), `max_ms` (a ceiling that prints `ms`), `within` + (±tolerance_pct), `min_tok_s`, `min_ratio` (compares against another metric + named in `reference`). ## The seam (why this is split out) diff --git a/plans/building-a-cvs-test-suite.md b/plans/building-a-cvs-test-suite.md index 986726c2..4da00808 100644 --- a/plans/building-a-cvs-test-suite.md +++ b/plans/building-a-cvs-test-suite.md @@ -99,7 +99,8 @@ the split. A workload is described by a pair of JSON files in the same directory: -- `*_config.json` — the variant: paths, model, image, container, params, sweep. +- `*_config.json` — the variant: paths, model, container (with `container.image`), + params, sweep. - `*threshold.json` — the per-cell pass/fail thresholds. The loader finds the threshold by **sibling glob** (exactly one `*threshold.json` @@ -121,8 +122,9 @@ metric and skips assertions, and a threshold/sweep mismatch warns instead of failing the load. Use it for un-calibrated workloads (e.g. throughput characterization where the published numbers are curves, not tabulated cells). Flip to `true` once you have real numbers — the coverage check then guarantees -every sweep cell has a threshold entry, so a green run can't have silently -skipped its verdict. +both that every sweep cell has a threshold entry AND that every cell carries a +spec for every gated metric (`GATED_METRICS`), so a green run can't have +silently skipped its verdict. ### The sweep selector (named combos + runs) @@ -162,7 +164,8 @@ orchestrator (`orch.exec`, which routes into the running container) and a typed - **The server command is built in Python** (`_server_argv`), not cloned from an external `.sh` repo. A run is self-contained. Per-model quirks come from - `roles.server.extra_serve_args` / `roles.server.env` in config. + `roles.server.serve_args` (a `{flag: value}` map) / `roles.server.env` in + config. - **`/tmp/server_env_script.sh`** is written by `build_server_cmd` and **sourced by both server and client**, so the two share one environment (HF token, cache pin, AITER flags). Each value is `shlex.quote`d. @@ -217,8 +220,14 @@ Patterns to copy: `decode_throughput_p50`, `success_rate`, …) via `_safe_div` (degrades to `None`, never crashes). `CLIENT_METRICS` is the ordered display surface. - **`evaluate_all(actuals, thresholds)`** — generic, framework-neutral. Kinds: - `min`, `max_ms`, `within`, `min_tok_s`, `min_ratio`. Raises `ThresholdViolation` - listing every failure. A `None` actual is a loud violation, not a TypeError. + `min`, `max`, `max_ms`, `within`, `min_tok_s`, `min_ratio`. Raises + `ThresholdViolation` listing every failure. A `None` actual is a loud + violation, not a TypeError. +- **`GATED_METRICS`** (in `vllm_parsing`) — the asserted SLO subset of + `CLIENT_METRICS`. The loader's coverage check requires a threshold spec for + every gated metric in every present cell, so a gated metric can't silently + fall through to a zero-assertion record-only row. A new metric is record-only + until added to the set. ### To add your own suite — checklist diff --git a/plans/dtni-dev-guide.md b/plans/dtni-dev-guide.md index 88935bf7..5dd63d2f 100644 --- a/plans/dtni-dev-guide.md +++ b/plans/dtni-dev-guide.md @@ -1,5 +1,16 @@ # DTNI suite developer guide +> **⚠️ SUPERSEDED — design doc, not current state.** This guide predates the +> restructure that shipped. It still references the old `cvs/lib/dtni/` and +> `cvs/input/dtni/` paths (now `cvs/lib/utils/` + `cvs/lib/inference/utils/`), +> the `concurrency_levels × sequence_combinations` cartesian (now named combos + +> a `runs[]` selector), the top-level `image` block (now `container.image`), +> `OrchestratorConfig.from_dicts` / `container.enabled|launch`, and other +> pre-merge shapes. For the as-built architecture, read +> `plans/building-a-cvs-test-suite.md` and each package's `AGENTS.md`. Kept for +> the design rationale (the §5–§7 *why* behind the seam, the Job split, and the +> config/threshold split), which is still accurate. + ## 1. Intro and scope This guide is for CVS developers who already run `cvs run` regularly and have edited a test wrapper or a lib helper, but who haven't worked under the new DTNI (data-center training and inference) layout. The goal is to give you the mental model and the concrete skeleton needed to port an existing suite or author a new one. From b553fcca0a5914afc4555c0ed9c715825717f71e Mon Sep 17 00:00:00 2001 From: Hamna Nimra Date: Wed, 17 Jun 2026 21:57:51 -0700 Subject: [PATCH 26/46] Restore InferenceMax uplift reverted by #228 Reverts commit 4a8425f, restoring the changes from PR #225 on dev/dtni. --- cvs/cli_plugins/run_plugin.py | 8 +- cvs/cli_plugins/unittests/test_run_plugin.py | 29 + ...355x_inferencemax_gpt_oss_120b_single.json | 54 -- .../mi300x_gpt_oss_120b_single_config.json | 53 ++ .../mi300x_gpt_oss_120b_single_threshold.json | 10 + ...erencemax_gpt_oss_120b_single_config.json} | 28 +- ...encemax_gpt_oss_120b_single_threshold.json | 10 + cvs/lib/dtni/vllm_benchmark_scripts/README.md | 12 + .../dtni/vllm_benchmark_scripts/__init__.py | 54 ++ .../vllm_serve_mi300x.sh | 43 ++ cvs/lib/inference/base.py | 64 +- cvs/lib/inference/inference_max.py | 56 -- .../inference/inferencemax_host_scripts.py | 25 + cvs/lib/inference/inferencemax_orch.py | 570 ++++++++++++++++++ .../utils/inferencemax_config_loader.py | 93 +++ cvs/lib/inference_lib.py | 18 +- cvs/lib/sglang_disagg_lib.py | 2 +- cvs/tests/inference/inferencemax/_shared.py | 53 ++ cvs/tests/inference/inferencemax/conftest.py | 205 +++++++ .../inferencemax_gpt_oss_120b_single.py | 294 --------- .../inferencemax/inferencemax_single.py | 115 ++++ docs/how-to/run-cvs-tests.rst | 100 +-- docs/install/cvs-install.rst | 7 +- .../configuration-files/inferencemax.rst | 110 +++- 24 files changed, 1453 insertions(+), 560 deletions(-) delete mode 100644 cvs/input/config_file/inference/inferencemax/mi355x_inferencemax_gpt_oss_120b_single.json create mode 100644 cvs/input/config_file/inference/inferencemax_single/mi300x_gpt_oss_120b_single/mi300x_gpt_oss_120b_single_config.json create mode 100644 cvs/input/config_file/inference/inferencemax_single/mi300x_gpt_oss_120b_single/mi300x_gpt_oss_120b_single_threshold.json rename cvs/input/config_file/inference/{inferencemax/mi300x_inferencemax_gpt_oss_120b_single.json => inferencemax_single/mi355x_inferencemax_gpt_oss_120b_single/mi355x_inferencemax_gpt_oss_120b_single_config.json} (52%) create mode 100644 cvs/input/config_file/inference/inferencemax_single/mi355x_inferencemax_gpt_oss_120b_single/mi355x_inferencemax_gpt_oss_120b_single_threshold.json create mode 100644 cvs/lib/dtni/vllm_benchmark_scripts/README.md create mode 100644 cvs/lib/dtni/vllm_benchmark_scripts/__init__.py create mode 100644 cvs/lib/dtni/vllm_benchmark_scripts/vllm_serve_mi300x.sh delete mode 100644 cvs/lib/inference/inference_max.py create mode 100644 cvs/lib/inference/inferencemax_host_scripts.py create mode 100644 cvs/lib/inference/inferencemax_orch.py create mode 100644 cvs/lib/inference/utils/inferencemax_config_loader.py create mode 100644 cvs/tests/inference/inferencemax/_shared.py create mode 100644 cvs/tests/inference/inferencemax/conftest.py delete mode 100644 cvs/tests/inference/inferencemax/inferencemax_gpt_oss_120b_single.py create mode 100644 cvs/tests/inference/inferencemax/inferencemax_single.py diff --git a/cvs/cli_plugins/run_plugin.py b/cvs/cli_plugins/run_plugin.py index 72a8c8d1..ab9936b0 100644 --- a/cvs/cli_plugins/run_plugin.py +++ b/cvs/cli_plugins/run_plugin.py @@ -23,8 +23,12 @@ def get_parser(self, subparsers): ) parser.add_argument( "--log-file", - default="/tmp/cvs/test.log", - help="Pytest: Path to file for logging output (default: /tmp/cvs/test.log)", + default=None, + metavar="PATH", + help=( + "Pytest: write logging output to this file (optional). " + "Parent directories are created automatically when set." + ), ) parser.add_argument( "--log-level", diff --git a/cvs/cli_plugins/unittests/test_run_plugin.py b/cvs/cli_plugins/unittests/test_run_plugin.py index ef73ad42..57fbfa1a 100644 --- a/cvs/cli_plugins/unittests/test_run_plugin.py +++ b/cvs/cli_plugins/unittests/test_run_plugin.py @@ -79,6 +79,35 @@ def test_run_test_multiple_functions(self, mock_exit, mock_pytest_main): mock_pytest_main.assert_called_once_with(expected_args) mock_exit.assert_called_once_with(0) + @patch("cvs.cli_plugins.run_plugin.pytest.main") + @patch("cvs.cli_plugins.run_plugin.sys.exit") + def test_run_test_omits_log_file_when_not_set(self, mock_exit, mock_pytest_main): + """No --log-file is passed to pytest when the user does not request file logging.""" + args = MagicMock() + args.test = "agfhc_cvs" + args.function = [] + args.cluster_file = "/path/to/cluster.json" + args.config_file = "/path/to/config.json" + args.html = None + args.self_contained_html = False + args.log_file = None + args.log_level = None + args.capture = None + args.extra_pytest_args = [] + + mock_pytest_main.return_value = 0 + + with patch.object(self.plugin, "get_test_file", return_value="/mock/path/test.py"): + self.plugin.run(args) + + expected_args = [ + "/mock/path/test.py", + "--cluster_file=/path/to/cluster.json", + "--config_file=/path/to/config.json", + ] + mock_pytest_main.assert_called_once_with(expected_args) + mock_exit.assert_called_once_with(0) + if __name__ == "__main__": unittest.main() diff --git a/cvs/input/config_file/inference/inferencemax/mi355x_inferencemax_gpt_oss_120b_single.json b/cvs/input/config_file/inference/inferencemax/mi355x_inferencemax_gpt_oss_120b_single.json deleted file mode 100644 index 516efca2..00000000 --- a/cvs/input/config_file/inference/inferencemax/mi355x_inferencemax_gpt_oss_120b_single.json +++ /dev/null @@ -1,54 +0,0 @@ -{ - "config": { - "container_image": "rocm/7.0:rocm7.0_ubuntu_22.04_vllm_0.10.1_instinct_20250927_rc1", - "container_name": "inference_max_rocm", - "_example_nnodes": "4", - "nnodes": "4", - "inferencemax_repo": "https://github.com/SemiAnalysisAI/InferenceX.git", - "benchmark_script_repo": "https://github.com/kimbochen/bench_serving.git", - "hf_token_file": "/home/{user-id}/.hf_token", - "shm_size": "128G", - "log_dir": "/home/{user-id}/LOGS", - "container_config": { - "device_list": [ - "/dev/dri", - "/dev/kfd" - ], - "volume_dict": { - "/home/{user-id}": "/home/{user-id}" - }, - "env_dict": {} - } - }, - "benchmark_params": { - "gpt-oss-120b": { - "backend": "vllm", - "base_url": "http://0.0.0.0", - "port_no": "8000", - "_example_dataset_name": "sharegpt|hf|random|sonnet|burstgpt", - "dataset_name": "random", - "max_concurrency": "64", - "model": "openai/gpt-oss-120b", - "num_prompts": "1000", - "input_sequence_length": "8192", - "output_sequence_length": "1024", - "burstiness": "1.0", - "seed": "0", - "max_model_length": "9216", - "random_range_ratio": "0.8", - "random_prefix_len": "0", - "tensor_parallelism": "8", - "_example_tokenizer_mode": "auto|slow|mistral|custom", - "tokenizer_mode": "auto", - "percentiles_metrics": "ttft,tpot,itl,e2el", - "metric_percentiles": "99", - "server_script": "gptoss_fp4_mi355x.sh", - "bench_serv_script": "benchmark_serving.py", - "result_dict": { - "output_throughput_per_sec": "4200", - "mean_ttft_ms": "500", - "mean_tpot_ms": "15" - } - } - } -} \ No newline at end of file diff --git a/cvs/input/config_file/inference/inferencemax_single/mi300x_gpt_oss_120b_single/mi300x_gpt_oss_120b_single_config.json b/cvs/input/config_file/inference/inferencemax_single/mi300x_gpt_oss_120b_single/mi300x_gpt_oss_120b_single_config.json new file mode 100644 index 00000000..35522962 --- /dev/null +++ b/cvs/input/config_file/inference/inferencemax_single/mi300x_gpt_oss_120b_single/mi300x_gpt_oss_120b_single_config.json @@ -0,0 +1,53 @@ +{ + "_comment": "Single-node InferenceMax. Set container_image and container_name to your values (use in committed samples). ISL+OSL must fit max_model_length. CVS expects percentile_metrics (not percentiles_metrics). Shared server scripts: cvs.lib.dtni.vllm_benchmark_scripts (host_benchmark_scripts_relpath). benchmark_server_script_path auto stages from checkout or that package. volume_dict: map only home→home; ContainerOrchestrator adds /home/:/workspace so do not add a second :/workspace bind (Docker duplicate mount). InferenceX files under /workspace appear on the host under your home (e.g. ~/server.log).", + "config": { + "container_image": "", + "container_name": "", + "use_host_mounted_server_script": true, + "benchmark_server_script_path": "auto", + "host_benchmark_scripts_relpath": "lib/dtni/vllm_benchmark_scripts", + "vllm_enforce_eager": true, + "hf_token_file": "/home/{user-id}/.hf_token", + "shm_size": "128G", + "log_dir": "/home/{user-id}/LOGS", + "container_config": { + "device_list": [ + "/dev/dri", + "/dev/kfd" + ], + "volume_dict": { + "/home/{user-id}": "/home/{user-id}" + }, + "env_dict": { + "AMDGCN_USE_BUFFER_OPS": "0", + "VLLM_ROCM_USE_AITER": "1", + "VLLM_ROCM_QUICK_REDUCE_QUANTIZATION": "INT4", + "VLLM_GPU_MEMORY_UTIL": "0.95" + } + } + }, + "benchmark_params": { + "gpt-oss-120b": { + "backend": "vllm", + "base_url": "http://0.0.0.0", + "port_no": "8000", + "dataset_name": "random", + "max_concurrency": "64", + "model": "openai/gpt-oss-120b", + "num_prompts": "1000", + "input_sequence_length": "7168", + "output_sequence_length": "1024", + "burstiness": "1.0", + "seed": "0", + "max_model_length": "8192", + "random_range_ratio": "0.8", + "random_prefix_len": "0", + "tensor_parallelism": "8", + "tokenizer_mode": "auto", + "percentile_metrics": "ttft,tpot,itl,e2el", + "metric_percentiles": "99", + "server_script": "fixed_seq_len/vllm_serve_mi300x.sh", + "bench_serv_script": "benchmark_serving.py" + } + } +} diff --git a/cvs/input/config_file/inference/inferencemax_single/mi300x_gpt_oss_120b_single/mi300x_gpt_oss_120b_single_threshold.json b/cvs/input/config_file/inference/inferencemax_single/mi300x_gpt_oss_120b_single/mi300x_gpt_oss_120b_single_threshold.json new file mode 100644 index 00000000..d4304c85 --- /dev/null +++ b/cvs/input/config_file/inference/inferencemax_single/mi300x_gpt_oss_120b_single/mi300x_gpt_oss_120b_single_threshold.json @@ -0,0 +1,10 @@ +{ + "_comment": "Pass/fail expectations for verify_inference_results (keyed by ISL, OSL, TP, CONC). Must match benchmark_params for the cell you run. Tune values for your hardware.", + "result_dict": { + "ISL=7168,OSL=1024,TP=8,CONC=64": { + "output_throughput_per_sec": "4200", + "mean_ttft_ms": "500", + "mean_tpot_ms": "15" + } + } +} diff --git a/cvs/input/config_file/inference/inferencemax/mi300x_inferencemax_gpt_oss_120b_single.json b/cvs/input/config_file/inference/inferencemax_single/mi355x_inferencemax_gpt_oss_120b_single/mi355x_inferencemax_gpt_oss_120b_single_config.json similarity index 52% rename from cvs/input/config_file/inference/inferencemax/mi300x_inferencemax_gpt_oss_120b_single.json rename to cvs/input/config_file/inference/inferencemax_single/mi355x_inferencemax_gpt_oss_120b_single/mi355x_inferencemax_gpt_oss_120b_single_config.json index 18a4d5a8..faf14939 100644 --- a/cvs/input/config_file/inference/inferencemax/mi300x_inferencemax_gpt_oss_120b_single.json +++ b/cvs/input/config_file/inference/inferencemax_single/mi355x_inferencemax_gpt_oss_120b_single/mi355x_inferencemax_gpt_oss_120b_single_config.json @@ -1,11 +1,8 @@ { + "_comment": "Single-node InferenceMax sample. Set container_image and container_name (e.g. replace ). ISL+OSL within max_model_length; CVS uses percentile_metrics.", "config": { - "container_image": "rocm/7.0:rocm7.0_ubuntu_22.04_vllm_0.10.1_instinct_20250927_rc1", - "container_name": "inference_max_rocm", - "_example_nnodes": "4", - "nnodes": "4", - "inferencemax_repo": "https://github.com/SemiAnalysisAI/InferenceX.git", - "benchmark_script_repo": "https://github.com/kimbochen/bench_serving.git", + "container_image": "", + "container_name": "", "hf_token_file": "/home/{user-id}/.hf_token", "shm_size": "128G", "log_dir": "/home/{user-id}/LOGS", @@ -25,30 +22,23 @@ "backend": "vllm", "base_url": "http://0.0.0.0", "port_no": "8000", - "_example_dataset_name": "sharegpt|hf|random|sonnet|burstgpt", "dataset_name": "random", "max_concurrency": "64", "model": "openai/gpt-oss-120b", "num_prompts": "1000", - "input_sequence_length": "8192", + "input_sequence_length": "7168", "output_sequence_length": "1024", "burstiness": "1.0", "seed": "0", - "max_model_length": "9216", + "max_model_length": "8192", "random_range_ratio": "0.8", "random_prefix_len": "0", "tensor_parallelism": "8", - "_example_tokenizer_mode": "auto|slow|mistral|custom", "tokenizer_mode": "auto", - "percentiles_metrics": "ttft,tpot,itl,e2el", + "percentile_metrics": "ttft,tpot,itl,e2el", "metric_percentiles": "99", - "server_script": "gptoss_fp4_mi300x.sh", - "bench_serv_script": "benchmark_serving.py", - "result_dict": { - "output_throughput_per_sec": "4200", - "mean_ttft_ms": "500", - "mean_tpot_ms": "15" - } + "server_script": "fixed_seq_len/vllm_serve_mi300x.sh", + "bench_serv_script": "benchmark_serving.py" } } -} \ No newline at end of file +} diff --git a/cvs/input/config_file/inference/inferencemax_single/mi355x_inferencemax_gpt_oss_120b_single/mi355x_inferencemax_gpt_oss_120b_single_threshold.json b/cvs/input/config_file/inference/inferencemax_single/mi355x_inferencemax_gpt_oss_120b_single/mi355x_inferencemax_gpt_oss_120b_single_threshold.json new file mode 100644 index 00000000..d4304c85 --- /dev/null +++ b/cvs/input/config_file/inference/inferencemax_single/mi355x_inferencemax_gpt_oss_120b_single/mi355x_inferencemax_gpt_oss_120b_single_threshold.json @@ -0,0 +1,10 @@ +{ + "_comment": "Pass/fail expectations for verify_inference_results (keyed by ISL, OSL, TP, CONC). Must match benchmark_params for the cell you run. Tune values for your hardware.", + "result_dict": { + "ISL=7168,OSL=1024,TP=8,CONC=64": { + "output_throughput_per_sec": "4200", + "mean_ttft_ms": "500", + "mean_tpot_ms": "15" + } + } +} diff --git a/cvs/lib/dtni/vllm_benchmark_scripts/README.md b/cvs/lib/dtni/vllm_benchmark_scripts/README.md new file mode 100644 index 00000000..4299d0d4 --- /dev/null +++ b/cvs/lib/dtni/vllm_benchmark_scripts/README.md @@ -0,0 +1,12 @@ +# vLLM benchmark server scripts (shared) + +Shell entrypoints for **`vllm serve`** used by CVS **vllm_single** (`VllmJob` in `cvs.lib.inference.vllm_orch`) and **InferenceMax** host-mounted server flows. + +- **`vllm_serve_mi300x.sh`** — default MI300-class server wrapper; the served checkpoint is whatever you set in `MODEL` (the filename is not model-specific). + +- Point **vLLM** `paths.benchmark_scripts_dir` (host path, bind-mounted into the container) at a directory that contains copies of—or symlinks to—these files, **or** mount this package directory. +- Point **InferenceMax** `host_benchmark_scripts_relpath` at `lib/dtni/vllm_benchmark_scripts` (relative to the `cvs` Python package root) unless you override `benchmark_server_script_path`. + +**Client benchmarks** use the Python file shipped with the installed **vLLM** package under `vllm/benchmarks/` (resolved at runtime inside the container). CVS no longer clones a third-party `bench_serving` git repo. + +Python API: `bundled_scripts_dir()`, `bash_export_bench_script_from_vllm_install()`, `validated_bench_script_basename()`. diff --git a/cvs/lib/dtni/vllm_benchmark_scripts/__init__.py b/cvs/lib/dtni/vllm_benchmark_scripts/__init__.py new file mode 100644 index 00000000..39327213 --- /dev/null +++ b/cvs/lib/dtni/vllm_benchmark_scripts/__init__.py @@ -0,0 +1,54 @@ +''' +Copyright 2025 Advanced Micro Devices, Inc. +All rights reserved. + +Canonical **vLLM benchmark server** shell helpers used by: + +* :mod:`cvs.lib.inference.vllm_orch` (``VllmJob`` — ``variant.paths.benchmark_scripts_dir`` should + include these files on the host path that is bind-mounted into the container), and +* :mod:`cvs.lib.inference.inferencemax_orch` (host-mounted / staged server scripts when + ``host_benchmark_scripts_relpath`` defaults to ``lib/dtni/vllm_benchmark_scripts``). + +Keeping scripts here avoids drift between InferenceMax wrappers and vLLM single orchestration. + +Client load generation uses the **vLLM install’s** ``benchmarks/