diff --git a/plans/cvs-config-api-refactor.md b/plans/cvs-config-api-refactor.md new file mode 100644 index 000000000..641f10f53 --- /dev/null +++ b/plans/cvs-config-api-refactor.md @@ -0,0 +1,880 @@ +# One config API for the training and inference suites + +**Scope:** vllm, atom, sglang, megatron, torchtitan, jaxmaxtext. +Health / rccl / ibperf / platform / mori come later. + +--- + +## The problem + +Six suites, six config loaders, six sets of conventions. + +``` +1,778 lines of loader code across the 6 suites +1,441 lines of conftest + + 6 fixtures — _deep_merge, cluster_dict, hf_token, orch, + variant_config, pytest_collection_modifyitems — + each defined separately in all 6 conftests +``` + +Each suite invented its own answer to the same questions: where do paths live, how is +a container declared, what is a sweep, what happens when the config is wrong. An +engineer who knows vLLM cold still cannot read an atom config without reading atom's +loader first. + +--- + +## The idea + +``` + BLOCK LIBRARY (framework owns) + ┌────────┬───────────┬────────────┬───────────────┬──────────────┐ + │ Paths │ ModelSpec │ Container │ ThresholdSpec │ Sweep × 3 │ + │ Model- │ │ Spec │ (7 kinds) │ Combo/Matrix/│ + │ Paths │ │ + Runtime │ │ Named │ + └───┬────┴─────┬─────┴──────┬─────┴───────┬───────┴──────┬───────┘ + └──────────┴────────────┴─────────────┴──────────────┘ + │ compose + ┌─────────────────┼─────────────────┐ + ▼ ▼ ▼ + vllm/schema.py atom/schema.py megatron/schema.py (suite owners) + │ │ │ + └─────────────────┴─────────────────┘ + │ register("vllm", model=…, rules=[…]) + ▼ + load_config(config, suite, cluster) + │ + ┌───────────┴───────────┐ + ▼ ▼ + typed config Problem[] + → tests run → no tests run, + every error at once, + each with a fix +``` + +**Blocks** — typed, reusable config fragments. A suite composes the ones it needs. +**Registry** — maps the suite name from `cvs run` to its schema. One load path. +**Rules** — the checks no single block can do: sweep-vs-thresholds, config-vs-cluster. + +--- + +## What a suite must implement + +Today this contract is implicit, and every suite guessed differently. Making it +explicit is most of the value. + +| | Required | Provided by framework | +|---|---|---| +| **Always** | a schema composing blocks | loading, substitution, parse, error format | +| **If it sweeps** | `sweep.entries()` + `cell_key(entry)` | `cells()`, coverage rule, parametrize IDs, report lookup | +| **If it containerizes** | a `container` block | orchestrator handoff | +| **If it gates** | a `thresholds` block | evaluation, record-only rendering | +| **Optional** | `rules` — cross-block and cluster checks | batching, reporting, the `fix` contract | + +Two methods. Everything else a suite writes today — `load_variant`, +`validate_sweep_selector`, `validate_thresholds_cover_sweep`, `_check_no_changeme`, +`expected_cells`, `orchestrator_container_from_variant` — moves into the framework. + +See **Appendix B** for exactly what each suite implements today and what survives. + +--- + +## The sweep problem, solved once + +> **Superseded in part.** This section preserves the existing sweep shapes behind a +> common `entries()` contract. "Departure: one `runs` block" below proposes replacing +> them instead. The inventory and drift analysis here still hold; the `SweepEntry` +> design is the fallback if the departure is rejected. + +### What "cell key" means + +Every sweep produces cells, and each cell's key is a string that must **exactly match +a key in the threshold file**. Nothing checks that the two agree in format — only that +they agree in content. + +Today that string is built three different ways, with three different signatures: + +```python +# vllm — formatted from params; PP segment appears only when pp > 1 +def cell_key(self, isl, osl, concurrency): + base = f"ISL={isl},OSL={osl},TP={self.params.tensor_parallelism}," + if int(self.params.pipeline_parallel_size) > 1: + base += f"PP={self.params.pipeline_parallel_size}," + return base + f"CONC={concurrency}" + +# atom — same idea, but the dimension set branches on the driver +def cell_key(self, isl, osl, concurrency): + key = f"ISL={isl},OSL={osl},TP={p.tensor_parallelism}" + if p.driver == "atom": + if nnodes > 1: + key += f",DP={nnodes},NNODES={nnodes}" + elif p.driver in ATOM_PP_DRIVERS: + if pp > 1 or nnodes > 1: + key += f",PP={p.pipeline_parallel_size}" + if nnodes > 1: + key += f",NNODES={p.nnodes}" + return f"{key},CONC={concurrency}" + +# megatron / torchtitan — different signature; the key is a declared combo id +def cell_key(self, combo_key: str) -> str: ... + +# sglang — has both of the above, plus a second key function taking nothing +def cell_key(self, isl, osl, concurrency): ... +def perf_cell_key(self): ... + +# jaxmaxtext — no method at all; the sweep entry's `name` IS the key +# "NNODES=2,STEPS=30,PRECISION=BF16,BATCH=3,GBS=48,SEQLEN=8192" +``` + +**Four distinct signatures** for one conceptual operation — `(isl, osl, concurrency)`, +`(combo_key)`, `()`, and no method at all. + +No shared code can call that. Which is why every suite reimplements coverage +checking, parametrize ID generation, and report cell lookup on top of it. + +### The fix: split enumeration from formatting + +```python +# On the sweep block — shape-specific, knows nothing about params +def entries(self) -> list[SweepEntry]: + """Enumerate the runs this sweep declares, in order.""" + +# On the config — one line, identical in every suite +def cells(self) -> list[str]: + return [self.cell_key(e) for e in self.sweep.entries()] +``` + +`entries()` is where the shapes differ. `cells()` is where they stop differing. + +`cell_key` stays suite-owned — atom's driver branching is real and cannot be +genericized — but it now has **one signature** across all six, so framework code can +call it. + +### What an entry carries + +The shapes share no dimensions at all — `isl/osl/concurrency` vs +`gbs/mbs/precision` vs arbitrary maxtext overrides — so `SweepEntry` cannot have +typed dimension fields. It has three: + +```python +@dataclass(frozen=True) +class SweepEntry: + ref: str # stable id — what the config called this run + dims: Dict[str, Any] # the parameters that vary across the sweep + payload: Dict[str, Any] # extras the job needs (SLOs, overrides, baselines) +``` + +`dims` is what `cell_key` formats. `payload` is what the job consumes. Nothing else. + +**vllm** — `sequence_combinations[{name, isl, osl, goodput_slo}]` + `runs[{combo, concurrency}]`: + +```python +def entries(self): + by_name = {c.name: c for c in self.sequence_combinations} + return [ + SweepEntry( + ref=f"{r.combo}@{r.concurrency}", + dims={"isl": by_name[r.combo].isl, "osl": by_name[r.combo].osl, + "concurrency": r.concurrency}, + payload={"goodput_slo": by_name[r.combo].goodput_slo}, + ) + for r in self.runs + ] +# → SweepEntry(ref="w1_isl=1000_osl=1000@16", +# dims={"isl": 1000, "osl": 1000, "concurrency": 16}, +# payload={"goodput_slo": {"ttft_ms": 1e9, ...}}) +``` + +**megatron / torchtitan** — `combinations{id: {...}}` + `runs[id]`: + +```python +def entries(self): + return [SweepEntry(ref=r, dims=self.combinations[r].dims(), + payload=self.combinations[r].payload()) + for r in self.runs] +# → SweepEntry(ref="llama3_3_70b-mi325-bs64-mbs1-fp8", +# dims={"global_batch_size": 64, "micro_batch_size": 1, "precision": "FP8"}, +# payload={"name": "llama3_3_70b_mbs1_gbs64_FP8", +# "result_dict": {"throughput_per_gpu": "100", ...}}) +``` + +**jaxmaxtext** — `training.sweeps[]` + `training.enabled_sweep_list[]`: + +```python +def entries(self): + chosen = set(self.enabled_sweep_list or [s.name for s in self.sweeps]) + return [SweepEntry(ref=s.name, dims=s.dims(), + payload={"maxtext_overrides": s.maxtext_overrides}) + for s in self.sweeps if s.name in chosen] +# → SweepEntry(ref="NNODES=2,STEPS=30,PRECISION=BF16,BATCH=3,GBS=48,SEQLEN=8192", +# dims={"nnodes": 2, "steps": 30, "precision": "BF16", +# "batch": 3, "gbs": 48, "seqlen": 8192}, +# payload={"maxtext_overrides": {"quantization": "", ...}}) +``` + +`cell_key` then reads `dims` and nothing else: + +```python +def cell_key(self, e): # vllm + d, p = e.dims, self.params + k = f"ISL={d['isl']},OSL={d['osl']},TP={p.tensor_parallelism}," + if p.pipeline_parallel_size > 1: + k += f"PP={p.pipeline_parallel_size}," + return k + f"CONC={d['concurrency']}" + +def cell_key(self, e): # jaxmaxtext + d = e.dims + return (f"NNODES={d['nnodes']},STEPS={d['steps']},PRECISION={d['precision']}," + f"BATCH={d['batch']},GBS={d['gbs']},SEQLEN={d['seqlen']}") +``` + +### The drift this closes + +jaxmaxtext is the case that shows why deriving the key matters. Today its dimensions +exist *only* inside the name string, which is also the threshold key. The config's own +comment states `GBS = per_device_batch_size * total GPUs`, and the arithmetic holds in +all three shipped configs: + +``` +NNODES=2, BATCH=3, GBS=48 → 3 × 2 × 8 = 48 ✓ +NNODES=1, BATCH=5, GBS=40 → 5 × 1 × 8 = 40 ✓ +NNODES=2, BATCH=2, GBS=32 → 2 × 2 × 8 = 32 ✓ +``` + +So the name is **derivable but hand-maintained**. Change `per_device_batch_size` to 4 +and forget to rename, and the key still resolves against the threshold file — because +the name *is* the key — while advertising a GBS that never ran. Nothing catches it. + +Declaring dims and deriving the name closes that. The migration does not require +rewriting configs first: keep the literal `name`, and have load assert derived == +declared, reporting a `Problem` on mismatch. That is the drift check today and the +deletion path tomorrow. + +### What that buys + +Everything downstream stops caring which sweep shape it got: + +| Consumer | Today | After | +|---|---|---| +| threshold coverage | reimplemented per suite | one rule over `cells()` | +| parametrize IDs | 4 variants, one of which can drift from the key | `cells()` | +| report cell lookup | per-suite key rebuild | `cells()` | + +And a new sweep shape costs one `entries()` method — nothing else changes. + +### Five shapes, one contract + +| Block | Config shape | Key origin | Suites | Files | +|---|---|---|---|---| +| `ComboSweep` | `sweep.sequence_combinations[]` + `sweep.runs[{combo, concurrency}]` | formatted from dims + params | vllm, atom | 42 | +| `MatrixSweep` | `sweep.combinations{id: …}` + `sweep.runs[id]` | formatted from dims — *not* the declared `name` | megatron, torchtitan | 12 | +| `NamedSweep` | `training.sweeps[]` + `training.enabled_sweep_list[]` | the entry's own `name` | jaxmaxtext | 3 | +| `BenchmarkSweep` | `{active_benchmark, benchmark_params}` — select-one | formatted from the selected params | sglang | 5 | +| *none* | nested `model_params` lookup, no sweep construct | n/a | jax, legacy megatron, legacy torchtitan | 13 | + +Note jax's sweep is nested under `training`, not at the top level like the others — +one more thing a newcomer has to discover by reading a loader. + +`MatrixSweep` carries **three** names per entry — the dict id, an inner `name`, and the +derived threshold key — and no two are equal. See the departure section below. + +sglang's `BenchmarkSweep` selects one benchmark rather than enumerating several, and it +derives cells *from* the threshold file, inverting the direction of truth. That's an +open decision, not a solved case. + +The 13 no-sweep configs cannot express a second run at all; they need a synthesized +single entry under any scheme. + +--- + +## Departure: one `runs` block + +The section above keeps the shapes and hides them behind a contract. The alternative is +to **not keep them**: one shape, declared names, every key function deleted. This is a +departure from how sweeps work in every suite today, so it is written out in full. + +### The inventory is five shapes, not three + +Counted across all shipped config files: + +| Shape | Config form | Key origin | Suites | Files | +|---|---|---|---|---| +| 1 | `sweep.{sequence_combinations[], runs[{combo, concurrency}]}` | derived — `cell_key(isl, osl, concurrency)` | vllm, atom | 42 | +| 2 | `sweep.{combinations{id: …}, runs[id]}` | derived — `cell_key(combo_key)` | megatron, torchtitan | 12 | +| 3 | `training.{sweeps[], enabled_sweep_list[]}` | **declared — `name` *is* the key** | jaxmaxtext | 3 | +| 4 | `{active_benchmark, benchmark_params}` | derived — `perf_cell_key(bp_dict)` | sglang | 5 | +| 5 | *none* — nested `model_params` lookup | n/a | jax (4), legacy megatron (3), legacy torchtitan (6) | 13 | + +Shape 4 reaches beyond the six suites in scope: `pytorch_xdit` uses it too, in 2 more +config files. Widening the scope adds shapes rather than reusing them. + +Two corrections to what this doc said earlier. Megatron and torchtitan **do** have a +sweep construct — shape 2 — in their current configs; it is only their legacy files +(3 megatron, 6 torchtitan) that fall into shape 5. And sglang's configs are shape 4, +not "no sweep block." + +Shape 5 matters: **13 config files cannot express a second run at all.** Those suites +are mid-migration already, which is the strongest argument that this is convergence +rather than imposition — four different teams reached for a sweep block and each +invented a different one. + +### The key functions + +**8 definitions across 6 modules in 4 signatures**, plus `expected_cells()` in 6: + +| Signature | Sites | +|---|---| +| `cell_key(self, isl, osl, concurrency)` | `atom_config_loader.py:147`, `vllm_config_loader.py:226`, `inferencing_config_loader.py:187`, `sglang_config_loader.py:326` | +| `cell_key(self, combo_key)` | `megatron/.../training_config_loader.py:147`, `torchtitan/training_config_loader.py:139` | +| `perf_cell_key(bp_dict)` | `sglang_config_loader.py:97` (module-level) | +| `perf_cell_key(self)` | `sglang_config_loader.py:331` | + +### Three names for one run + +A megatron combination declares an id, a `name`, and a threshold key — and no two are +equal: + +``` +dict key llama3_3_70b-mi325-bs64-mbs1-fp8 +name llama3_3_70b_mbs1_gbs64_FP8 +threshold key MBS=1,GBS=64,PRECISION=FP8 ← derived, appears nowhere in the file +``` + +To find the threshold for a run, a reader has to know which of the three the lookup +uses and then reconstruct it from a formatter in Python. + +### Declare-then-select is dead weight + +Three of the five shapes declare entries in one list and enable them in another. In the +shipped single-node DeepSeek vllm config, **3 combos are declared and 1 is run** — the +other two are inert text the reader must recognise as inert. jaxmaxtext's two lists are +character-for-character identical, so its selector selects everything. + +The two selector implementations also disagree on strictness: vllm's +`validate_sweep_selector` **raises** on an unknown reference, while jaxmaxtext's +`warnings.warn`s and silently runs a wider sweep than asked for. + +### The proposal + +```jsonc +"runs": [ + { "name": "ISL=1024,OSL=1024,TP=8,CONC=16", + "overrides": { "params": { "num_prompts": "640" } } } +] +``` + +Four changes, in increasing order of departure: + +1. **Derived keys become declared.** All 8 key functions and all 6 `expected_cells()` + delete. The key is in the file. +2. **Two levels collapse to one.** Selection moves to the CLI (`--runs a,b`). JSON has + no comments, so an in-file selector is the only way to disable an entry today — a + CLI flag removes the need. +3. **Typed axes become arbitrary overrides.** This is the capability, not a cleanup. + Today a vllm sweep can vary ISL, OSL and concurrency *and nothing else*; megatron can + vary GBS, MBS and precision *and nothing else*. Sweeping any other axis means editing + Python. Overrides let a run vary any field the schema has. +4. **Threshold home unifies.** Megatron's inline `result_dict` lifts out to the sibling + threshold file, where every other suite already keeps it. + +### What it costs + +**Denormalization.** vllm declares ISL/OSL once and references it from N runs; a flat +list repeats them. Mitigated by inheritance — the config body holds the base and each +run declares only its delta — but a wide sweep is more lines than today. + +**Names can lie.** `GBS=32` in a declared name is hand-written and unchecked. For +jaxmaxtext that is already true; for vllm and megatron it is a **regression** from a key +that is currently computed. This is the one place the departure is strictly worse, and +it is the open decision below. + +**13 files need a synthesized single run** to move off shape 5. + +### Migration is a script, not a re-calibration + +`cell_key()` is deterministic. Set each declared `name` to exactly the string +`cell_key()` emits today and **no threshold file changes at all** — 42 shape-1 configs +and 12 shape-2 configs convert mechanically, thresholds untouched. The departure can +land without a single re-measured number. + +### The cross-product question reopens + +This doc argued against an implicit `matrix:` cross product because generated names +drift against hand-written threshold keys. That objection dies once thresholds are +**emitted** from the run rather than typed: the name and the key come from one source +and cannot disagree. So the sequencing is — threshold emitter first, then `matrix:` +becomes safe sugar that expands to named `runs` at load. + +### Open decisions + +1. **Run identity.** Long descriptive names that can lie; short opaque ids plus a + rendered display string; or long names plus a validator recomputing the derivable + dimensions. Anything auto-resolved cannot appear in a name computed at collection + time, which pushes toward opaque. +2. **sglang.** It derives cells *from* the threshold file, inverting the direction of + truth. Converting it is a restructure of all 5 configs, not a rename. +3. **`matrix:` sugar** — after the emitter, or not at all. +4. **Overrides must be `extra="forbid"`** — a typo'd key otherwise yields a run that + looks swept and isn't. Merge must be deep; `_deep_merge` currently exists in six + AST-identical copies. + +--- + +## What changes, per suite + +| Suite | Today | After | +|---|---|---| +| **vllm** | standalone model — does not extend the shared base; private copies of `Paths`, `ModelSpec`, `ContainerConfig`; all 19 params typed `str`; drops unknown keys on load; 4 validators | composes shared blocks; `ComboSweep`; real types; unknown keys rejected | +| **atom** | extends the shared base, but imports `Sweep`, `validate_sweep_selector` and `validate_thresholds_cover_sweep` sideways out of **vllm's** module; 6 validators; own `expand_sweep`, `orchestrator_container_from_variant` | `ComboSweep` from the library; no cross-suite import; sweep expansion and orchestrator handoff move to framework | +| **sglang** | extends the base; two load paths (`_is_legacy_root` → legacy vs unified), but **all 5 shipped configs are legacy** — no `schema_version`, no `framework`, top-level `config`/`benchmark_params`; 526-line conftest; two key functions (`cell_key` + `perf_cell_key`); derives cells from thresholds; `orch` subsets cluster hosts | **the largest port by far** — 5 config files restructured, 138 loader lines deleted, and the only suite needing host-subsetting. Not a registration. | +| **megatron** | does **not** extend the base; **no `paths` block** — paths live in a `config` grab-bag with NCCL, topology and run flags; own `MatrixSweep`, `validate_sweep_selector`, `validate_thresholds_cover_sweep`, `_check_no_changeme`; unknown run refs warn and skip | extends base; 4 config files gain `paths`; `MatrixSweep` from library; those four functions become framework; unknown refs are errors | +| **torchtitan** | does **not** extend the base; same missing `paths` block; byte-identical copies of megatron's sweep block and all four functions — differs only in class names | same blocks and same framework code as megatron; 8 config files gain `paths`; the duplicate file shrinks to a schema | +| **jaxmaxtext** | extends the base; `NamedSweep` with name-as-key (already the right pattern); own `validate_thresholds_cover_training`; a bad enable-list entry silently widens the run | `NamedSweep` from library; coverage becomes the shared rule; enable-list mismatches are errors | + +### The shared fixtures are not equally shared + +"One `orch`, one `hf_token`" hides very different amounts of work. Measured by +comparing the six implementations directly: + +| Fixture | Distinct impls | Reality | +|---|---|---| +| `_deep_merge` | 6 copies, **1 body** | AST-identical in all six; only the docstrings drifted. Pure delete. | +| `cluster_dict` | 2 | Five identical; sglang differs. | +| `orch` | 6 files, **3 behaviours** | vllm / megatron / torchtitan / jax bodies are identical (docstring and log text only). **atom differs solely to inject `roles.server.env`** — the one-env-block change erases that. sglang is genuinely different. | +| `hf_token` | 4 | Diverges *semantically*, not cosmetically — see below. | + +So the dedup is cheaper than it looks for `_deep_merge` and `orch`, and more expensive +than it looks for `hf_token`, which is not a copy-paste problem but a behaviour +disagreement: + +```python +# vllm / sglang — a pre-staged model needs no token +if not os.path.isfile(path): + if variant_config.model.remote == 0: + return "" + pytest.skip(...) + +# atom — skips unconditionally +if not os.path.isfile(path): + pytest.skip(f"hf_token file missing: {path}") + +# megatron / torchtitan — different config path entirely +path = variant_config.config['hf_token_file'] +``` + +Identical cluster, identical pre-staged model: vllm **runs**, atom **skips**. Which +you get depends only on which suite you picked. Unifying the fixture forces that +question to be answered once — that is a decision, not a refactor. + +### Config files *do* change — for three suites, not one + +| Suite | Configs | What has to change | +|---|---|---| +| **sglang** | 5 | Full restructure: no `schema_version`, no `framework`, top-level `config` / `benchmark_params` | +| **megatron** | 4 | Gain a `paths` block; split the `config` grab-bag | +| **torchtitan** | 8 | Same | +| vllm, atom, jax | 45 | No change | + +**17 of the 62 config files across these six suites** — not 5. (The 57 "envelope" +configs counted elsewhere in this doc exclude sglang's 5 entirely, because they carry +no `schema_version`. That omission is itself the finding.) + +megatron and torchtitan have no `paths` block at all — +they keep paths inside a `config` catch-all that mixes four concerns: + +``` +paths hf_token_file, log_dir, data_cache_dir, rocm_dir, scripts_dir, megatron_root +network nccl_debug, nccl_socket_ifname, gloo_socket_ifname, nccl_ib_gid_index, + nccl_ib_hca, nccl_ib_hca_list, nic_type +topology nnodes, master_address +run training_iterations, verify_network_errors +``` + +That is why megatron's `hf_token` reads `variant_config.config['hf_token_file']` while +every inference suite reads `variant_config.paths.hf_token_file`. + +The payoff is concentrated in sglang. **138 of its 453 loader lines (30%)** exist only +to reverse-engineer a missing `paths` block — `_infer_models_dir` recovers `models_dir` +from `container_config.volume_dict`, `_infer_shared_fs` recovers `shared_fs` by +string-slicing `log_dir`, plus `_legacy_server_env`, `legacy_container_block_from_inference`, +`legacy_paths_from_inference`, `_is_legacy_root`, `_load_legacy_variant`. Give sglang a +real `paths` block and all 138 lines delete. + +### What genuinely does not change + +- **Test bodies.** `variant_config` still arrives as a typed object. Raw dict access in + test and job code is already near zero — 0–2 sites per suite — so the port does not + reach into job internals. +- **Orchestrator and report.** `container.model_dump()` keeps its current contract. +- **45 of the 62 config files** — every vllm, atom, and jaxmaxtext config. + +### One capability the framework must decide on + +sglang's `orch` rewrites the cluster dict to **scope the orchestrator to a subset of +hosts**, branching three ways (single / distributed / disaggregated) on which suite was +selected. No other suite subsets hosts. Either the framework supports host-subsetting +as a first-class concept or sglang keeps a suite-owned `orch`. That is the one item on +this list that is an architecture question rather than a port. + +--- + +## What a suite author writes + +```python +# cvs/lib/inference/atom/schema.py + +class AtomConfig(BaseVariantConfig): # or compose blocks directly + framework: Literal["atom"] + params: AtomParams # the only suite-specific part + sweep: ComboSweep + + def cell_key(self, entry) -> str: # suite-owned formatter + ... + +def check_scaling_baseline(cfg, cluster) -> list[Problem]: + ... + +register("atom", model=AtomConfig, rules=[check_scaling_baseline]) +``` + +Then the conftest calls `load_config()` instead of `load_variant()`. That is the port. + +--- + +## What the user sees when it's wrong + +Today: one error, raised at the first problem, sometimes mid-run. + +After: + +``` +config error: mi325x_vllm_deepseek-r1.json + + sweep.runs[0].combo + "w_deepseek_1k1k" does not name any entry in sweep.sequence_combinations + fix: use one of: w_deepseek-r1-0528_fp8_1k1k, ..._1k8k, ..._8k1k + + enforce_thresholds + enforce_thresholds is true but no threshold file was resolved + fix: set "enforce_thresholds": false to characterize first, then set + "threshold_json" once you have numbers + +2 problems; no tests were run. +``` + +Every problem at once, before anything launches, each naming the next action. The +`fix` line is mandatory on every rule — if you can't say what the user should do, the +check isn't ready. + +--- + +## Threshold ergonomics + +Two questions, answered separately: + +| `enforce_thresholds` | threshold file | Behaviour | +|---|---|---| +| `false` | absent | record-only, **no error** — characterizing a new shape | +| `false` | present | report shows spec, bar, margin; no verdict | +| `true` | present | gates the run | +| `true` | absent | error at load | + +Flip one boolean to move between gated and ungated, file left in place. Today a +threshold file is mandatory even when nothing is being gated. + +--- + +## Why this is cheap + +Most of it already exists in `cvs/lib/utils/config_loader.py` — *"framework-agnostic +config machinery shared by every CVS suite."* + +- **6 of 6** already call its `substitute_config` +- **3 of 6** already extend its `BaseVariantConfig` (atom, sglang, jaxmaxtext) +- `Paths`, `ModelSpec`, `ContainerSpec`, `RuntimeSpec` already live there + +The genuinely new code is the registry (~30 lines), the `Problem` type, the `entries()` +contract, and the threshold-optional change. The rest is moving blocks that exist into +a library, and deleting the copies. + +--- + +## Phasing + +The six do not cost the same, so they do not go in one bucket. + +| | | Config files touched | +|---|---|---| +| **1. Framework** | blocks + registry + `Problem` + 3 shared rules. No suite changes. | 0 | +| **2. Two pilots** | **atom** — schema only, proves blocks + registry. **megatron** — proves the config migration, since it needs a `paths` block. One of each kind. | 4 | +| **3. Follow the pilots** | **vllm** and **jax** follow atom (schema move, no config change). **torchtitan** replays megatron's migration. | 8 | +| **4. sglang** | Its own project: restructure 5 configs, delete the 138-line legacy adapter, and settle host-subsetting. | 5 | +| **5. Everything else** | health / rccl / ibperf / platform / mori — scoped separately, later. | — | + +Phase 2 is deliberately one of each kind so both paths are proven before Phase 3 +replays them. sglang is last because it is the only suite whose port is also an +architecture decision. + +Unregistered suites keep their current loader throughout. Nothing is deleted until +its replacement is proven. + +--- + +## The ask + +1. Agreement that blocks + registry + rules is the right shape. +2. A suite owner each for the two Phase 2 ports. +3. Two decisions we can't make alone: + - **Cell keys.** Adopting `cells()` as the single producer means either + regenerating existing threshold files or keeping per-suite formatters for + compatibility. + - **Threshold generation.** Is there anything that produces a threshold file from + a record-only run? Without it, "flip to enforce" means hand-authoring the file. + +--- +--- + +# Appendix A — Block reference + +Every block, what it holds, and who declares it. + +### `Paths` / `ModelPaths` + +```python +class Paths(_Forbid): + shared_fs: str = Field(min_length=1) + log_dir: str = Field(min_length=1) + +class ModelPaths(Paths): + models_dir: str = Field(min_length=1) + hf_token_file: str = Field(min_length=1) +``` + +Split by whether the suite pulls models. All six training/inference suites use +`ModelPaths`; the later suites use `Paths`. + +`min_length=1` is load-bearing — a present-but-empty path currently satisfies `str` +and fails later, mid-run. + +Participates in three-pass substitution: `{user-id}` from the cluster file, then +self-reference (`{shared_fs}`), then cross-block (`{paths.log_dir}`). + +### `ModelSpec` + +```python +class ModelSpec(_Forbid): + id: str = Field(min_length=1) # HF repo id or absolute local path + remote: Literal[0, 1] = 0 + precision: str = "" +``` + +Owns the `remote=1 not implemented` guard. The guard lives here — not on the enclosing +config — so a suite composing `ModelSpec` without the shared envelope still gets it. + +### `ContainerSpec` / `RuntimeSpec` / `RuntimeArgs` + +```python +class RuntimeArgs(_Allow): # allow: docker/podman own this vocabulary + volumes: List[str] = [] + devices: List[str] = [] + env: Dict[str, str] = {} + network: str = "" + ipc: str = "" + shm_size: str = "" + privileged: bool = False + +class RuntimeSpec(_Forbid): + name: Literal["docker", "podman"] = "docker" + args: RuntimeArgs + +class ContainerSpec(_Forbid): + lifetime: Literal["no_launch", "per_run", "persistent"] = "per_run" + name: str = Field(min_length=1) + image: str = Field(min_length=1) + runtime: RuntimeSpec +``` + +`ContainerSpec` is closed — four members. `RuntimeArgs` is open because runtime flags +are the runtime's vocabulary, but the members CVS reads are declared so they're typed +and discoverable. + +`container.model_dump()` must keep producing what `OrchestratorConfig` consumes. + +### `env` — one block + +```python +env: Dict[str, str] = {} # top level, applied to the container + # and exported into the job process +``` + +Environment variables are set once for a run. One block, at the top level — which is +what the code already does, as below. + +Today there are four places they live, plus a fifth that is declared and unused: + +| Location | Configs | | +|---|---|---| +| `roles.server.env` | 42 | vllm, atom | +| `training.env_vars` | 3 | jaxmaxtext | +| hardcoded in `megatron_lib.py:540-560` | — | `TORCH_NCCL_ASYNC_ERROR_HANDLING`, `NCCL_IB_*` | +| hardcoded in `torchtitan_lib.py:389-393` | — | `HSA_FORCE_FINE_GRAIN_PCIE`, `PYTORCH_HIP_ALLOC_CONF` | +| `container.runtime.args.env` | **0** | schema supports it; nothing sets it | + +The per-role scoping the current schema allows is not merely unused — the code +actively collapses it. vllm writes one env script and has **both** processes source it: + +```python +# vllm_job.py:319 — one script, written once +self.orch.exec("bash -c " + shlex.quote(f"printf '%s' ... > /tmp/server_env_script.sh")) + +# vllm_job.py:363 — the server sources it +inner = f"source /tmp/server_env_script.sh && nohup {serve_cmd} ..." + +# vllm_job.py:591 — and so does the benchmark client +client_cmd = f"source /tmp/server_env_script.sh && {bench_cmd} ..." +``` + +So `roles.server.env` already reaches the client. The name says otherwise, which is +exactly the kind of thing a newcomer has to read the job code to discover. + +The rest of the evidence agrees: + +- `roles` is exactly `{server}` in **42 of 42** configs that have it. +- The one config with genuine prefill/decode roles + (`mi30x_sglang_deepseek_r1_0528_disaggregated.json`) splits node lists, ports, and + policies — but not env. + +The two hardcoded sets matter most for a newcomer. There is no config field for them, +so tuning NCCL on megatron means finding `megatron_lib.py:551` and editing library +code. Folding them into `env` with defaults makes them visible and overridable. + +If disaggregated serving later needs a split, `roles..env` merging **over** the +base block is purely additive — existing configs keep working, no migration. Of the 8 +env keys in the corpus most are cluster-wide (`HF_HUB_OFFLINE`, `TRANSFORMERS_OFFLINE`, +`GPU_ARCHS`), but `VLLM_ROCM_USE_AITER` and `AMDGCN_USE_BUFFER_OPS` are kernel-selection +flags that could plausibly differ between a compute-bound prefill node and a +memory-bound decode node. That is the trigger to build it — base plus override, not two +parallel blocks as today. Deferring until then costs nothing. + +Redaction is framework-owned against one declared secret-key set, and applies wherever +the block is rendered. Suites do not write their own: a per-suite regex is a per-suite +chance to get the escaping wrong, and the failure is silent. + +### `ThresholdSpec` + +A tagged union on `kind`. Seven kinds, and they do not share a field set. + +| kind | extra fields | meaning | +|---|---|---| +| `info` | — | records, never gates | +| `min` | `value` | lower bound | +| `max` | `value` | upper bound, unit-agnostic (counts) | +| `max_ms` | `value` | upper bound, milliseconds | +| `min_tok_s` | `value` | lower bound, tokens/sec | +| `within` | `value`, `tolerance_pct` | band around a target | +| `min_ratio` | `value`, `reference` | ratio against another metric | + +```python +ThresholdSpec = Annotated[Union[...], Field(discriminator="kind")] +Thresholds = Dict[str, Dict[str, ThresholdSpec]] # cell -> metric -> spec +``` + +A union is right here and wrong at the top level: this set is closed and owned by one +evaluator that already switches on exactly these seven strings. Suite schemas are +open-ended, which is why the registry keys on a plain string instead. + +A misspelled `kind` currently reaches the evaluator and is reported per-metric at run +time. Here it fails at load. + +### The sweep blocks + +Each implements `entries() -> list[SweepEntry]`. See the sweep section above for the +entry shape and the implementations. sglang's select-one `BenchmarkSweep` and the 13 +no-sweep configs are not modelled here — both are open decisions. + +```python +class ComboSweep(_Forbid): # vllm, atom + sequence_combinations: List[SeqCombo] = Field(min_length=1) + runs: List[Run] = Field(min_length=1) + +class MatrixSweep(_Forbid): # megatron, torchtitan + combinations: Dict[str, MatrixCombo] = Field(min_length=1) + runs: List[str] = Field(min_length=1) + +class NamedSweep(_Forbid): # jaxmaxtext — lives under `training`, not top level + sweeps: List[NamedEntry] = Field(min_length=1) + enabled_sweep_list: List[str] = [] # empty = all +``` + +Two rules apply to all of them: + +- An unresolvable reference in `runs` / `enabled_sweep_list` is an **error**. A sweep + that drops a bad reference reports green for a run that never happened — megatron + warns and skips today, and jax silently widens the run. +- A selector matching nothing is an error, not an empty sweep. + +### `BaseVariantConfig` — preset assembly + +```python +class BaseVariantConfig(_Forbid): + schema_version: Literal[1] + framework: str + gpu_arch: str + enforce_thresholds: bool = True + threshold_json: str = "" + paths: ModelPaths + model: ModelSpec + container: ContainerSpec + thresholds: Thresholds = {} +``` + +A convenience, not a requirement. It carries **no validation of its own** — every +guard lives on the block it belongs to. A suite needing a different set composes +blocks directly and loads through the same registry entry. + +The membership is not a guess. Across the 57 envelope configs on disk: + +``` +57/57 schema_version, framework, gpu_arch, enforce_thresholds, + threshold_json, container → the shared envelope +54/57 sweep → all but the no-sweep suite +45/57 paths, model → ModelPaths users +42/57 roles, params → suite-specific +``` + +`framework` and `gpu_arch` are in every config and declared separately in every +suite schema. Six declarations of a universal field is exactly the duplication this +removes. + +--- +--- + +# Appendix B — Per-suite implementation inventory + +What each suite writes today. ✓ = own implementation, ↗ = imported from another +suite, — = not present. + +| | vllm | atom | sglang | megatron | torchtitan | jax | +|---|---|---|---|---|---|---| +| extends `BaseVariantConfig` | — | ✓ | ✓ | — | — | ✓ | +| `load_variant` | ✓ | ✓ | ✓ | ✓ | ✓ | ✓ | +| `cell_key` | ✓ | ✓ | ✓ ×2 | ✓ | ✓ | — | +| `cell_key` signature | isl,osl,conc | isl,osl,conc | isl,osl,conc | combo_key | combo_key | n/a | +| `expected_cells` | ✓ | ✓ | — | ✓ | ✓ | ✓ | +| `validate_sweep_selector` | ✓ | ↗ vllm | — | ✓ | ✓ | — | +| `validate_thresholds_cover_*` | ✓ | ↗ vllm | — | ✓ | ✓ | ✓ | +| `_check_no_changeme` | — | — | — | ✓ | ✓ | ✓ | +| `expand_sweep` | — | ✓ | — | — | — | — | +| `orchestrator_container_from_variant` | — | ✓ | ✓ | — | — | — | +| legacy load path | — | — | ✓ | — | — | — | +| `@model_validator` | 4 | 5 | 1 | 2 | 2 | 0 | +| `@field_validator` | 1 | 1 | 0 | 0 | 0 | 1 | +| loader lines | 275 | 358 | 453 | 218 | 210 | 264 | +| conftest lines | 180 | 149 | 526 | 187 | 164 | 235 | + +**After:** every row above except `cell_key`, the suite schema, and suite-specific +rules moves into the framework. `cell_key` stays, with one signature. + +The megatron / torchtitan columns are identical because the files are — their sweep +blocks and all four helper functions differ only in class names. diff --git a/plans/cvs-config-generation.md b/plans/cvs-config-generation.md new file mode 100644 index 000000000..e850835aa --- /dev/null +++ b/plans/cvs-config-generation.md @@ -0,0 +1,285 @@ +# CVS config generation + +Companion to [`cvs-config-api-refactor.md`](cvs-config-api-refactor.md). That doc +defines the config *schema* and the `auto` resolution phase. This one is about getting a +config file to exist in the first place. + +**It is downstream of the refactor and cannot be read as freestanding.** Generation +emits whatever shape the schema defines; if the schema's sweep departure lands after a +generator ships, every generated file is immediately legacy. Sequencing is treated +explicitly at the end. + +Scope is the same six suites: vllm, atom, sglang, megatron, torchtitan, jaxmaxtext. + +--- + +## The problem this solves + +`cvs copy-config` is generation today, and it is `shutil.copyfile` +(`copy_config_plugin.py:129`). It hands the user a template with `` still in +it. Across `cvs/input/config_file/` that is **75 files carrying 354 occurrences**, out +of 145 configs total. Authoring the rest by hand is the friction. + +The asymmetry is the tell: + +| Half | State | +|---|---| +| **Cluster file** | *generated.* `cvs generate cluster_json` takes `--hosts` (or a hosts file), expands `192.168.1.10-20` and `host[1-10]`, renders a Jinja template. | +| **Config file** | *copied.* Byte-for-byte. Placeholders survive. | + +So CVS already believes in generation — it just stopped at the easier of the two files. + +**Generation is not resolution.** Resolution (`"auto"`, in the companion doc) fills +values in memory at run time: zero files, always current, nothing for the user to see or +tune. Generation produces an artifact. Both consume the same probes, so the probe layer +gets built once with two consumers — but they are not substitutes, because a config the +user cannot see is a config the user cannot tune. + +### The extension point already exists + +`GeneratorPlugin` (`cli_plugins/generate_plugin.py:43-92`) is an ABC with dynamic +discovery over `cvs/input/generate/` and `cvs/reports/generate/`. Adding +`cvs generate config` is **dropping one file into a directory** — no CLI plumbing, no +registry edit. Jinja2 is already a dependency and templates already have a home at +`cvs/input/templates//`. + +The probes exist too, all currently unwired for this purpose: + +| Probe | Where | Used today for | +|---|---|---| +| HCA names + socket netdev, with cross-node asymmetry detection | `lib/utils/ib_discovery.py` | `auto` in atom/vllm fabric resolution | +| total / used / free VRAM | `parse_mem_usage`, `lib/utils/gpu.py` | captured, marked *"Not used as test rows"* | +| model size on disk | `_du_bytes`, `tests/inference/vllm/vllm.py:142` | download-progress poll, not a fit check | +| GPU architecture | `get_model_from_rocm_smi_output` | a torchtitan fixture | + +Nothing here needs inventing. It needs wiring and a front door. + +--- + +## Case study: vllm llama3.1-70B + +vllm ships exactly two configs for this model — `mi300x_vllm_llama31-70b_fp8_single.json` +(83 lines) and `..._distributed.json` (87). A field-level diff of the two returns **nine +differences, and not one of them is knowledge**: + +| Field | single → distributed | What it actually is | +|---|---|---| +| `params.nnodes` | `"1"` → `"2"` | `len(orch.hosts)` | +| `params.master_addr` | absent → `` | `orch.hosts[0]` | +| `params.pipeline_parallel_size` | `"1"` → `"2"` | `= nnodes` on the mp backend | +| `roles.server.ib_netdev` | absent → `` | probe — `ip -4 -o addr show` on the host's own IP | +| `roles.server.ib_hca_devices` | absent → `"auto"` | **already resolved this way** | +| `params.master_port` | absent → `"29501"` | static | +| `container.name` | `w1_…` → `w2_…` | cosmetic | +| `sweep.sequence_combinations` | `w1_isl=1000_osl=1000` → `w2_…` | **name prefix only** | +| `sweep.runs` | `combo: "w1_…"` → `"w2_…"` | same prefix; concurrency 16 in both | + +The last two are worth dwelling on. The sweep entries are otherwise identical — same +`isl`, same `osl`, same `goodput_slo`, same concurrency. And `cell_key` +(`lib/inference/utils/vllm_config_loader.py:226-238`) formats the threshold key from +`isl`, `osl`, `tp`, `pp` and `concurrency`: **the combo `name` never enters the key.** +So the `w1_`/`w2_` prefix changes no behavior, no lookup, and no result. It is a +distinction that distinguishes nothing. + +Strip the cosmetics and an entire 87-line file exists to express **five derivable facts +and one port number.** + +### It is not a vllm quirk + +Counting files whose names differ only by a topology suffix (`_single`, +`_distributed`, `_disaggregated`, `_multinode`): + +``` +2 inference/atom/mi300x_atom_deepseek-r1_fp8_.json +2 inference/atom/mi355x_atom_deepseek-r1_fp8_.json +3 inference/sglang/mi30x_sglang_deepseek_r1_0528_.json +2 inference/vllm/mi300x_vllm_llama31-70b_fp8_.json +2 training/jax/mi300x_jax_llama3_1_70b_.json +2 training/jaxmaxtext/mi300x_jaxmaxtext_llama-3.3-70b_.json +2 training/megatron/mi325x_megatron_llama-3.3-70b_.json +2 training/megatron/mi3xx_megatron_llama_.json +2 training/torchtitan/mi3xx_torchtitan_deepseek_.json +2 training/torchtitan/mi3xx_torchtitan_llama_.json +2 training/torchtitan/mi3xx_torchtitan_qwen3_.json +``` + +**23 files in 11 topology sets, spanning all six suites plus jax. 12 of them are the +redundant members.** Every suite independently decided topology is a filename axis. + +### And the easy case isn't easy either + +The *single-node* file still carries two ``: `threshold_json` and +`container.image`. Neither is cluster-specific. Both are knowable. So even the +zero-multinode path is not a zero-edit path today. + +--- + +## Possible solutions + +The design question is **what generation reads from**. + +| | Source of truth | Trade | +|---|---|---| +| **S1** | Clone-and-retarget the nearest shipped config | Inherits all measured tuning for free. Also inherits its bugs and its shape, and drifts as the configs drift. | +| **S2** | A thin per-(model, arch) recipe behind a template | Clean, queryable, one obvious home for a new model. A second corpus to maintain — and on day one it is a transcription of S1. | +| **S3** | Synthesize from schema defaults | Nothing to maintain. Produces a config nobody tuned: `num_prompts: 320` is not a default, it is somebody's measured result. | +| **S4** | No file at all — resolve `(model, cluster)` at run time | Zero artifacts to rot. Kills the tuning path, which is the stated requirement. | + +**Recommendation: S1 now, S2 as the migration target.** S1 ships against the corpus that +already exists and is honest about where its numbers came from. S2 is what S1 becomes +once generation has revealed which fields actually vary across models — that information +does not exist yet, and guessing it now is how the wrong recipe format gets locked in. +S3 and S4 are coherent positions, but each surrenders something already asked for. + +### Independent of S1–S4: topology stops being a file axis + +One config; `nnodes` from the cluster file; `pp` derived. The 12 redundant files above +stop needing to exist. This is the largest concrete win in the generation story and it +does not depend on which source-of-truth option wins. + +--- + +## The nine ideas + +### G1 — Clone-and-retarget, not synthesize + +The 145 shipped configs are accumulated tuning knowledge. `num_prompts: 320`, +`client_poll_count: 90`, the `serve_args` — none are schema defaults; they are per-model +results someone measured. Generation picks the nearest shipped config as a base and +re-targets it. Pydantic can dump structure; it cannot dump knowledge. + +Falls out for free: the missing-architecture problem. There is no MI325X vllm config +anywhere — but generating from the MI300X one and swapping the threshold reference +produces a usable starting point instead of nothing. + +### G2 — Invert the front door: cluster-first + +Today the flow is *pick a config, hope it fits the cluster*. Flip it: + +``` +cvs generate config vllm --cluster cluster.json + → probes: 3 nodes × 8 GPUs, 192 GB/GPU, models staged in /shared/models + → offers the (model, topology) pairs that fit + → writes the one you pick +``` + +Same fit arithmetic as the preflight gate, run as a **generator** rather than a +validator. Minimum input stays node IPs, plus the two questions nothing can answer for +the user. + +### G3 — Freeze the tuning surface, `auto` the hardware + +The split rule for what gets written as a literal versus left as `auto`: + +| | Treatment | Why | +|---|---|---| +| `num_prompts`, concurrency, `max-model-len`, sweep shape | **frozen literal** | this *is* the tuning surface; invisible is useless | +| HCAs, netdev, `rocm_dir`, `gpu_arch` | **`auto`** | only the hardware knows, and it changes if they re-cable | +| `nnodes`, `master_addr` | **omitted entirely** | restatements of the cluster file; keeping them creates a divergence class | + +The failure mode to avoid is freezing everything — a fully concrete config generated on +cluster A silently misdescribes cluster B. + +### G4 — Generate the pair, in record mode + +Emit the config **and** its threshold file together, zeros throughout, +`enforce_thresholds: false`. That is exactly the intended lifecycle: the first run is +meant to be an easy pass that records. + +### G5 — Every generated value carries its provenance + +"Why is `num_prompts` 320?" should be answerable from the file. The sources are a small +closed set: `probe`, `cluster-file`, `recipe:`, `default`, `you`. CVS already has +the `_comment_X` convention to carry it. + +Generation also gets to **fix an existing problem**: the `_example_` keys in the +megatron and jax configs ship real foreign-cluster values (`bnxt_re0-7`, `ens51f1np1`, +`rocep28s0`). AGENTS.md forbids shipping cluster-specific values precisely because users +copy them. A generator replaces those with what was discovered on *their* cluster. + +### G6 — Stamp it, diff it, never clobber it + +CSPs will commit these files. Stamp the source recipe and its version; `--diff` shows +what moved upstream since; refuse to overwrite a file the user has edited. +Generated-then-owned is the normal lifecycle, not an edge case. + +### G7 — Write outside the package + +The harness runs from a `site-packages` copy, not the git source. Generated configs must +land in a user-owned path — writing into `cvs/input/config_file/` puts them where the +next `pip install` erases them. + +### G8 — Starter sweeps, not a blank `runs` block + +Offer *smoke* / *qualification* / *full envelope* and populate `runs` accordingly. This +is where "set up sweeps across any axis" actually gets delivered: most users want a good +default sweep, and the ones who don't will edit it — which is the point of writing a +file rather than resolving in memory. + +### G9 — The round-trip test is the honesty gate + +`generate → load → validate` must pass with **zero edits**, for every (suite, model) the +catalog claims to support. One test in CI, and generation cannot rot as schemas move. + +It is also the only mechanical acceptance test for "a user with only node IPs can run a +suite." Without it, that goal is an aspiration that cannot fail. + +--- + +## Where this fits + +Generation owns the head of the pipe. The refactor owns everything from `load` onward. +They meet at exactly one contract — the schema. + +``` +cvs generate cluster_json ← exists today + → cvs generate config ← this doc (G1/G2) + → load + validate ← refactor doc: the schema + → resolve auto ← refactor doc: the new I/O phase + → materialize the resolved config into the run dir ← refactor doc + → run, records results + → cvs generate thresholds ← the missing emitter (G4) + → enforce +``` + +Two consequences worth stating plainly. + +**Generation is the refactor's acceptance test.** G9 is what proves the schema is +actually usable from a standing start, rather than merely well typed. + +**Sequencing is a real risk.** The refactor proposes replacing five sweep shapes with one +flat `runs` block. A generator shipped before that lands writes files in the old shape, +and the migration then has two producers to fix instead of one. Either generation +targets the post-departure shape from day one, or it ships after the departure. +Preference: **target the post-departure shape**, and let the generator be the first +consumer that proves it works. + +--- + +## What generation cannot do + +Three things stay the user's, and the design should stop pretending otherwise: + +1. **Which model.** +2. **What they are proving** — smoke, qualification, or full envelope. +3. **Their SLOs.** + +Two questions plus node IPs is the floor. Everything else is derivable, probeable, or +already shipped. + +--- + +## Open questions + +1. **Recipe format** — S1 or S2, and if S2, when. The answer probably depends on data + that only running S1 produces. +2. **Interactive or flags-only?** A wizard fits the day-0 target; flags fit CI. Likely + both, wizard when a TTY is present. +3. **Does generation require cluster access?** G2's cluster-first flow does. Offline + generation (`--arch mi355x --nodes 2`) is weaker but works from a laptop. Supporting + both means offline emits more `auto`. +4. **Who curates recipes** as models are added — the same ownership question as canonical + model naming in the companion doc. +5. **Does `cvs copy-config` survive?** If generation covers the same ground with better + output, keeping both means two ways to get a config and one of them ships + ``.