From c7efa8d62025a2cc2ca3ee22e389b363578b6e61 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Thu, 29 Oct 2026 18:04:34 -0700 Subject: [PATCH 1/3] feat(vllm): build the vLLM config schema from the shared blocks Move the vLLM schema into vllm_schema.py, composed from config_blocks, and load it through load_config. vllm_config_loader.py keeps load_variant and re-exports VariantConfig and serialize_cli_options. A config that still has a or value now fails at test collection, and the error names every such field, instead of failing later at image pull or docker run. Keys starting with _ are ignored at any level, including threshold metrics. runtime.args accepts the other Docker runtime keys and rejects env, so container.env stays the only env block. The suite's conftest takes the merged container from load_config and no longer keeps its own deep merge. The vLLM config tests move beside the schema. The packaged-catalog test is replaced by loading each shipped template with its placeholders filled. --- CHANGELOG.md | 2 +- .../unittests/test_vllm_config_loader.py | 176 -------- .../test_vllm_config_loader_accuracy.py | 103 ----- .../unittests/test_vllm_deck_profile.py | 11 +- .../unittests/test_vllm_config_loader.py | 109 +++++ .../unittests/test_vllm_schema.py} | 149 ++++--- cvs/lib/inference/utils/vllm_config_loader.py | 393 +----------------- cvs/lib/inference/utils/vllm_schema.py | 380 +++++++++++++++++ cvs/tests/inference/vllm/conftest.py | 55 +-- cvs/unittests/support.py | 30 ++ docs/how-to/test-suites/inference/vllm.rst | 8 +- .../configuration-files/inference/vllm.rst | 42 +- 12 files changed, 663 insertions(+), 795 deletions(-) delete mode 100644 cvs/lib/inference/unittests/test_vllm_config_loader.py delete mode 100644 cvs/lib/inference/unittests/test_vllm_config_loader_accuracy.py create mode 100644 cvs/lib/inference/utils/unittests/test_vllm_config_loader.py rename cvs/lib/inference/{unittests/test_vllm_config_contract.py => utils/unittests/test_vllm_schema.py} (58%) create mode 100644 cvs/lib/inference/utils/vllm_schema.py diff --git a/CHANGELOG.md b/CHANGELOG.md index c8eab9138..fa896024d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,7 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- vLLM configs that set `HF_TOKEN` or `HUGGING_FACE_HUB_TOKEN` in `container.env` now fail to load; set `paths.hf_token_file` instead, on a path mounted in the container on each node. +- vLLM configs that set `HF_TOKEN` or `HUGGING_FACE_HUB_TOKEN` in `container.env` now fail to load; set `paths.hf_token_file` instead, on a path mounted in the container on each node. A vLLM config that still has a `` or `` value fails at test collection, and the error names every such field. Keys starting with `_` are ignored at any level, including threshold metrics. `container.runtime.args` accepts the other Docker runtime keys, such as `shm_size` and `cap_add`, and rejects `env`. - ATOM reads the Hugging Face token from `paths.hf_token_file` and mounts that file read-only in the container. The file must exist and be non-empty on every node. If that mount is missing inside the container, the server env script fails instead of starting the server without a token. `HF_TOKEN`, `HUGGING_FACE_HUB_TOKEN`, and `HUGGINGFACE_HUB_TOKEN` in the container environment are dropped. - ibperf: - A node with no partner in the discovered NIC set fails and is excluded, instead of running as a server with no client. diff --git a/cvs/lib/inference/unittests/test_vllm_config_loader.py b/cvs/lib/inference/unittests/test_vllm_config_loader.py deleted file mode 100644 index d1b83c031..000000000 --- a/cvs/lib/inference/unittests/test_vllm_config_loader.py +++ /dev/null @@ -1,176 +0,0 @@ -'''Unit tests for strict vLLM threshold schema validation.''' - -import math -import unittest - -from pydantic import ValidationError - -from cvs.lib.inference.utils.vllm_config_loader import ( - GATED_GPU_METRICS, - GATED_PROM_METRICS, - VariantConfig, -) - - -CELL = "ISL=128,OSL=2048,TP=8,PP=1,CONC=16" -FAKE_CRED_FILE = "/home/x/.hf" - - -def _variant(thresholds, enforce=False): - return VariantConfig( - enforce_thresholds=enforce, - threshold_json="threshold.json", - paths={ - "shared_fs": "/home/x", - "models_dir": "/home/x/models", - "log_dir": "/home/x/LOGS", - "hf_token_file": FAKE_CRED_FILE, - }, - container={"name": "test", "image": "test", "runtime": {"name": "docker", "args": {}}}, - server_params={"model": "amd/model", "tensor_parallel_size": 8}, - sweeps={CELL: {}}, - runs=[CELL], - thresholds=thresholds, - ) - - -class TestPartialThresholdCells(unittest.TestCase): - def test_one_spec_cell_loads_with_enforcement_off_and_on(self): - thresholds = {CELL: {"output_throughput": {"kind": "min", "value": 0}}} - for enforce in (False, True): - with self.subTest(enforce=enforce): - variant = _variant(thresholds, enforce=enforce) - self.assertEqual(variant.thresholds, thresholds) - - def test_empty_cell_loads_with_enforcement_off_and_on(self): - for enforce in (False, True): - with self.subTest(enforce=enforce): - variant = _variant({CELL: {}}, enforce=enforce) - self.assertEqual(variant.thresholds[CELL], {}) - - def test_cell_metadata_loads_with_enforcement_off_and_on(self): - thresholds = { - CELL: { - "_comment": "calibrate before enforcing", - "_example": {"output_throughput": {"kind": "min", "value": 1}}, - "output_throughput": {"kind": "min", "value": 0}, - } - } - for enforce in (False, True): - with self.subTest(enforce=enforce): - variant = _variant(thresholds, enforce=enforce) - self.assertEqual(variant.thresholds, thresholds) - - def test_non_metadata_unknown_key_is_rejected(self): - for enforce in (False, True): - for metric in ("notes", "_typo", "_metadata", "_"): - with self.subTest(enforce=enforce, metric=metric): - with self.assertRaisesRegex( - ValidationError, - f"unknown vLLM threshold metric {metric!r}", - ): - _variant( - {CELL: {metric: {"kind": "min", "value": 0}}}, - enforce=enforce, - ) - - def test_enforcement_still_requires_selected_run_cell(self): - _variant({}, enforce=False) - with self.assertRaisesRegex(ValidationError, "missing threshold coverage"): - _variant({}, enforce=True) - - def test_accuracy_is_exempt_from_vllm_metric_validation(self): - accuracy = { - "gsm8k": { - "gsm8k.exact_match__strict-match": { - "kind": "min", - "value": 0, - "tolerance_pct": 5, - } - } - } - variant = _variant({CELL: {}, "accuracy": accuracy}, enforce=True) - self.assertEqual(variant.thresholds["accuracy"], accuracy) - - -class TestStrictThresholdSpecs(unittest.TestCase): - def assertRejected(self, metric, spec): - with self.assertRaises(ValidationError): - _variant({CELL: {metric: spec}}) - - def test_rejects_nonfinite_or_non_builtin_values(self): - for value in (True, "1", None, [], {}, math.nan, math.inf, -math.inf): - with self.subTest(value=value): - self.assertRejected( - "output_throughput", - {"kind": "min", "value": value}, - ) - - def test_rejects_missing_unknown_and_legacy_kinds(self): - for spec in ( - {"value": 1}, - {"kind": "min"}, - {"kind": "info", "value": 1}, - {"kind": "min_tok_s", "value": 1}, - {"kind": "max_ms", "value": 1}, - {"kind": "within", "value": 1}, - {"kind": "min_ratio", "value": 1}, - {"kind": "other", "value": 1}, - ): - with self.subTest(spec=spec): - self.assertRejected("output_throughput", spec) - - def test_rejects_direction_mismatch(self): - self.assertRejected("output_throughput", {"kind": "max", "value": 1}) - self.assertRejected("mean_ttft_ms", {"kind": "min", "value": 1}) - - def test_rejects_prefixed_and_unknown_metric_names(self): - for metric in ("client.output_throughput", "gpu.gpu_compute_util_pct", "prom.queue_time_p50_ms", "new_metric"): - with self.subTest(metric=metric): - self.assertRejected(metric, {"kind": "min", "value": 1}) - - def test_rejects_extra_spec_fields(self): - for field, value in ( - ("unit", "tok/s"), - ("reference", "total_token_throughput"), - ("tolerance_pct", 5), - ("extra", 1), - ): - with self.subTest(field=field): - self.assertRejected( - "output_throughput", - {"kind": "min", "value": 1, field: value}, - ) - - def test_accepts_finite_int_float_zero_and_negative(self): - for value in (0, -1, 1.5): - with self.subTest(value=value): - variant = _variant({CELL: {"output_throughput": {"kind": "min", "value": value}}}) - self.assertEqual(variant.thresholds[CELL]["output_throughput"]["value"], value) - - -class TestDatasourceViews(unittest.TestCase): - def test_gpu_and_prometheus_names_are_bare_and_complete(self): - self.assertEqual( - GATED_GPU_METRICS, - { - "peak_gpu_memory_mb", - "model_load_memory_mb", - "model_load_s", - "gpu_bandwidth_util_pct", - "gpu_compute_util_pct", - }, - ) - self.assertEqual( - GATED_PROM_METRICS, - { - "queue_time_p50_ms", - "queue_time_p95_ms", - "prefill_time_p50_ms", - "prefill_time_p95_ms", - }, - ) - - -if __name__ == "__main__": - unittest.main() diff --git a/cvs/lib/inference/unittests/test_vllm_config_loader_accuracy.py b/cvs/lib/inference/unittests/test_vllm_config_loader_accuracy.py deleted file mode 100644 index dd2f94cea..000000000 --- a/cvs/lib/inference/unittests/test_vllm_config_loader_accuracy.py +++ /dev/null @@ -1,103 +0,0 @@ -''' -Copyright 2025 Advanced Micro Devices, Inc. -All rights reserved. - -Unit tests for the `accuracy` field wired onto vllm's VariantConfig (step 5 of -the accuracy-harness plan). AccuracyConfig itself is fully covered by -test_accuracy_config.py; these tests only pin the wiring: default/opt-in -behavior and pass-through of an explicit accuracy block. -''' - -import unittest - -from cvs.lib.inference.utils.accuracy_config import AccuracyConfig -from cvs.lib.inference.utils.vllm_config_loader import VariantConfig - -FAKE_CRED_FILE = "/home/x/.hf" - - -def _base_kwargs(**overrides): - cell = "ISL=1024,OSL=1024,TP=8,PP=1,CONC=16" - kwargs = dict( - enforce_thresholds=False, - threshold_json="threshold.json", - paths={ - "shared_fs": "/home/x", - "models_dir": "/home/x/models", - "log_dir": "/home/x/LOGS", - "hf_token_file": FAKE_CRED_FILE, - }, - container={"name": "test", "image": "test", "runtime": {"name": "docker", "args": {}}}, - server_params={"model": "/models/test-model", "tensor_parallel_size": 8}, - sweeps={cell: {}}, - runs=[cell], - thresholds={}, - ) - kwargs.update(overrides) - return kwargs - - -class TestVariantConfigAccuracyField(unittest.TestCase): - def test_defaults_to_empty_accuracy_config_when_omitted(self): - vc = VariantConfig(**_base_kwargs()) - self.assertIsInstance(vc.accuracy, AccuracyConfig) - self.assertEqual(vc.accuracy.tasks, []) - - def test_accepts_explicit_accuracy_tasks(self): - vc = VariantConfig(**_base_kwargs(accuracy={"tasks": [{"id": "mmlu", "task": "mmlu", "num_fewshot": 5}]})) - self.assertEqual(len(vc.accuracy.tasks), 1) - self.assertEqual(vc.accuracy.tasks[0].id, "mmlu") - self.assertEqual(vc.accuracy.tasks[0].num_fewshot, 5) - - def test_duplicate_task_ids_rejected_through_variant_config(self): - with self.assertRaises(ValueError): - VariantConfig( - **_base_kwargs( - accuracy={ - "tasks": [ - {"id": "mmlu", "task": "mmlu"}, - {"id": "mmlu", "task": "mmlu"}, - ] - } - ) - ) - - -class TestAccuracyThresholdKeyDoesNotTripSweepCoverage(unittest.TestCase): - """The top-level "accuracy" threshold key must not be flagged as an - unrecognized sweep-cell key by _check_thresholds_cover_sweep, now that it - delegates to the shared validate_thresholds_cover_sweep.""" - - _CELL = "ISL=1024,OSL=1024,TP=8,PP=1,CONC=16" - - def _full_gated_specs(self): - return {"output_throughput": {"kind": "min", "value": 0}} - - def test_accuracy_key_alongside_full_sweep_coverage_constructs(self): - vc = VariantConfig( - **_base_kwargs( - enforce_thresholds=True, - thresholds={ - self._CELL: self._full_gated_specs(), - "accuracy": {"mmlu": {"mmlu.acc__none": {"kind": "min", "value": 0.5}}}, - }, - ) - ) - self.assertIn("accuracy", vc.thresholds) - - def test_typo_key_is_rejected_at_config_load(self): - with self.assertRaisesRegex(ValueError, "threshold cells"): - VariantConfig( - **_base_kwargs( - enforce_thresholds=True, - thresholds={ - self._CELL: self._full_gated_specs(), - "accuracy": {"mmlu": {"mmlu.acc__none": {"kind": "min", "value": 0.5}}}, - "acuracy": {}, - }, - ) - ) - - -if __name__ == "__main__": - unittest.main() diff --git a/cvs/lib/inference/unittests/test_vllm_deck_profile.py b/cvs/lib/inference/unittests/test_vllm_deck_profile.py index 978587f47..746a695a5 100644 --- a/cvs/lib/inference/unittests/test_vllm_deck_profile.py +++ b/cvs/lib/inference/unittests/test_vllm_deck_profile.py @@ -3,6 +3,7 @@ All rights reserved. ''' +import tempfile import unittest from pathlib import Path @@ -20,6 +21,7 @@ from cvs.lib.report.registry import get_resolved_profile, resolve_suite_report_config from cvs.lib.report.rundeck.config_adapter import build_inference_config_from_profile from cvs.lib.report.inference import build_inference_report_payload +from cvs.unittests.support import write_filled_config class TestVllmDeckProfile(unittest.TestCase): @@ -121,10 +123,11 @@ def test_split_suite_stems_reuse_vllm_profile(self): def test_distributed_cell_uses_bound_threshold_key(self): root = Path(__file__).resolve().parents[3] - variant = load_variant( - root / "input/config_file/inference/vllm/mi3xx_vllm_llama33-70b_fp8_distributed.json", - {"username": "test"}, - ) + with tempfile.TemporaryDirectory() as tmp: + config = write_filled_config( + root / "input/config_file/inference/vllm/mi3xx_vllm_llama33-70b_fp8_distributed.json", tmp + ) + variant = load_variant(config, {"username": "test"}) variant.bind_effective_topology(EffectiveVllmTopology("distributed", ("node0", "node1"), 2)) cell_id = variant.expected_cells()[0] variant.enforce_thresholds = True diff --git a/cvs/lib/inference/utils/unittests/test_vllm_config_loader.py b/cvs/lib/inference/utils/unittests/test_vllm_config_loader.py new file mode 100644 index 000000000..7767df052 --- /dev/null +++ b/cvs/lib/inference/utils/unittests/test_vllm_config_loader.py @@ -0,0 +1,109 @@ +'''Unit tests for loading shipped and user vLLM configs through load_variant.''' + +import json +import tempfile +import unittest +from pathlib import Path + +from cvs.lib.inference.utils.vllm_config_loader import VariantConfig, load_variant +from cvs.lib.utils.config_loader import ConfigError, load_config +from cvs.unittests.support import write_filled_config + +CONFIG_DIR = Path(__file__).resolve().parents[4] / "input" / "config_file" / "inference" / "vllm" +DISTRIBUTED_TEMPLATE = CONFIG_DIR / "mi3xx_vllm_llama33-70b_fp8_distributed.json" +CELL = "ISL=1024,OSL=1024,TP=8,PP=1,CONC=16" + + +def _templates(): + return sorted(path for path in CONFIG_DIR.glob("*.json") if not path.name.endswith("threshold.json")) + + +class TestShippedTemplates(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.tmp = Path(self._tmp.name) + + def test_every_template_loads_once_its_placeholders_are_filled(self): + templates = _templates() + self.assertTrue(templates) + for template in templates: + with self.subTest(config=template.name): + load_variant(write_filled_config(template, self.tmp), {"username": "test"}) + + def test_unfilled_template_lists_every_placeholder_in_one_error(self): + with self.assertRaises(ConfigError) as ctx: + load_variant(DISTRIBUTED_TEMPLATE, {"username": "test"}) + self.assertEqual( + sorted(problem.split(":", 1)[0] for problem in ctx.exception.problems), + [ + "container.env.GLOO_SOCKET_IFNAME", + "container.env.NCCL_IB_GID_INDEX", + "container.env.NCCL_SOCKET_IFNAME", + "container.env.TP_SOCKET_IFNAME", + "container.image", + "container.runtime.args.volumes.1", + "container.runtime.args.volumes.3", + "server_params.model", + ], + ) + + def test_placeholder_in_an_option_value_fails_the_load(self): + filled = write_filled_config(DISTRIBUTED_TEMPLATE, self.tmp) + config = json.loads(filled.read_text()) + config["server_params"]["served_model_name"] = "" + (cell,) = config["runs"] + config["sweeps"][cell]["tokenizer"] = "" + filled.write_text(json.dumps(config)) + with self.assertRaises(ConfigError) as ctx: + load_variant(filled, {"username": "test"}) + self.assertEqual( + sorted(problem.split(":", 1)[0] for problem in ctx.exception.problems), + ["server_params.served_model_name", f"sweeps.{cell}.tokenizer"], + ) + + +class TestContainerForOrchestrator(unittest.TestCase): + def test_merged_container_has_only_keys_docker_reads(self): + with tempfile.TemporaryDirectory() as tmp: + tmp = Path(tmp) + (tmp / "threshold.json").write_text("{}") + config = { + "enforce_thresholds": False, + "threshold_json": str(tmp / "threshold.json"), + "paths": {"shared_fs": "/home/x", "models_dir": "/models", "log_dir": "/logs", "hf_token_file": "/t"}, + "container": { + "name": "vllm", + "image": "rocm/vllm:1", + "env": {"NCCL_DEBUG": "INFO"}, + "runtime": {"name": "docker", "args": {"volumes": ["/models:/models:ro"]}}, + }, + "server_params": {"model": "/models/m", "tensor_parallel_size": 8}, + "sweeps": {CELL: {}}, + "runs": [CELL], + } + (tmp / "config.json").write_text(json.dumps(config)) + loaded = load_config(VariantConfig, tmp / "config.json", {}) + self.assertEqual( + loaded.container, + { + "lifetime": "per_run", + "name": "vllm", + "image": "rocm/vllm:1", + "env": {"NCCL_DEBUG": "INFO"}, + "runtime": { + "name": "docker", + "args": { + "network": "host", + "ipc": "host", + "privileged": True, + "volumes": ["/models:/models:ro"], + "devices": [], + }, + }, + }, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/cvs/lib/inference/unittests/test_vllm_config_contract.py b/cvs/lib/inference/utils/unittests/test_vllm_schema.py similarity index 58% rename from cvs/lib/inference/unittests/test_vllm_config_contract.py rename to cvs/lib/inference/utils/unittests/test_vllm_schema.py index 01085d1a1..ddbc74500 100644 --- a/cvs/lib/inference/unittests/test_vllm_config_contract.py +++ b/cvs/lib/inference/utils/unittests/test_vllm_schema.py @@ -1,15 +1,13 @@ -import json +'''Unit tests for the vLLM config rules in cvs.lib.inference.utils.vllm_schema.''' + import unittest -from pathlib import Path from pydantic import ValidationError -from cvs.lib.inference.utils.vllm_config_loader import ( +from cvs.lib.inference.utils.vllm_schema import ( ContainerEnvTokenError, RunCell, VariantConfig, - _strip_metadata, - load_variant, serialize_cli_options, ) from cvs.unittests.support import FAKE_CRED @@ -19,11 +17,6 @@ FAKE_CRED_FILE = "/home/test/.hf_token" -def _packaged_configs(): - root = Path(__file__).resolve().parents[3] / "input" / "config_file" / "inference" / "vllm" - return sorted(path for path in root.glob("*.json") if not path.name.endswith("threshold.json")) - - def _config(**overrides): value = { "enforce_thresholds": False, @@ -51,6 +44,12 @@ def _config(**overrides): return value +def _with_args(args): + config = _config() + config["container"]["runtime"]["args"] = args + return config + + class TestRunCell(unittest.TestCase): def test_parses_only_canonical_cell_keys(self): cell = RunCell.parse(CELL) @@ -66,7 +65,7 @@ def test_parses_only_canonical_cell_keys(self): RunCell.parse(invalid) -class TestVllmConfigContract(unittest.TestCase): +class TestRuns(unittest.TestCase): def test_resolves_selected_run_with_cell_overrides(self): variant = VariantConfig(**_config()) (run,) = variant.resolved_runs() @@ -93,9 +92,8 @@ def test_rejects_mismatched_parallelism_and_missing_enforced_threshold(self): with self.assertRaisesRegex(ValidationError, "missing threshold"): VariantConfig(**_config(enforce_thresholds=True)) - def test_record_only_mode_allows_unmeasured_selected_cells(self): - self.assertEqual(VariantConfig(**_config()).expected_cells(), [CELL]) +class TestContainer(unittest.TestCase): def test_accepts_complete_container_network_environment(self): config = _config() config["container"]["env"] = { @@ -135,43 +133,34 @@ def test_rejects_hf_token_in_container_env_without_echoing_value(self): self.assertIn("paths.hf_token_file", str(ctx.exception)) self.assertNotIn(FAKE_CRED, repr(ctx.exception.args)) - def test_other_container_errors_keep_their_input(self): - config = _config() - config["container"]["env"] = {"NCCL_SOCKET_IFNAME": "eno0"} - with self.assertRaises(ValidationError) as ctx: - VariantConfig(**config) - self.assertIn("input_value", str(ctx.exception)) - - def test_preserves_runtime_environment_without_model_policy(self): - for env in ( - {"VLLM_ROCM_USE_AITER": "0", "VLLM_ROCM_USE_AITER_MHA": "0"}, - {"VLLM_ROCM_USE_AITER": "1", "VLLM_ROCM_USE_AITER_MHA": "0"}, - {"CUSTOM_RUNTIME_OPTION": "custom value"}, - ): - with self.subTest(env=env): - config = _config() - config["container"]["env"] = env - variant = VariantConfig(**config) - self.assertEqual(variant.container.env, env) - - -class TestMetadataAndOptionSerialization(unittest.TestCase): - def test_strips_local_comments_but_preserves_option_payload(self): - value = _strip_metadata( - { - "_comment": "root", - "server_params": { - "_comment": "server", - "json_arg": {"_comment": "upstream payload"}, - }, - "sweeps": {CELL: {"_comment": "cell", "json_arg": {"_comment": "upstream payload"}}}, - "accuracy": {"tasks": [{"_comment": "task", "id": "gsm8k", "task": "gsm8k"}]}, - } + def test_runtime_args_accept_docker_keys_but_not_env(self): + variant = VariantConfig(**_with_args({"shm_size": "64g", "cap_add": ["SYS_PTRACE"], "ulimit": ["memlock=-1"]})) + self.assertEqual(variant.container.model_dump()["runtime"]["args"]["shm_size"], "64g") + for args in ({"env": {"NCCL_DEBUG": "INFO"}}, {"unknown_flag": True}): + with self.subTest(args=args): + with self.assertRaises(ValidationError): + VariantConfig(**_with_args(args)) + + +class TestCommentKeysAndOptions(unittest.TestCase): + def test_comment_keys_are_ignored_but_option_payloads_keep_theirs(self): + config = _config( + _comment="root", + server_params={ + "_comment": "server", + "model": "/models/x", + "tensor_parallel_size": 8, + "pipeline_parallel_size": 2, + "json_arg": {"_comment": "upstream payload"}, + }, + sweeps={"_comment": "cells", CELL: {"_comment": "cell", "json_arg": {"_comment": "upstream payload"}}}, + accuracy={"_comment": "accuracy", "tasks": [{"_comment": "task", "id": "gsm8k", "task": "gsm8k"}]}, ) - self.assertNotIn("_comment", value) - self.assertEqual(value["server_params"]["json_arg"], {"_comment": "upstream payload"}) - self.assertEqual(value["sweeps"][CELL]["json_arg"], {"_comment": "upstream payload"}) - self.assertNotIn("_comment", value["accuracy"]["tasks"][0]) + config["container"]["runtime"]["args"] = {"_comment_mounts": "notes", "volumes": ["/data:/data"]} + variant = VariantConfig(**config) + self.assertEqual(variant.server_params.extra_options(), {"json_arg": {"_comment": "upstream payload"}}) + self.assertEqual(variant.sweeps, {CELL: {"json_arg": {"_comment": "upstream payload"}}}) + self.assertEqual(variant.accuracy.tasks[0].id, "gsm8k") def test_serializes_documented_value_shapes(self): options = { @@ -214,24 +203,50 @@ def test_rejects_ambiguous_false_and_reserved_names(self): VariantConfig(**_config(benchmark_params={"num_prompts": 25, "percentile_metrics": "ttft"})) -class TestPackagedVllmCatalog(unittest.TestCase): - def test_shipped_configs_resolve_their_declared_contract(self): - configs = _packaged_configs() - self.assertTrue(configs, "No packaged vLLM configs found") - for path in configs: - with self.subTest(config=path.name): - raw = json.loads(path.read_text()) - threshold_path = path.parent / raw["threshold_json"] - self.assertTrue(threshold_path.is_file(), f"Missing packaged threshold file: {threshold_path}") - variant = load_variant(path, {"username": "test"}) - runs = variant.resolved_runs() - self.assertTrue(runs) - self.assertEqual([run.cell.key for run in runs], variant.runs) - self.assertTrue(set(variant.runs) <= set(variant.sweeps)) - self.assertTrue(set(variant.thresholds) - {"accuracy"} <= set(variant.sweeps)) - for run in runs: - self.assertEqual(run.cell.tp, variant.server_params.tensor_parallel_size) - self.assertEqual(run.cell.pp, variant.server_params.pipeline_parallel_size) +class TestThresholds(unittest.TestCase): + def _variant(self, thresholds, enforce=False): + return VariantConfig(**_config(enforce_thresholds=enforce, thresholds=thresholds)) + + def test_partial_and_empty_cells_load_with_enforcement_off_and_on(self): + for thresholds in ({CELL: {"output_throughput": {"kind": "min", "value": 0}}}, {CELL: {}}): + for enforce in (False, True): + with self.subTest(thresholds=thresholds, enforce=enforce): + self.assertEqual(self._variant(thresholds, enforce=enforce).thresholds, thresholds) + + def test_enforcement_still_requires_selected_run_cell(self): + self._variant({}, enforce=False) + with self.assertRaisesRegex(ValidationError, "missing threshold coverage"): + self._variant({}, enforce=True) + + def test_underscore_metrics_are_ignored_and_other_unknown_metrics_rejected(self): + spec = {"kind": "min", "value": 0} + variant = self._variant({CELL: {"_comment": "calibrate", "_typo": spec, "_": spec, "output_throughput": spec}}) + self.assertEqual(variant.thresholds, {CELL: {"output_throughput": spec}}) + with self.assertRaisesRegex(ValidationError, "unknown vLLM threshold metric 'notes'"): + self._variant({CELL: {"notes": spec}}) + + def test_rejects_specs_outside_the_vllm_direction_rule(self): + for metric, spec in ( + ("output_throughput", {"kind": "max", "value": 1}), + ("mean_ttft_ms", {"kind": "min", "value": 1}), + ("output_throughput", {"kind": "min_tok_s", "value": 1}), + ("output_throughput", {"kind": "within", "value": 1, "tolerance_pct": 5}), + ("output_throughput", {"kind": "min", "value": 1, "optional": False}), + ("output_throughput", {"kind": "min", "value": 1, "unit": "tok/s"}), + ("output_throughput", {"kind": "min", "value": "1"}), + ("client.output_throughput", {"kind": "min", "value": 1}), + ): + with self.subTest(metric=metric, spec=spec): + with self.assertRaises(ValidationError): + self._variant({CELL: {metric: spec}}) + + def test_accuracy_thresholds_are_not_metric_cells(self): + accuracy = {"gsm8k": {"gsm8k.exact_match__strict-match": {"kind": "min", "value": 0, "tolerance_pct": 5}}} + cell = {"output_throughput": {"kind": "min", "value": 0}} + variant = self._variant({CELL: cell, "accuracy": accuracy}, enforce=True) + self.assertEqual(variant.thresholds["accuracy"], accuracy) + with self.assertRaisesRegex(ValidationError, "threshold cells"): + self._variant({CELL: cell, "accuracy": accuracy, "acuracy": {}}, enforce=True) if __name__ == "__main__": diff --git a/cvs/lib/inference/utils/vllm_config_loader.py b/cvs/lib/inference/utils/vllm_config_loader.py index 07278663d..7c5aea9e1 100644 --- a/cvs/lib/inference/utils/vllm_config_loader.py +++ b/cvs/lib/inference/utils/vllm_config_loader.py @@ -1,390 +1,13 @@ -'''Typed config schema and run resolver for CVS vLLM workloads.''' +'''Load a CVS vLLM config. The schema lives in ``vllm_schema``.''' -from __future__ import annotations +from cvs.lib.inference.utils.vllm_schema import VariantConfig, serialize_cli_options # noqa: F401 - re-exported +from cvs.lib.utils.config_loader import load_config -import json -import re -from dataclasses import dataclass -from typing import Any, Dict, List, Literal, Optional, Union -from pydantic import BaseModel, ConfigDict, Field, PrivateAttr, create_model, model_validator +def load_variant(config_path, cluster): + """Load a vLLM config and its threshold file against a cluster file path or dict. -from cvs.lib.inference.utils.accuracy_config import AccuracyConfig -from cvs.lib.inference.utils.vllm_metrics import METRIC_REGISTRY, validate_threshold_spec -from cvs.lib.utils.config_loader import substitute_config - -GATED_GPU_METRICS = {definition.name for definition in METRIC_REGISTRY if definition.datasource == "gpu"} -GATED_PROM_METRICS = {definition.name for definition in METRIC_REGISTRY if definition.datasource == "prometheus"} -_CELL_RE = re.compile( - r"^ISL=(?P[1-9]\d*),OSL=(?P[1-9]\d*),TP=(?P[1-9]\d*),PP=(?P[1-9]\d*),CONC=(?P[1-9]\d*)$" -) -_METADATA_PREFIXES = ("_comment", "_example") -_SOCKET_ENV = {"NCCL_SOCKET_IFNAME", "GLOO_SOCKET_IFNAME", "TP_SOCKET_IFNAME"} -_HF_TOKEN_ENV = {"HF_TOKEN", "HUGGING_FACE_HUB_TOKEN"} -_SERVER_RESERVED = {"master_addr", "master_port", "nnodes", "node_rank", "headless"} - - -class _Forbid(BaseModel): - model_config = ConfigDict(extra="forbid") - - -class _Options(BaseModel): - """Strict at the section boundary, flexible only for upstream CLI options.""" - - model_config = ConfigDict(extra="allow") - - def extra_options(self) -> Dict[str, Any]: - return dict(self.model_extra or {}) - - -VllmThresholdSpec = create_model( - "VllmThresholdSpec", - __base__=_Forbid, - kind=(Any, ...), - value=(Any, ...), -) - - -def _validate_vllm_thresholds(thresholds): - for cell_key, cell in thresholds.items(): - if cell_key == "accuracy": - continue - if not isinstance(cell, dict): - raise ValueError(f"vLLM threshold cell {cell_key!r} must be an object") - for metric, raw_spec in cell.items(): - if metric.startswith(_METADATA_PREFIXES): - continue - spec = VllmThresholdSpec.model_validate(raw_spec).model_dump() - validate_threshold_spec(metric, spec) - - -class RuntimeArgs(_Forbid): - network: str = "host" - ipc: str = "host" - privileged: bool = True - volumes: List[str] = Field(default_factory=list) - devices: List[str] = Field(default_factory=list) - - -class Runtime(_Forbid): - name: Literal["docker"] = "docker" - args: RuntimeArgs = Field(default_factory=RuntimeArgs) - - -class ContainerEnvTokenError(Exception): - """A Hugging Face token name in container.env. - - Not a ValueError, so pydantic raises it as is instead of wrapping it in a - ValidationError whose str(), errors() and json() would carry the token value. + Raises ``ConfigError`` listing every invalid field, including each + ```` placeholder still in the config. """ - - -class ContainerConfig(_Forbid): - lifetime: Literal["no_launch", "per_run", "persistent"] = "per_run" - name: str - image: str - env: Dict[str, str] = Field(default_factory=dict) - runtime: Runtime = Field(default_factory=Runtime) - - @model_validator(mode="before") - @classmethod - def _reject_hf_token_env(cls, data): - env = data.get("env") if isinstance(data, dict) else None - tokens = sorted(name for name in env or {} if str(name).upper() in _HF_TOKEN_ENV) - if tokens: - raise ContainerEnvTokenError( - f"container.env must not set {tokens}: container.env values become docker run -e arguments. " - "Put the Hugging Face token in the file named by paths.hf_token_file instead." - ) - return data - - @model_validator(mode="after") - def _validate_network_env(self): - configured_sockets = _SOCKET_ENV & set(self.env) - if configured_sockets and configured_sockets != _SOCKET_ENV: - missing = sorted(_SOCKET_ENV - configured_sockets) - raise ValueError(f"container.env must set all socket interface variables together; missing: {missing}") - empty_sockets = sorted(name for name in configured_sockets if not self.env[name].strip()) - if empty_sockets: - raise ValueError(f"container.env socket interface variables cannot be empty: {empty_sockets}") - if "NCCL_IB_HCA" in self.env and not self.nccl_ib_hcas: - raise ValueError("container.env.NCCL_IB_HCA must name at least one device") - return self - - @property - def nccl_ib_hcas(self): - value = self.env.get("NCCL_IB_HCA") - if value is None: - return None - return [device.strip() for device in value.split(",") if device.strip()] - - @property - def socket_env_configured(self): - return bool(_SOCKET_ENV & set(self.env)) - - -class Paths(_Forbid): - shared_fs: str - models_dir: str - log_dir: str - hf_token_file: str - - -class ServerParams(_Options): - """Harness fields plus arbitrary vLLM ``serve`` flags in snake_case.""" - - backend: Literal["vllm"] = "vllm" - model: str - tensor_parallel_size: int - pipeline_parallel_size: int = 1 - port: int = 8888 - dist_init_port: int = 29501 - server_poll_iterations: int = 60 - server_poll_wait_s: int = 60 - server_warmup_wait_s: int = 330 - distributed_executor_backend: Literal["mp", "ray"] = "mp" - - @model_validator(mode="after") - def _validate_upstream_options(self): - options = self.extra_options() - _validate_cli_option_map(options, section="server_params") - collisions = sorted(_SERVER_RESERVED & set(options)) - if collisions: - raise ValueError(f"server_params cannot override harness fields: {collisions}") - return self - - -class BenchmarkParams(_Options): - """Harness fields plus arbitrary vLLM ``bench serve`` flags in snake_case.""" - - backend: Literal["vllm"] = "vllm" - dataset_name: str = "random" - num_prompts: int = 3200 - request_rate: Union[str, float] = "inf" - burstiness: float = 1.0 - tokenizer_mode: str = "auto" - seed: int = 0 - random_range_ratio: float = 0.0 - random_prefix_len: int = 0 - client_poll_iterations: int = 20 - client_poll_wait_s: int = 60 - client_initial_wait_s: int = 120 - trust_remote_code: bool = False - ignore_eos: bool = True - - @model_validator(mode="after") - def _validate_upstream_options(self): - options = self.extra_options() - _validate_cli_option_map(options, section="benchmark_params") - collisions = sorted(_BENCHMARK_RESERVED & set(options)) - if collisions: - raise ValueError(f"benchmark_params cannot override harness fields: {collisions}") - return self - - -_BENCHMARK_RESERVED = { - "backend", - "base_url", - "model", - "max_concurrency", - "random_input_len", - "random_output_len", - "result_dir", - "result_filename", - "percentile_metrics", - "metric_percentiles", -} -_OPTION_NAME_RE = re.compile(r"^[a-z][a-z0-9_]*$") - - -def _validate_cli_option_map(options: Dict[str, Any], *, section: str) -> None: - for name, value in options.items(): - if not _OPTION_NAME_RE.fullmatch(name): - raise ValueError(f"{section} option names must be snake_case: {name!r}") - if value is False: - raise ValueError(f"{section}.{name}=false is ambiguous; omit it or use the option's negative flag") - if isinstance(value, list) and any(isinstance(item, (list, dict)) for item in value): - raise ValueError(f"{section}.{name} lists may contain scalar values only") - if isinstance(value, dict): - try: - json.dumps(value, separators=(",", ":")) - except (TypeError, ValueError) as exc: - raise ValueError(f"{section}.{name} must be JSON serializable") from exc - - -def serialize_cli_options(options: Dict[str, Any]) -> List[str]: - """Serialize generic snake-case option maps for vLLM command lines.""" - _validate_cli_option_map(options, section="option") - argv = [] - for name, value in options.items(): - if value is None: - continue - flag = f"--{name.replace('_', '-')}" - if value is True: - argv.append(flag) - elif isinstance(value, list): - argv.extend([flag, *(str(item) for item in value)]) - elif isinstance(value, dict): - argv.extend([flag, json.dumps(value, separators=(",", ":"))]) - else: - argv.extend([flag, str(value)]) - return argv - - -@dataclass(frozen=True) -class RunCell: - key: str - isl: int - osl: int - tp: int - pp: int - concurrency: int - - @classmethod - def parse(cls, value: str) -> RunCell: - match = _CELL_RE.fullmatch(value) - if not match: - raise ValueError("run cell must be canonical ISL=,OSL=,TP=,PP=,CONC=") - values = {name: int(number) for name, number in match.groupdict().items()} - return cls(value, **values) - - -@dataclass(frozen=True) -class ResolvedRun: - cell: RunCell - benchmark_params: Dict[str, Any] - - -def _strip_metadata(value: Any) -> Any: - """Remove local comments at schema boundaries without touching option payloads.""" - if not isinstance(value, dict): - return value - cleaned = {key: item for key, item in value.items() if not key.startswith(_METADATA_PREFIXES)} - for section in ("paths", "container", "runtime", "args", "accuracy"): - if isinstance(cleaned.get(section), dict): - cleaned[section] = _strip_metadata(cleaned[section]) - if isinstance(cleaned.get("tasks"), list): - cleaned["tasks"] = [ - {key: item for key, item in task.items() if not key.startswith(_METADATA_PREFIXES)} - if isinstance(task, dict) - else task - for task in cleaned["tasks"] - ] - for section in ("server_params", "benchmark_params"): - if isinstance(cleaned.get(section), dict): - cleaned[section] = { - key: item for key, item in cleaned[section].items() if not key.startswith(_METADATA_PREFIXES) - } - if isinstance(cleaned.get("sweeps"), dict): - cleaned["sweeps"] = { - key: { - option: option_value - for option, option_value in values.items() - if not option.startswith(_METADATA_PREFIXES) - } - if isinstance(values, dict) - else values - for key, values in cleaned["sweeps"].items() - if not key.startswith(_METADATA_PREFIXES) - } - return cleaned - - -class VariantConfig(_Forbid): - enforce_thresholds: bool = True - threshold_json: str - ib_hca_devices: Union[Literal["auto"], List[str], None] = "auto" - ib_netdev: Optional[str] = None - paths: Paths - container: ContainerConfig - server_params: ServerParams - benchmark_params: BenchmarkParams = Field(default_factory=BenchmarkParams) - sweeps: Dict[str, Dict[str, Any]] - runs: List[str] - thresholds: Dict[str, Dict[str, Any]] = Field(default_factory=dict) - accuracy: AccuracyConfig = Field(default_factory=AccuracyConfig) - _effective_topology: Any = PrivateAttr(default=None) - _run_cells: Dict[str, RunCell] = PrivateAttr(default_factory=dict) - - @model_validator(mode="after") - def _validate_runs_and_thresholds(self): - _validate_vllm_thresholds(self.thresholds) - if not self.runs: - raise ValueError("runs must be a nonempty explicit list") - parsed = {key: RunCell.parse(key) for key in self.sweeps} - if len(set(self.runs)) != len(self.runs): - raise ValueError("runs contains duplicate cells") - unknown_runs = sorted(set(self.runs) - set(parsed)) - if unknown_runs: - raise ValueError(f"runs reference unknown sweeps: {unknown_runs}") - for cell in parsed.values(): - if ( - cell.tp != self.server_params.tensor_parallel_size - or cell.pp != self.server_params.pipeline_parallel_size - ): - raise ValueError(f"{cell.key} conflicts with server_params tensor/pipeline parallel size") - for cell_key, overrides in self.sweeps.items(): - if not isinstance(overrides, dict): - raise ValueError(f"sweeps.{cell_key} must be an object") - reserved = sorted(_BENCHMARK_RESERVED & set(overrides)) - if reserved: - raise ValueError(f"sweeps.{cell_key} cannot override harness fields: {reserved}") - boolean_fields = {"ignore_eos", "trust_remote_code"} & set(overrides) - for name in boolean_fields: - if not isinstance(overrides[name], bool): - raise ValueError(f"sweeps.{cell_key}.{name} must be boolean") - cli_overrides = {name: value for name, value in overrides.items() if name not in boolean_fields} - _validate_cli_option_map(cli_overrides, section=f"sweeps.{cell_key}") - threshold_cells = set(self.thresholds) - {"accuracy"} - unknown_thresholds = sorted(threshold_cells - set(parsed)) - if unknown_thresholds: - raise ValueError(f"threshold cells have no matching sweep: {unknown_thresholds}") - if self.enforce_thresholds: - missing = [key for key in self.runs if key not in threshold_cells] - if missing: - raise ValueError(f"selected runs missing threshold coverage: {missing}") - self._run_cells = parsed - return self - - @property - def model_id(self) -> str: - return self.server_params.model - - def bind_effective_topology(self, topology) -> None: - self._effective_topology = topology - - def cell(self, key: str) -> RunCell: - return self._run_cells[key] - - def expected_cells(self) -> List[str]: - return list(self.runs) - - def cell_key(self, isl, osl, concurrency, **_unused) -> str: - return ( - f"ISL={isl},OSL={osl},TP={self.server_params.tensor_parallel_size}," - f"PP={self.server_params.pipeline_parallel_size},CONC={concurrency}" - ) - - def resolved_runs(self) -> List[ResolvedRun]: - base = self.benchmark_params.model_dump() - extras = self.benchmark_params.extra_options() - resolved = [] - for key in self.runs: - values = {**base, **extras, **self.sweeps[key]} - values.update( - { - "random_input_len": self.cell(key).isl, - "random_output_len": self.cell(key).osl, - "max_concurrency": self.cell(key).concurrency, - } - ) - resolved.append(ResolvedRun(cell=self.cell(key), benchmark_params=values)) - return resolved - - -def load_variant(config_path, cluster_dict): - """Load a vLLM config, threshold file, placeholders, and selected runs.""" - raw, thresholds = substitute_config(config_path, cluster_dict) - raw = _strip_metadata(raw) - raw["thresholds"] = thresholds - return VariantConfig(**raw) + return load_config(VariantConfig, config_path, cluster).config diff --git a/cvs/lib/inference/utils/vllm_schema.py b/cvs/lib/inference/utils/vllm_schema.py new file mode 100644 index 000000000..16e84f5ec --- /dev/null +++ b/cvs/lib/inference/utils/vllm_schema.py @@ -0,0 +1,380 @@ +'''Typed config schema and run resolver for CVS vLLM workloads.''' + +from __future__ import annotations + +import json +import re +from dataclasses import dataclass +from typing import Annotated, Any, Dict, List, Literal, Optional, Union + +from pydantic import AfterValidator, ConfigDict, Field, PrivateAttr, field_validator, model_validator + +from cvs.lib.inference.utils.accuracy_config import AccuracyConfig +from cvs.lib.inference.utils.vllm_metrics import validate_threshold_spec +from cvs.lib.utils.config_blocks import ( + PLACEHOLDER_MESSAGE, + ConfigStr, + ContainerSpec, + Paths, + RuntimeArgs, + RuntimeSpec, + ThresholdFile, + _Allow, + _Forbid, + has_placeholder, + placeholders_rejected, +) + +_CELL_RE = re.compile( + r"^ISL=(?P[1-9]\d*),OSL=(?P[1-9]\d*),TP=(?P[1-9]\d*),PP=(?P[1-9]\d*),CONC=(?P[1-9]\d*)$" +) +_SOCKET_ENV = {"NCCL_SOCKET_IFNAME", "GLOO_SOCKET_IFNAME", "TP_SOCKET_IFNAME"} +_HF_TOKEN_ENV = {"HF_TOKEN", "HUGGING_FACE_HUB_TOKEN"} +_SERVER_RESERVED = {"master_addr", "master_port", "nnodes", "node_rank", "headless"} + + +def _reject_option_placeholder(value, info): + if not placeholders_rejected(info): + return value + text = value if isinstance(value, str) else json.dumps(value, default=str) + if has_placeholder(text): + raise ValueError(PLACEHOLDER_MESSAGE) + return value + + +OptionValue = Annotated[Any, AfterValidator(_reject_option_placeholder)] + + +class _Options(_Allow): + """Strict at the section boundary, flexible only for upstream CLI options.""" + + __pydantic_extra__: Dict[str, OptionValue] + + def extra_options(self): + return dict(self.model_extra or {}) + + +def _validate_vllm_thresholds(thresholds): + """Check every cell spec and return the thresholds without comment keys. + + The report code reads ``variant.thresholds`` as plain dicts, so the specs + are returned as written rather than as ``ThresholdSpec`` models. + """ + cells = ThresholdFile.model_validate(thresholds).cells() + for cell_key, cell in cells.items(): + for metric, spec in cell.items(): + if "optional" in spec.model_fields_set: + raise ValueError(f"{cell_key}.{metric}: vLLM thresholds do not support optional") + validate_threshold_spec(metric, spec.model_dump()) + cleaned = {cell_key: {metric: thresholds[cell_key][metric] for metric in cell} for cell_key, cell in cells.items()} + if "accuracy" in thresholds: + cleaned["accuracy"] = thresholds["accuracy"] + return cleaned + + +class VllmRuntimeArgs(RuntimeArgs): + model_config = ConfigDict(extra="forbid") + + network: str = "host" + ipc: str = "host" + privileged: bool = True + volumes: List[ConfigStr] = Field(default_factory=list) + devices: List[str] = Field(default_factory=list) + + # container.env is the one env block, so the Hugging Face token check + # cannot be bypassed through docker run -e arguments built from here. + @model_validator(mode="before") + @classmethod + def _reject_env(cls, data): + if isinstance(data, dict) and "env" in data: + raise ValueError("vLLM does not read runtime.args.env; set these variables in container.env") + return data + + +class VllmRuntime(RuntimeSpec): + model_config = ConfigDict(extra="forbid") + + name: Literal["docker"] = "docker" + args: VllmRuntimeArgs = Field(default_factory=VllmRuntimeArgs) + + +class ContainerEnvTokenError(Exception): + """A Hugging Face token name in container.env. + + Not a ValueError, so pydantic raises it as is instead of wrapping it in a + ValidationError whose str(), errors() and json() would carry the token value. + """ + + +class VllmContainerSpec(ContainerSpec): + runtime: VllmRuntime = Field(default_factory=VllmRuntime) + + @model_validator(mode="before") + @classmethod + def _reject_hf_token_env(cls, data): + env = data.get("env") if isinstance(data, dict) else None + tokens = sorted(name for name in env or {} if str(name).upper() in _HF_TOKEN_ENV) + if tokens: + raise ContainerEnvTokenError( + f"container.env must not set {tokens}: container.env values become docker run -e arguments. " + "Put the Hugging Face token in the file named by paths.hf_token_file instead." + ) + return data + + @model_validator(mode="after") + def _validate_network_env(self): + configured_sockets = _SOCKET_ENV & set(self.env) + if configured_sockets and configured_sockets != _SOCKET_ENV: + missing = sorted(_SOCKET_ENV - configured_sockets) + raise ValueError(f"container.env must set all socket interface variables together; missing: {missing}") + empty_sockets = sorted(name for name in configured_sockets if not self.env[name].strip()) + if empty_sockets: + raise ValueError(f"container.env socket interface variables cannot be empty: {empty_sockets}") + if "NCCL_IB_HCA" in self.env and not self.nccl_ib_hcas: + raise ValueError("container.env.NCCL_IB_HCA must name at least one device") + return self + + @property + def nccl_ib_hcas(self): + value = self.env.get("NCCL_IB_HCA") + if value is None: + return None + return [device.strip() for device in value.split(",") if device.strip()] + + @property + def socket_env_configured(self): + return bool(_SOCKET_ENV & set(self.env)) + + +class ServerParams(_Options): + """Harness fields plus arbitrary vLLM ``serve`` flags in snake_case.""" + + backend: Literal["vllm"] = "vllm" + model: ConfigStr + tensor_parallel_size: int + pipeline_parallel_size: int = 1 + port: int = 8888 + dist_init_port: int = 29501 + server_poll_iterations: int = 60 + server_poll_wait_s: int = 60 + server_warmup_wait_s: int = 330 + distributed_executor_backend: Literal["mp", "ray"] = "mp" + + @model_validator(mode="after") + def _validate_upstream_options(self): + options = self.extra_options() + _validate_cli_option_map(options, section="server_params") + collisions = sorted(_SERVER_RESERVED & set(options)) + if collisions: + raise ValueError(f"server_params cannot override harness fields: {collisions}") + return self + + +class BenchmarkParams(_Options): + """Harness fields plus arbitrary vLLM ``bench serve`` flags in snake_case.""" + + backend: Literal["vllm"] = "vllm" + dataset_name: str = "random" + num_prompts: int = 3200 + request_rate: Union[str, float] = "inf" + burstiness: float = 1.0 + tokenizer_mode: str = "auto" + seed: int = 0 + random_range_ratio: float = 0.0 + random_prefix_len: int = 0 + client_poll_iterations: int = 20 + client_poll_wait_s: int = 60 + client_initial_wait_s: int = 120 + trust_remote_code: bool = False + ignore_eos: bool = True + + @model_validator(mode="after") + def _validate_upstream_options(self): + options = self.extra_options() + _validate_cli_option_map(options, section="benchmark_params") + collisions = sorted(_BENCHMARK_RESERVED & set(options)) + if collisions: + raise ValueError(f"benchmark_params cannot override harness fields: {collisions}") + return self + + +_BENCHMARK_RESERVED = { + "backend", + "base_url", + "model", + "max_concurrency", + "random_input_len", + "random_output_len", + "result_dir", + "result_filename", + "percentile_metrics", + "metric_percentiles", +} +_OPTION_NAME_RE = re.compile(r"^[a-z][a-z0-9_]*$") + + +def _validate_cli_option_map(options, *, section): + for name, value in options.items(): + if not _OPTION_NAME_RE.fullmatch(name): + raise ValueError(f"{section} option names must be snake_case: {name!r}") + if value is False: + raise ValueError(f"{section}.{name}=false is ambiguous; omit it or use the option's negative flag") + if isinstance(value, list) and any(isinstance(item, (list, dict)) for item in value): + raise ValueError(f"{section}.{name} lists may contain scalar values only") + if isinstance(value, dict): + try: + json.dumps(value, separators=(",", ":")) + except (TypeError, ValueError) as exc: + raise ValueError(f"{section}.{name} must be JSON serializable") from exc + + +def serialize_cli_options(options): + """Serialize generic snake-case option maps for vLLM command lines.""" + _validate_cli_option_map(options, section="option") + argv = [] + for name, value in options.items(): + if value is None: + continue + flag = f"--{name.replace('_', '-')}" + if value is True: + argv.append(flag) + elif isinstance(value, list): + argv.extend([flag, *(str(item) for item in value)]) + elif isinstance(value, dict): + argv.extend([flag, json.dumps(value, separators=(",", ":"))]) + else: + argv.extend([flag, str(value)]) + return argv + + +@dataclass(frozen=True) +class RunCell: + key: str + isl: int + osl: int + tp: int + pp: int + concurrency: int + + @classmethod + def parse(cls, value): + match = _CELL_RE.fullmatch(value) + if not match: + raise ValueError("run cell must be canonical ISL=,OSL=,TP=,PP=,CONC=") + values = {name: int(number) for name, number in match.groupdict().items()} + return cls(value, **values) + + +@dataclass(frozen=True) +class ResolvedRun: + cell: RunCell + benchmark_params: Dict[str, Any] + + +class VariantConfig(_Forbid): + enforce_thresholds: bool = True + threshold_json: str + ib_hca_devices: Union[Literal["auto"], List[str], None] = "auto" + ib_netdev: Optional[str] = None + paths: Paths + container: VllmContainerSpec + server_params: ServerParams + benchmark_params: BenchmarkParams = Field(default_factory=BenchmarkParams) + sweeps: Dict[str, Dict[str, OptionValue]] + runs: List[str] + thresholds: Dict[str, Dict[str, Any]] = Field(default_factory=dict) + accuracy: AccuracyConfig = Field(default_factory=AccuracyConfig) + _effective_topology: Any = PrivateAttr(default=None) + _run_cells: Dict[str, RunCell] = PrivateAttr(default_factory=dict) + + @field_validator("sweeps", mode="before") + @classmethod + def _drop_sweep_comments(cls, value): + if not isinstance(value, dict): + return value + return { + key: {name: item for name, item in entry.items() if not name.startswith("_")} + if isinstance(entry, dict) + else entry + for key, entry in value.items() + if not key.startswith("_") + } + + @field_validator("thresholds") + @classmethod + def _check_thresholds(cls, value): + return _validate_vllm_thresholds(value) + + @model_validator(mode="after") + def _validate_runs_and_thresholds(self): + if not self.runs: + raise ValueError("runs must be a nonempty explicit list") + parsed = {key: RunCell.parse(key) for key in self.sweeps} + if len(set(self.runs)) != len(self.runs): + raise ValueError("runs contains duplicate cells") + unknown_runs = sorted(set(self.runs) - set(parsed)) + if unknown_runs: + raise ValueError(f"runs reference unknown sweeps: {unknown_runs}") + for cell in parsed.values(): + if ( + cell.tp != self.server_params.tensor_parallel_size + or cell.pp != self.server_params.pipeline_parallel_size + ): + raise ValueError(f"{cell.key} conflicts with server_params tensor/pipeline parallel size") + for cell_key, overrides in self.sweeps.items(): + if not isinstance(overrides, dict): + raise ValueError(f"sweeps.{cell_key} must be an object") + reserved = sorted(_BENCHMARK_RESERVED & set(overrides)) + if reserved: + raise ValueError(f"sweeps.{cell_key} cannot override harness fields: {reserved}") + boolean_fields = {"ignore_eos", "trust_remote_code"} & set(overrides) + for name in boolean_fields: + if not isinstance(overrides[name], bool): + raise ValueError(f"sweeps.{cell_key}.{name} must be boolean") + cli_overrides = {name: value for name, value in overrides.items() if name not in boolean_fields} + _validate_cli_option_map(cli_overrides, section=f"sweeps.{cell_key}") + threshold_cells = set(self.thresholds) - {"accuracy"} + unknown_thresholds = sorted(threshold_cells - set(parsed)) + if unknown_thresholds: + raise ValueError(f"threshold cells have no matching sweep: {unknown_thresholds}") + if self.enforce_thresholds: + missing = [key for key in self.runs if key not in threshold_cells] + if missing: + raise ValueError(f"selected runs missing threshold coverage: {missing}") + self._run_cells = parsed + return self + + @property + def model_id(self): + return self.server_params.model + + def bind_effective_topology(self, topology): + self._effective_topology = topology + + def cell(self, key): + return self._run_cells[key] + + def expected_cells(self): + return list(self.runs) + + def cell_key(self, isl, osl, concurrency, **_unused): + return ( + f"ISL={isl},OSL={osl},TP={self.server_params.tensor_parallel_size}," + f"PP={self.server_params.pipeline_parallel_size},CONC={concurrency}" + ) + + def resolved_runs(self): + base = self.benchmark_params.model_dump() + extras = self.benchmark_params.extra_options() + resolved = [] + for key in self.runs: + values = {**base, **extras, **self.sweeps[key]} + values.update( + { + "random_input_len": self.cell(key).isl, + "random_output_len": self.cell(key).osl, + "max_concurrency": self.cell(key).concurrency, + } + ) + resolved.append(ResolvedRun(cell=self.cell(key), benchmark_params=values)) + return resolved diff --git a/cvs/tests/inference/vllm/conftest.py b/cvs/tests/inference/vllm/conftest.py index 927c88c57..9d9b51fdf 100644 --- a/cvs/tests/inference/vllm/conftest.py +++ b/cvs/tests/inference/vllm/conftest.py @@ -3,7 +3,6 @@ All rights reserved. ''' -import json from pathlib import Path import pytest @@ -11,8 +10,8 @@ from cvs.core.orchestrators.factory import OrchestratorConfig, OrchestratorFactory from cvs.lib import globals from cvs.lib.inference.vllm_topology import resolve_vllm_topology, scope_vllm_cluster -from cvs.lib.inference.utils.vllm_config_loader import load_variant from cvs.lib.inference.utils.vllm_metrics import VLLM_RESULTS_COLUMNS +from cvs.lib.inference.utils.vllm_schema import VariantConfig from cvs.lib.report.benchmark_metric_registry import ( benchmark_metric_columns_for_nodeid, benchmark_metric_rows_from_item, @@ -26,7 +25,7 @@ render_benchmark_metrics_html, ) from cvs.lib.report.subtest_reports import called_from_subtest_context, is_subtest_report -from cvs.lib.utils_lib import resolve_cluster_config_placeholders +from cvs.lib.utils.config_loader import load_config from cvs.tests.inference.vllm._shared import validate_vllm_execution_mode log = globals.log @@ -68,39 +67,20 @@ def _attach_metric_panel(report, rows) -> None: stamp_benchmark_metric_rows_on_report(report, rows) -def _deep_merge(base, override): - """Recursively merge `override` onto `base` (dicts merged key-wise, scalars/lists replaced). - - Protects cluster-set SCALAR and DICT container keys (e.g. shm_size, an env - map) from being wiped by a top-level replace: they survive unless the variant - overrides that same key. List keys (e.g. runtime.args, volume mounts) are - REPLACED here, not unioned -- the cluster's list values are recombined with - the variant's additively further downstream, in container.py's getters. - """ - if not (isinstance(base, dict) and isinstance(override, dict)): - return override - out = dict(base) - for k, v in override.items(): - out[k] = _deep_merge(base[k], v) if k in base else v - return out - - @pytest.fixture(scope="module") -def cluster_dict(pytestconfig): +def loaded_config(pytestconfig): cluster_file = pytestconfig.getoption("cluster_file") if not cluster_file: pytest.fail("--cluster_file is required") - with open(cluster_file) as fp: - d = json.load(fp) - return resolve_cluster_config_placeholders(d) - - -@pytest.fixture(scope="module") -def variant_config(pytestconfig, cluster_dict): config_file = pytestconfig.getoption("config_file") if not config_file: pytest.fail("--config_file is required") - return load_variant(config_file, cluster_dict) + return load_config(VariantConfig, config_file, cluster_file) + + +@pytest.fixture(scope="module") +def variant_config(loaded_config): + return loaded_config.config class _Lifecycle: @@ -134,7 +114,7 @@ def lifecycle(): @pytest.fixture(scope="module") -def orch(cluster_dict, variant_config, lifecycle, vllm_mode): +def orch(loaded_config, lifecycle, vllm_mode): """Construct a ContainerOrchestrator and own ONLY its teardown safety net. The actual launch/sshd happen in test_launch_container / test_setup_sshd @@ -143,19 +123,12 @@ def orch(cluster_dict, variant_config, lifecycle, vllm_mode): the container is still torn down here. When test_teardown ran successfully it sets lifecycle.torn_down, so the finalizer no-ops (no double teardown). """ - # OrchestratorConfig.from_configs does a top-level dict.update, so a bare variant - # container block would wipe the cluster file's container settings. Deep-merge the - # variant ONTO the cluster block so cluster-set scalar/dict keys survive, with the - # variant winning on conflicting keys. (List keys like runtime.args are replaced - # here but recombined additively downstream in container.py's getters.) - suite_cluster = scope_vllm_cluster(vllm_mode, cluster_dict) - container_block = _deep_merge( - suite_cluster.get("container", {}), - variant_config.container.model_dump(), - ) + # from_configs replaces top-level keys, so the container block passed here must + # already hold the cluster's container settings; load_config merged them. + suite_cluster = scope_vllm_cluster(vllm_mode, loaded_config.cluster) testsuite_config = { "orchestrator": "container", - "container": container_block, + "container": loaded_config.container, } cfg = OrchestratorConfig.from_configs(suite_cluster, testsuite_config) o = OrchestratorFactory.create_orchestrator(log, cfg) diff --git a/cvs/unittests/support.py b/cvs/unittests/support.py index 4f3c33787..bcb099289 100644 --- a/cvs/unittests/support.py +++ b/cvs/unittests/support.py @@ -10,8 +10,11 @@ Tests also use ``FAKE_CRED`` instead of credential-looking literals. ''' +import json +import re import subprocess # nosec B404 - test helper; callers pass argv lists of commands the test itself builds import xml.etree.ElementTree as ElementTree # nosec B405 - parses only files the calling test generated +from pathlib import Path TMP = "/tmp" # nosec B108 - CVS uses /tmp as its shared remote scratch dir; tests assert the real paths FAKE_CRED = "fake-cred" @@ -32,3 +35,30 @@ def run_trusted(argv, **kwargs): def parse_trusted_xml(path): """Parse an XML file the calling test generated and return the ElementTree.""" return ElementTree.parse(path) # nosec B314 - input is a file the calling test generated + + +_PLACEHOLDER_RE = re.compile(r"]*)?>", re.IGNORECASE) + + +def write_filled_config(template_path, out_dir): + """Write a copy of a shipped config with every ```` value filled in. + + Comment keys are copied unchanged. ``threshold_json`` is made absolute so + the copy still reads the shipped threshold file. Returns the copy's path. + """ + template_path = Path(template_path) + + def fill(node): + if isinstance(node, dict): + return {key: item if key.startswith("_") else fill(item) for key, item in node.items()} + if isinstance(node, list): + return [fill(item) for item in node] + if isinstance(node, str): + return _PLACEHOLDER_RE.sub("filled", node) + return node + + config = fill(json.loads(template_path.read_text())) + config["threshold_json"] = str(template_path.parent / config["threshold_json"]) + out_path = Path(out_dir) / template_path.name + out_path.write_text(json.dumps(config)) + return out_path diff --git a/docs/how-to/test-suites/inference/vllm.rst b/docs/how-to/test-suites/inference/vllm.rst index 02b5529dc..32d83c567 100644 --- a/docs/how-to/test-suites/inference/vllm.rst +++ b/docs/how-to/test-suites/inference/vllm.rst @@ -68,7 +68,7 @@ Step 2: Fill in the placeholders Replace every ```` before the run. ``{user-id}`` is not one of those: it becomes the login name of the ``cvs run`` process, unless the cluster file sets ``username``. -In the **cluster file**, set your SSH user, private key path, and node addresses. Unresolved ```` strings in the cluster file stop startup. The vLLM loader does not apply that same hard-exit to the workload file, except when ``threshold_json`` itself still contains ````. An unreplaced image name or mount fails later, at pull or ``docker run``. See :doc:`/how-to/run-with-containers` for the cluster-file walkthrough. +In the **cluster file**, set your SSH user, private key path, and node addresses. Unresolved ```` strings in the cluster file stop startup. A ```` or ```` left in the workload file fails test collection, and the error names every field that still has one. See :doc:`/how-to/run-with-containers` for the cluster-file walkthrough. In the **Llama single-node file**, replace only these strings: @@ -313,15 +313,15 @@ CVS then bootstraps a Ray cluster before serving: ``ray start --head --port=1 unless distributed-executor-backend=ray``. You have two or more hosts on ``mp`` without pipeline parallelism. Raise ``pipeline_parallel_size``, or set ``distributed_executor_backend`` to ``"ray"``. **"vllm_distributed requires container.env.NCCL_SOCKET_IFNAME on multi-host clusters".** Set the socket interface. If you set any of ``NCCL_SOCKET_IFNAME``, ``GLOO_SOCKET_IFNAME``, or ``TP_SOCKET_IFNAME``, set all three. -**"Container image not specified in config".** ``container.image`` is empty. The orchestrator logs that line, then ``test_launch_container`` fails because ``setup_containers()`` returned false. The vLLM schema requires ``image``, so omitting the key fails validation instead. An empty string overwrites a cluster-file image. The packaged Llama files use ````, which is not empty and is not a real image until you replace it. +**"Container image not specified in config".** ``container.image`` is empty. The orchestrator logs that line, then ``test_launch_container`` fails because ``setup_containers()`` returned false. The vLLM schema requires ``image``, so omitting the key fails validation instead. An empty string overwrites a cluster-file image. -**"env" under "container.runtime.args".** ``RuntimeArgs`` rejects unknown keys, so ``env`` there fails config load. Put ``env`` on ``container``. +**"env" under "container.runtime.args".** ``env`` under ``runtime.args`` fails config load. Put ``env`` on ``container``. **A threshold fails with "actual must be a finite built-in int or float".** The gated datasource did not produce a valid value. Inspect the raw benchmark artifact, GPU telemetry, or server metrics for that cell. The suite owns benchmark percentile collection; workload configuration cannot override it. diff --git a/docs/reference/configuration-files/inference/vllm.rst b/docs/reference/configuration-files/inference/vllm.rst index 8e8805086..5dd902ef7 100644 --- a/docs/reference/configuration-files/inference/vllm.rst +++ b/docs/reference/configuration-files/inference/vllm.rst @@ -127,6 +127,9 @@ A vLLM configuration file has these top-level keys: ``false`` is rejected on those extra options. The harness booleans ``benchmark_params.ignore_eos`` and ``trust_remote_code`` do accept ``false``. Use an option's negative form instead of ``false`` for any other flag. + Keys that start with ``_`` are comments and are ignored, except inside + ``container.env`` and option values. A ```` or ```` + left in any value fails config load, and the error names every such field. Placeholder substitution ------------------------ @@ -289,7 +292,7 @@ The vLLM ``container`` block selects the image, the container name, the lifetime Container block keys -------------------- -``ContainerConfig`` accepts only these keys. Anything else, including ``image_tar``, is a validation error. +``container`` accepts only these keys. Anything else, including ``image_tar``, is a validation error. .. list-table:: :widths: 3 2 5 @@ -312,21 +315,23 @@ Container block keys - The only value the vLLM schema accepts. * - ``runtime.args`` - see below - - Only ``network``, ``ipc``, ``privileged``, ``volumes``, and ``devices``. + - The Docker keys under :ref:`vllm-runtime-args`. ``env`` is rejected. * - ``env`` - ``{}`` - Environment variables passed as ``docker run -e``. **On** ``container``, not under ``runtime.args``. ``HF_TOKEN`` and ``HUGGING_FACE_HUB_TOKEN`` are rejected; set ``paths.hf_token_file`` instead. -``image_tar``, ``runtime.args.registry``, ``runtime.args.shm_size``, and ``runtime.args.cap_add`` are not fields of this model. Putting them in the vLLM file fails config load. Set those on the cluster file's ``container`` block. The vLLM conftest deep-merges the variant onto that block and keeps cluster keys the variant does not set. The orchestrator still emits them: ``image_tar`` loads an image, ``registry`` logs in before a pull, ``shm_size`` becomes ``--shm-size``, and ``cap_add`` appends to the orchestrator's default capabilities. See :doc:`/reference/cluster/cluster-file` for the orchestrator container keys that page documents. +``image_tar`` is not a field of this model; set it on the cluster file's ``container`` block. ``runtime.args`` keys such as ``registry``, ``shm_size``, and ``cap_add`` can be set in either file. CVS deep-merges the vLLM ``container`` block onto the cluster file's block and keeps cluster keys the vLLM file does not set. The orchestrator emits them: ``image_tar`` loads an image, ``registry`` logs in before a pull, ``shm_size`` becomes ``--shm-size``, and ``cap_add`` appends to the orchestrator's default capabilities. See :doc:`/reference/cluster/cluster-file` for the orchestrator container keys that page documents. .. warning:: - Put ``env`` on ``container``. ``RuntimeArgs`` forbids unknown keys, so ``env`` under ``runtime.args`` is rejected before launch. + Put ``env`` on ``container``. ``env`` under ``runtime.args`` is rejected at config load. + +.. _vllm-runtime-args: Runtime arguments ----------------- -``RuntimeArgs`` accepts only the keys below. Schema defaults are ``network`` ``host``, ``ipc`` ``host``, ``privileged`` true, and empty ``volumes`` and ``devices``. The orchestrator then appends its own mounts and devices; you cannot drop those defaults from the vLLM file. +``runtime.args`` accepts only the keys below. Schema defaults are ``network`` ``host``, ``ipc`` ``host``, ``privileged`` true, and empty ``volumes`` and ``devices``. The orchestrator then appends its own mounts and devices; you cannot drop those defaults from the vLLM file. .. list-table:: :widths: 3 7 @@ -344,6 +349,14 @@ Runtime arguments - ``--ipc``. Schema default ``host``. * - ``privileged`` - ``--privileged`` when true. Schema default true. + * - ``cap_add``, ``security_opt``, ``group_add``, ``ulimit`` + - Lists appended to the orchestrator's default capabilities, security options, groups, and ulimits. + * - ``user`` + - ``--user``. + * - ``shm_size`` + - ``--shm-size``. + * - ``registry`` + - Registry login before a pull. See `Registry authentication`_. The assembled command is: @@ -355,7 +368,7 @@ The container is a long-lived sidecar; every workload command runs through ``doc .. note:: - ``--gpus`` is deliberately never emitted. GPU access on AMD hardware comes from the ``/dev/kfd`` and ``/dev/dri`` device mounts plus the ``video`` group, which the orchestrator adds. Capabilities, security options, groups, ulimits, shared-memory size, registry login, and image-tar load are orchestrator behavior. Set them in the cluster file, not in the vLLM schema. ``runtime.args.shm_size`` in the vLLM file is rejected; the same key on the cluster file is emitted as ``--shm-size``. + ``--gpus`` is deliberately never emitted. GPU access on AMD hardware comes from the ``/dev/kfd`` and ``/dev/dri`` device mounts plus the ``video`` group, which the orchestrator adds. Image-tar load is orchestrator behavior; set ``image_tar`` in the cluster file. Container lifetime ------------------ @@ -386,7 +399,7 @@ The ``lifetime`` key controls when the container is started and stopped relative Registry authentication ----------------------- -``runtime.args.registry`` and top-level ``image_tar`` are rejected in the vLLM file. Set them on the cluster file's ``container`` block if you need a private pull or a ``docker load``. The orchestrator then does the following. ``username`` and ``password_file`` are required; ``server`` defaults to Docker Hub. The password is read from a file on each remote host. There is no inline password key, and the login is kept out of the logs. Login is skipped when ``image_tar`` is set, because a tar load does not pull. +Set ``runtime.args.registry`` in either file, and ``image_tar`` on the cluster file's ``container`` block, if you need a private pull or a ``docker load``. The orchestrator then does the following. ``username`` and ``password_file`` are required; ``server`` defaults to Docker Hub. The password is read from a file on each remote host. There is no inline password key, and the login is kept out of the logs. Login is skipped when ``image_tar`` is set, because a tar load does not pull. Image resolution order at launch: @@ -406,7 +419,7 @@ The variant's ``container`` block is deep-merged **onto** the cluster file's blo .. warning:: - A loaded vLLM ``container`` block always contributes ``lifetime``, ``name``, and ``image``. ``name`` and ``image`` are required, so omitting them fails validation. An empty ``image`` string overwrites a cluster-file image. The orchestrator then logs ``Container image not specified in config`` and ``test_launch_container`` fails because ``setup_containers()`` returned false. Set ``image`` in the vLLM file. The packaged Llama files use ```` there. + A loaded vLLM ``container`` block always contributes ``lifetime``, ``name``, and ``image``. ``name`` and ``image`` are required, so omitting them fails validation. An empty ``image`` string overwrites a cluster-file image. The orchestrator then logs ``Container image not specified in config`` and ``test_launch_container`` fails because ``setup_containers()`` returned false. Set ``image`` in the vLLM file. The packaged files ship ```` there, which fails config load until you replace it. .. _vllm-paths: @@ -775,12 +788,11 @@ Sweep specs are strict at load time regardless of enforcement: - ``kind`` must equal the registry direction, exactly ``min`` or ``max``. - ``value`` must be a finite JSON number. Booleans, strings, null, arrays, objects, NaN, and infinity are rejected. -- Cell-level keys beginning with ``_comment`` or ``_example`` are metadata and - are ignored by metric validation; all other keys must name a registered - metric. +- Cell-level keys beginning with ``_`` are comments and are ignored; all + other keys must name a registered metric. - Prefixed names (``client.*``, ``gpu.*``, ``prom.*``), unknown names, legacy kinds (including ``min_tok_s`` and ``max_ms``), references, tolerances, - units, ``info``, and extra fields are rejected. + units, ``info``, ``optional``, and extra fields are rejected. ``min`` fails below its value and passes at or above it. ``max`` fails above its value and passes at or below it. A gated actual that is missing, null, @@ -1107,13 +1119,15 @@ Troubleshooting * - ``duplicate task id(s)`` - Two ``accuracy.tasks`` entries share an ``id``. * - ``unknown vLLM threshold metric ''`` - - The name is not one of the 56 registry metrics. ``_comment`` and ``_example`` keys are ignored. + - The name is not one of the 56 registry metrics. Keys beginning with ``_`` are ignored. * - `` threshold kind must be '', got ''`` - ``kind`` does not match the registry direction. Use the required ``min`` or ``max`` value shown in the message. * - ``: actual must be a finite built-in int or float, got `` - The gated datasource did not produce a valid value. Inspect the benchmark artifact, GPU telemetry, or server metrics for that cell; percentile collection is harness-owned. * - Unknown key under ``container`` or ``runtime.args`` - - ``ContainerConfig`` and ``RuntimeArgs`` forbid extra keys. ``env`` belongs on ``container``. ``image_tar``, ``registry``, ``shm_size``, and ``cap_add`` belong on the cluster file. See :doc:`/reference/cluster/cluster-file`. + - ``container`` and ``runtime.args`` forbid keys they do not define. ``env`` belongs on ``container``, and ``image_tar`` on the cluster file. See :doc:`/reference/cluster/cluster-file`. + * - ``: still a placeholder; replace it with the value for your setup`` + - A template value was not replaced. The error lists every such field. * - ``runtime.name`` is not ``docker`` - The vLLM schema accepts only ``docker``. From 638d7bb7bbb48287342a1f3217228635a329d64d Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 30 Oct 2026 13:46:14 -0700 Subject: [PATCH 2/3] test(vllm): lock the container merge The merge test loads a cluster env, volume list, and shm_size, and checks that editing the merged container does not change the cluster. The unfilled-template test names the placeholders that template's check reports. --- .../unittests/test_vllm_config_loader.py | 54 +++++++++++-------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/cvs/lib/inference/utils/unittests/test_vllm_config_loader.py b/cvs/lib/inference/utils/unittests/test_vllm_config_loader.py index 7767df052..5bafbb972 100644 --- a/cvs/lib/inference/utils/unittests/test_vllm_config_loader.py +++ b/cvs/lib/inference/utils/unittests/test_vllm_config_loader.py @@ -31,7 +31,7 @@ def test_every_template_loads_once_its_placeholders_are_filled(self): with self.subTest(config=template.name): load_variant(write_filled_config(template, self.tmp), {"username": "test"}) - def test_unfilled_template_lists_every_placeholder_in_one_error(self): + def test_unfilled_distributed_template_names_the_placeholders_it_checks(self): with self.assertRaises(ConfigError) as ctx: load_variant(DISTRIBUTED_TEMPLATE, {"username": "test"}) self.assertEqual( @@ -64,7 +64,7 @@ def test_placeholder_in_an_option_value_fails_the_load(self): class TestContainerForOrchestrator(unittest.TestCase): - def test_merged_container_has_only_keys_docker_reads(self): + def test_cluster_env_volumes_and_shm_size_merge_without_aliasing(self): with tempfile.TemporaryDirectory() as tmp: tmp = Path(tmp) (tmp / "threshold.json").write_text("{}") @@ -75,34 +75,42 @@ def test_merged_container_has_only_keys_docker_reads(self): "container": { "name": "vllm", "image": "rocm/vllm:1", - "env": {"NCCL_DEBUG": "INFO"}, - "runtime": {"name": "docker", "args": {"volumes": ["/models:/models:ro"]}}, + "env": {"B": "config"}, + "runtime": {"name": "docker", "args": {"volumes": ["/config:/config"]}}, }, "server_params": {"model": "/models/m", "tensor_parallel_size": 8}, "sweeps": {CELL: {}}, "runs": [CELL], } (tmp / "config.json").write_text(json.dumps(config)) - loaded = load_config(VariantConfig, tmp / "config.json", {}) - self.assertEqual( - loaded.container, - { - "lifetime": "per_run", - "name": "vllm", - "image": "rocm/vllm:1", - "env": {"NCCL_DEBUG": "INFO"}, - "runtime": { - "name": "docker", - "args": { - "network": "host", - "ipc": "host", - "privileged": True, - "volumes": ["/models:/models:ro"], - "devices": [], + cluster = { + "container": { + "name": "from-cluster", + "env": {"A": "cluster", "B": "cluster"}, + "runtime": { + "name": "docker", + "args": { + "volumes": ["/cluster:/cluster"], + "shm_size": "64g", + "registry": {"username": "user", "password_file": "/home/x/registry"}, + }, }, - }, - }, - ) + } + } + loaded = load_config(VariantConfig, tmp / "config.json", cluster) + self.assertEqual(loaded.container["name"], "vllm") + self.assertEqual(loaded.container["env"], {"A": "cluster", "B": "config"}) + self.assertEqual(loaded.container["runtime"]["args"]["volumes"], ["/config:/config"]) + self.assertEqual(loaded.container["runtime"]["args"]["shm_size"], "64g") + loaded.container["env"]["A"] = "mutated" + loaded.container["runtime"]["args"]["volumes"].append("/extra") + loaded.container["runtime"]["args"]["shm_size"] = "1g" + loaded.container["runtime"]["args"]["registry"]["username"] = "mutated" + cluster_container = loaded.cluster["container"] + self.assertEqual(cluster_container["env"]["A"], "cluster") + self.assertEqual(cluster_container["runtime"]["args"]["volumes"], ["/cluster:/cluster"]) + self.assertEqual(cluster_container["runtime"]["args"]["shm_size"], "64g") + self.assertEqual(cluster_container["runtime"]["args"]["registry"]["username"], "user") if __name__ == "__main__": From 190d298d21cbd335e09c26ef8cee2511ac03cd56 Mon Sep 17 00:00:00 2001 From: Atul Nair Date: Fri, 30 Oct 2026 14:53:32 -0700 Subject: [PATCH 3/3] refactor(vllm): move the vLLM schema to cvs/lib/conf_lib/schemas The suite calls load_config with the schema directly, so vllm_config_loader.py and load_variant are removed. --- cvs/lib/conf_lib/AGENTS.md | 4 +- cvs/lib/conf_lib/schemas/__init__.py | 4 + .../conf_lib/schemas/unittests/__init__.py | 4 + .../schemas/unittests/test_vllm.py} | 112 ++++++++++++++++- .../schemas/vllm.py} | 2 +- .../unittests/test_vllm_deck_profile.py | 5 +- cvs/lib/inference/unittests/test_vllm_job.py | 2 +- .../unittests/test_vllm_job_ray_backend.py | 4 +- .../unittests/test_vllm_job_server_reuse.py | 2 +- .../unittests/test_vllm_config_loader.py | 117 ------------------ cvs/lib/inference/utils/vllm_config_loader.py | 13 -- cvs/lib/inference/vllm_job.py | 2 +- cvs/tests/inference/vllm/_common.py | 5 +- cvs/tests/inference/vllm/conftest.py | 4 +- 14 files changed, 134 insertions(+), 146 deletions(-) create mode 100644 cvs/lib/conf_lib/schemas/__init__.py create mode 100644 cvs/lib/conf_lib/schemas/unittests/__init__.py rename cvs/lib/{inference/utils/unittests/test_vllm_schema.py => conf_lib/schemas/unittests/test_vllm.py} (67%) rename cvs/lib/{inference/utils/vllm_schema.py => conf_lib/schemas/vllm.py} (99%) delete mode 100644 cvs/lib/inference/utils/unittests/test_vllm_config_loader.py delete mode 100644 cvs/lib/inference/utils/vllm_config_loader.py diff --git a/cvs/lib/conf_lib/AGENTS.md b/cvs/lib/conf_lib/AGENTS.md index 83a4eed4c..feccc3597 100644 --- a/cvs/lib/conf_lib/AGENTS.md +++ b/cvs/lib/conf_lib/AGENTS.md @@ -1,6 +1,8 @@ # cvs/lib/conf_lib — config blocks and `load_config` -Suite schemas are built from `config_blocks.py` and loaded with `loader.load_config`. +Suite schemas are built from `config_blocks.py`, live in `schemas/` (one module per suite, such +as `schemas/vllm.py`), and are loaded with `loader.load_config`. A suite with a schema here calls +`load_config` directly; it has no loader module of its own. Substitution (`substitute_config`) and `BaseVariantConfig` stay in `cvs/lib/utils/config_loader.py`, which older suite loaders import. diff --git a/cvs/lib/conf_lib/schemas/__init__.py b/cvs/lib/conf_lib/schemas/__init__.py new file mode 100644 index 000000000..a73a51e90 --- /dev/null +++ b/cvs/lib/conf_lib/schemas/__init__.py @@ -0,0 +1,4 @@ +''' +Copyright 2026 Advanced Micro Devices, Inc. +All rights reserved. +''' diff --git a/cvs/lib/conf_lib/schemas/unittests/__init__.py b/cvs/lib/conf_lib/schemas/unittests/__init__.py new file mode 100644 index 000000000..a73a51e90 --- /dev/null +++ b/cvs/lib/conf_lib/schemas/unittests/__init__.py @@ -0,0 +1,4 @@ +''' +Copyright 2026 Advanced Micro Devices, Inc. +All rights reserved. +''' diff --git a/cvs/lib/inference/utils/unittests/test_vllm_schema.py b/cvs/lib/conf_lib/schemas/unittests/test_vllm.py similarity index 67% rename from cvs/lib/inference/utils/unittests/test_vllm_schema.py rename to cvs/lib/conf_lib/schemas/unittests/test_vllm.py index ddbc74500..826342731 100644 --- a/cvs/lib/inference/utils/unittests/test_vllm_schema.py +++ b/cvs/lib/conf_lib/schemas/unittests/test_vllm.py @@ -1,20 +1,27 @@ -'''Unit tests for the vLLM config rules in cvs.lib.inference.utils.vllm_schema.''' +'''Unit tests for the vLLM config schema in cvs.lib.conf_lib.schemas.vllm.''' +import json +import tempfile import unittest +from pathlib import Path from pydantic import ValidationError -from cvs.lib.inference.utils.vllm_schema import ( +from cvs.lib.conf_lib.loader import ConfigError, load_config +from cvs.lib.conf_lib.schemas.vllm import ( ContainerEnvTokenError, RunCell, VariantConfig, serialize_cli_options, ) -from cvs.unittests.support import FAKE_CRED +from cvs.unittests.support import FAKE_CRED, write_filled_config CELL = "ISL=1024,OSL=1024,TP=8,PP=2,CONC=16" FAKE_CRED_FILE = "/home/test/.hf_token" +CONFIG_DIR = Path(__file__).resolve().parents[4] / "input" / "config_file" / "inference" / "vllm" +DISTRIBUTED_TEMPLATE = CONFIG_DIR / "mi3xx_vllm_llama33-70b_fp8_distributed.json" +MERGE_CELL = "ISL=1024,OSL=1024,TP=8,PP=1,CONC=16" def _config(**overrides): @@ -249,5 +256,104 @@ def test_accuracy_thresholds_are_not_metric_cells(self): self._variant({CELL: cell, "accuracy": accuracy, "acuracy": {}}, enforce=True) +def _templates(): + return sorted(path for path in CONFIG_DIR.glob("*.json") if not path.name.endswith("threshold.json")) + + +class TestShippedTemplates(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.tmp = Path(self._tmp.name) + + def test_every_template_loads_once_its_placeholders_are_filled(self): + templates = _templates() + self.assertTrue(templates) + for template in templates: + with self.subTest(config=template.name): + load_config(VariantConfig, write_filled_config(template, self.tmp), {"username": "test"}) + + def test_unfilled_distributed_template_names_the_placeholders_it_checks(self): + with self.assertRaises(ConfigError) as ctx: + load_config(VariantConfig, DISTRIBUTED_TEMPLATE, {"username": "test"}) + self.assertEqual( + sorted(problem.split(":", 1)[0] for problem in ctx.exception.problems), + [ + "container.env.GLOO_SOCKET_IFNAME", + "container.env.NCCL_IB_GID_INDEX", + "container.env.NCCL_SOCKET_IFNAME", + "container.env.TP_SOCKET_IFNAME", + "container.image", + "container.runtime.args.volumes.1", + "container.runtime.args.volumes.3", + "server_params.model", + ], + ) + + def test_placeholder_in_an_option_value_fails_the_load(self): + filled = write_filled_config(DISTRIBUTED_TEMPLATE, self.tmp) + config = json.loads(filled.read_text()) + config["server_params"]["served_model_name"] = "" + (cell,) = config["runs"] + config["sweeps"][cell]["tokenizer"] = "" + filled.write_text(json.dumps(config)) + with self.assertRaises(ConfigError) as ctx: + load_config(VariantConfig, filled, {"username": "test"}) + self.assertEqual( + sorted(problem.split(":", 1)[0] for problem in ctx.exception.problems), + ["server_params.served_model_name", f"sweeps.{cell}.tokenizer"], + ) + + +class TestContainerForOrchestrator(unittest.TestCase): + def test_cluster_env_volumes_and_shm_size_merge_without_aliasing(self): + with tempfile.TemporaryDirectory() as tmp: + tmp = Path(tmp) + (tmp / "threshold.json").write_text("{}") + config = { + "enforce_thresholds": False, + "threshold_json": str(tmp / "threshold.json"), + "paths": {"shared_fs": "/home/x", "models_dir": "/models", "log_dir": "/logs", "hf_token_file": "/t"}, + "container": { + "name": "vllm", + "image": "rocm/vllm:1", + "env": {"B": "config"}, + "runtime": {"name": "docker", "args": {"volumes": ["/config:/config"]}}, + }, + "server_params": {"model": "/models/m", "tensor_parallel_size": 8}, + "sweeps": {MERGE_CELL: {}}, + "runs": [MERGE_CELL], + } + (tmp / "config.json").write_text(json.dumps(config)) + cluster = { + "container": { + "name": "from-cluster", + "env": {"A": "cluster", "B": "cluster"}, + "runtime": { + "name": "docker", + "args": { + "volumes": ["/cluster:/cluster"], + "shm_size": "64g", + "registry": {"username": "user", "password_file": "/home/x/registry"}, + }, + }, + } + } + loaded = load_config(VariantConfig, tmp / "config.json", cluster) + self.assertEqual(loaded.container["name"], "vllm") + self.assertEqual(loaded.container["env"], {"A": "cluster", "B": "config"}) + self.assertEqual(loaded.container["runtime"]["args"]["volumes"], ["/config:/config"]) + self.assertEqual(loaded.container["runtime"]["args"]["shm_size"], "64g") + loaded.container["env"]["A"] = "mutated" + loaded.container["runtime"]["args"]["volumes"].append("/extra") + loaded.container["runtime"]["args"]["shm_size"] = "1g" + loaded.container["runtime"]["args"]["registry"]["username"] = "mutated" + cluster_container = loaded.cluster["container"] + self.assertEqual(cluster_container["env"]["A"], "cluster") + self.assertEqual(cluster_container["runtime"]["args"]["volumes"], ["/cluster:/cluster"]) + self.assertEqual(cluster_container["runtime"]["args"]["shm_size"], "64g") + self.assertEqual(cluster_container["runtime"]["args"]["registry"]["username"], "user") + + if __name__ == "__main__": unittest.main() diff --git a/cvs/lib/inference/utils/vllm_schema.py b/cvs/lib/conf_lib/schemas/vllm.py similarity index 99% rename from cvs/lib/inference/utils/vllm_schema.py rename to cvs/lib/conf_lib/schemas/vllm.py index 16e84f5ec..47e8aec10 100644 --- a/cvs/lib/inference/utils/vllm_schema.py +++ b/cvs/lib/conf_lib/schemas/vllm.py @@ -11,7 +11,7 @@ from cvs.lib.inference.utils.accuracy_config import AccuracyConfig from cvs.lib.inference.utils.vllm_metrics import validate_threshold_spec -from cvs.lib.utils.config_blocks import ( +from cvs.lib.conf_lib.config_blocks import ( PLACEHOLDER_MESSAGE, ConfigStr, ContainerSpec, diff --git a/cvs/lib/inference/unittests/test_vllm_deck_profile.py b/cvs/lib/inference/unittests/test_vllm_deck_profile.py index 746a695a5..d6711ffe6 100644 --- a/cvs/lib/inference/unittests/test_vllm_deck_profile.py +++ b/cvs/lib/inference/unittests/test_vllm_deck_profile.py @@ -8,7 +8,8 @@ from pathlib import Path from cvs.lib.inference.vllm_topology import EffectiveVllmTopology -from cvs.lib.inference.utils.vllm_config_loader import load_variant +from cvs.lib.conf_lib.loader import load_config +from cvs.lib.conf_lib.schemas.vllm import VariantConfig from cvs.lib.inference.utils.vllm_metrics import ( CLIENT_METRICS, METRIC_CATEGORIES, @@ -127,7 +128,7 @@ def test_distributed_cell_uses_bound_threshold_key(self): config = write_filled_config( root / "input/config_file/inference/vllm/mi3xx_vllm_llama33-70b_fp8_distributed.json", tmp ) - variant = load_variant(config, {"username": "test"}) + variant = load_config(VariantConfig, config, {"username": "test"}).config variant.bind_effective_topology(EffectiveVllmTopology("distributed", ("node0", "node1"), 2)) cell_id = variant.expected_cells()[0] variant.enforce_thresholds = True diff --git a/cvs/lib/inference/unittests/test_vllm_job.py b/cvs/lib/inference/unittests/test_vllm_job.py index 308001308..d2445f2fe 100644 --- a/cvs/lib/inference/unittests/test_vllm_job.py +++ b/cvs/lib/inference/unittests/test_vllm_job.py @@ -16,7 +16,7 @@ from unittest import mock from cvs.lib.inference.unittests.fake_orch import FakeOrch -from cvs.lib.inference.utils.vllm_config_loader import VariantConfig +from cvs.lib.conf_lib.schemas.vllm import VariantConfig from cvs.lib.inference.vllm_job import VllmJob from cvs.unittests.support import FAKE_CRED, TMP, run_trusted diff --git a/cvs/lib/inference/unittests/test_vllm_job_ray_backend.py b/cvs/lib/inference/unittests/test_vllm_job_ray_backend.py index 89620ccf5..5c9b1016e 100644 --- a/cvs/lib/inference/unittests/test_vllm_job_ray_backend.py +++ b/cvs/lib/inference/unittests/test_vllm_job_ray_backend.py @@ -4,7 +4,7 @@ Unit tests for the Ray distributed-executor-backend support added to cvs.lib.inference.vllm_job.VllmJob and -cvs.lib.inference.utils.vllm_config_loader.VariantConfig. +cvs.lib.conf_lib.schemas.vllm.VariantConfig. These tests were originally derived from the Ray execution contract before implementation and retain coverage of its observable behavior. @@ -60,7 +60,7 @@ from pydantic import ValidationError from cvs.lib.inference.vllm_topology import EffectiveVllmTopology -from cvs.lib.inference.utils.vllm_config_loader import VariantConfig, serialize_cli_options +from cvs.lib.conf_lib.schemas.vllm import VariantConfig, serialize_cli_options from cvs.lib.inference.vllm_job import VllmJob RAY = {"distributed-executor-backend": "ray"} diff --git a/cvs/lib/inference/unittests/test_vllm_job_server_reuse.py b/cvs/lib/inference/unittests/test_vllm_job_server_reuse.py index 963617f85..888be3dcb 100644 --- a/cvs/lib/inference/unittests/test_vllm_job_server_reuse.py +++ b/cvs/lib/inference/unittests/test_vllm_job_server_reuse.py @@ -14,7 +14,7 @@ import json import unittest import unittest.mock as mock -from cvs.lib.inference.utils.vllm_config_loader import VariantConfig, serialize_cli_options +from cvs.lib.conf_lib.schemas.vllm import VariantConfig, serialize_cli_options from cvs.lib.inference.vllm_job import VllmJob from cvs.unittests.support import TMP diff --git a/cvs/lib/inference/utils/unittests/test_vllm_config_loader.py b/cvs/lib/inference/utils/unittests/test_vllm_config_loader.py deleted file mode 100644 index 5bafbb972..000000000 --- a/cvs/lib/inference/utils/unittests/test_vllm_config_loader.py +++ /dev/null @@ -1,117 +0,0 @@ -'''Unit tests for loading shipped and user vLLM configs through load_variant.''' - -import json -import tempfile -import unittest -from pathlib import Path - -from cvs.lib.inference.utils.vllm_config_loader import VariantConfig, load_variant -from cvs.lib.utils.config_loader import ConfigError, load_config -from cvs.unittests.support import write_filled_config - -CONFIG_DIR = Path(__file__).resolve().parents[4] / "input" / "config_file" / "inference" / "vllm" -DISTRIBUTED_TEMPLATE = CONFIG_DIR / "mi3xx_vllm_llama33-70b_fp8_distributed.json" -CELL = "ISL=1024,OSL=1024,TP=8,PP=1,CONC=16" - - -def _templates(): - return sorted(path for path in CONFIG_DIR.glob("*.json") if not path.name.endswith("threshold.json")) - - -class TestShippedTemplates(unittest.TestCase): - def setUp(self): - self._tmp = tempfile.TemporaryDirectory() - self.addCleanup(self._tmp.cleanup) - self.tmp = Path(self._tmp.name) - - def test_every_template_loads_once_its_placeholders_are_filled(self): - templates = _templates() - self.assertTrue(templates) - for template in templates: - with self.subTest(config=template.name): - load_variant(write_filled_config(template, self.tmp), {"username": "test"}) - - def test_unfilled_distributed_template_names_the_placeholders_it_checks(self): - with self.assertRaises(ConfigError) as ctx: - load_variant(DISTRIBUTED_TEMPLATE, {"username": "test"}) - self.assertEqual( - sorted(problem.split(":", 1)[0] for problem in ctx.exception.problems), - [ - "container.env.GLOO_SOCKET_IFNAME", - "container.env.NCCL_IB_GID_INDEX", - "container.env.NCCL_SOCKET_IFNAME", - "container.env.TP_SOCKET_IFNAME", - "container.image", - "container.runtime.args.volumes.1", - "container.runtime.args.volumes.3", - "server_params.model", - ], - ) - - def test_placeholder_in_an_option_value_fails_the_load(self): - filled = write_filled_config(DISTRIBUTED_TEMPLATE, self.tmp) - config = json.loads(filled.read_text()) - config["server_params"]["served_model_name"] = "" - (cell,) = config["runs"] - config["sweeps"][cell]["tokenizer"] = "" - filled.write_text(json.dumps(config)) - with self.assertRaises(ConfigError) as ctx: - load_variant(filled, {"username": "test"}) - self.assertEqual( - sorted(problem.split(":", 1)[0] for problem in ctx.exception.problems), - ["server_params.served_model_name", f"sweeps.{cell}.tokenizer"], - ) - - -class TestContainerForOrchestrator(unittest.TestCase): - def test_cluster_env_volumes_and_shm_size_merge_without_aliasing(self): - with tempfile.TemporaryDirectory() as tmp: - tmp = Path(tmp) - (tmp / "threshold.json").write_text("{}") - config = { - "enforce_thresholds": False, - "threshold_json": str(tmp / "threshold.json"), - "paths": {"shared_fs": "/home/x", "models_dir": "/models", "log_dir": "/logs", "hf_token_file": "/t"}, - "container": { - "name": "vllm", - "image": "rocm/vllm:1", - "env": {"B": "config"}, - "runtime": {"name": "docker", "args": {"volumes": ["/config:/config"]}}, - }, - "server_params": {"model": "/models/m", "tensor_parallel_size": 8}, - "sweeps": {CELL: {}}, - "runs": [CELL], - } - (tmp / "config.json").write_text(json.dumps(config)) - cluster = { - "container": { - "name": "from-cluster", - "env": {"A": "cluster", "B": "cluster"}, - "runtime": { - "name": "docker", - "args": { - "volumes": ["/cluster:/cluster"], - "shm_size": "64g", - "registry": {"username": "user", "password_file": "/home/x/registry"}, - }, - }, - } - } - loaded = load_config(VariantConfig, tmp / "config.json", cluster) - self.assertEqual(loaded.container["name"], "vllm") - self.assertEqual(loaded.container["env"], {"A": "cluster", "B": "config"}) - self.assertEqual(loaded.container["runtime"]["args"]["volumes"], ["/config:/config"]) - self.assertEqual(loaded.container["runtime"]["args"]["shm_size"], "64g") - loaded.container["env"]["A"] = "mutated" - loaded.container["runtime"]["args"]["volumes"].append("/extra") - loaded.container["runtime"]["args"]["shm_size"] = "1g" - loaded.container["runtime"]["args"]["registry"]["username"] = "mutated" - cluster_container = loaded.cluster["container"] - self.assertEqual(cluster_container["env"]["A"], "cluster") - self.assertEqual(cluster_container["runtime"]["args"]["volumes"], ["/cluster:/cluster"]) - self.assertEqual(cluster_container["runtime"]["args"]["shm_size"], "64g") - self.assertEqual(cluster_container["runtime"]["args"]["registry"]["username"], "user") - - -if __name__ == "__main__": - unittest.main() diff --git a/cvs/lib/inference/utils/vllm_config_loader.py b/cvs/lib/inference/utils/vllm_config_loader.py deleted file mode 100644 index 7c5aea9e1..000000000 --- a/cvs/lib/inference/utils/vllm_config_loader.py +++ /dev/null @@ -1,13 +0,0 @@ -'''Load a CVS vLLM config. The schema lives in ``vllm_schema``.''' - -from cvs.lib.inference.utils.vllm_schema import VariantConfig, serialize_cli_options # noqa: F401 - re-exported -from cvs.lib.utils.config_loader import load_config - - -def load_variant(config_path, cluster): - """Load a vLLM config and its threshold file against a cluster file path or dict. - - Raises ``ConfigError`` listing every invalid field, including each - ```` placeholder still in the config. - """ - return load_config(VariantConfig, config_path, cluster).config diff --git a/cvs/lib/inference/vllm_job.py b/cvs/lib/inference/vllm_job.py index dc9d822d9..37bc997a1 100644 --- a/cvs/lib/inference/vllm_job.py +++ b/cvs/lib/inference/vllm_job.py @@ -41,7 +41,7 @@ from typing import Optional from cvs.lib import globals -from cvs.lib.inference.utils.vllm_config_loader import serialize_cli_options +from cvs.lib.conf_lib.schemas.vllm import serialize_cli_options from cvs.lib.inference.utils.vllm_metrics import project_vllm_metrics from cvs.lib.utils.model_query_lib import OpenAIProbe diff --git a/cvs/tests/inference/vllm/_common.py b/cvs/tests/inference/vllm/_common.py index 7c43ae1c9..800f6b46b 100644 --- a/cvs/tests/inference/vllm/_common.py +++ b/cvs/tests/inference/vllm/_common.py @@ -8,7 +8,8 @@ import pytest from cvs.lib import globals -from cvs.lib.inference.utils.vllm_config_loader import load_variant +from cvs.lib.conf_lib.loader import load_config +from cvs.lib.conf_lib.schemas.vllm import VariantConfig from cvs.lib.inference.utils.vllm_metrics import ( METRIC_REGISTRY, VLLM_RESULTS_COLUMNS, @@ -42,7 +43,7 @@ def pytest_generate_tests(metafunc): config_file = metafunc.config.getoption("config_file") if not config_file: return - variant = load_variant(config_file, {}) + variant = load_config(VariantConfig, config_file, {}).config runs = variant.resolved_runs() ids = [run.cell.key for run in runs] if "accuracy_task" in metafunc.fixturenames: diff --git a/cvs/tests/inference/vllm/conftest.py b/cvs/tests/inference/vllm/conftest.py index 9d9b51fdf..e217717db 100644 --- a/cvs/tests/inference/vllm/conftest.py +++ b/cvs/tests/inference/vllm/conftest.py @@ -11,7 +11,8 @@ from cvs.lib import globals from cvs.lib.inference.vllm_topology import resolve_vllm_topology, scope_vllm_cluster from cvs.lib.inference.utils.vllm_metrics import VLLM_RESULTS_COLUMNS -from cvs.lib.inference.utils.vllm_schema import VariantConfig +from cvs.lib.conf_lib.loader import load_config +from cvs.lib.conf_lib.schemas.vllm import VariantConfig from cvs.lib.report.benchmark_metric_registry import ( benchmark_metric_columns_for_nodeid, benchmark_metric_rows_from_item, @@ -25,7 +26,6 @@ render_benchmark_metrics_html, ) from cvs.lib.report.subtest_reports import called_from_subtest_context, is_subtest_report -from cvs.lib.utils.config_loader import load_config from cvs.tests.inference.vllm._shared import validate_vllm_execution_mode log = globals.log