ATOM P2: ship MI325X lab-validated configs and drop engine-blocked models - #381
Conversation
cijohnson
left a comment
cijohnson
left a comment
There was a problem hiding this comment.
I think you need to rebase your branch from main
503e513 to
c11c2a1
Compare
…s only. Use platform-specific mi325x stems (lab-validated on MI325X), aligned threshold cell keys (PP=, bare metrics), and Sphinx docs without local inference markdown drafts.
bfc0016 to
b096c00
Compare
Rename ATOM variant JSON files to mi3xx family stems while keeping platform-specific mi325x threshold files. Run ruff format on atom_config_loader and its unit tests to satisfy CI fmt-check.
Drop redundant gpu_arch from shipped mi3xx configs; load_variant derives it from the {gpu}_atom_* stem with optional JSON override.
Extend schema v2 stems with perf, vllm, and sglang profiles including acc_warmup and accuracy suites, rename and align mi325x threshold files per profile, and update the config loader, unit tests, and how-to docs.
Export METRIC_UNITS for the atom run-deck profile, register --config_profile on the pytest CLI, and fix the invalid BrokenPipeError filterwarnings entry for pytest 9.
8e3088a to
433bec1
Compare
MiniMax and MiMo need block-size 128 and compile-disabled startup flags on openai_server; atom_orch now defaults enforce-eager for native atom driver. Remove GLM 5.1 ATOM variant files that fail on rc3 in lab.
Add atom_serving_config bridge and rename parity configs to atom_vllm/atom_sglang stems under the atom suite.
Re-add W2 mi3xx_atom_vllm_gpt-oss-120b_mxfp4_single with thresholds and pass platform/long_context_accuracy through the serving bridge.
Restore W2 mi3xx_atom_sglang_gpt-oss-120b_mxfp4_single with mi325x thresholds in the unified serving schema.
Convert sweeps/runs configs at pytest collection time so inference parametrization runs, and ship mi3xx native V4 Pro single-node stems for 112 lab validation.
Required by AtomVariantConfig validation; add loader unit test for the W5 longctx stem.
ROCm atom images expose vLLM as a Python package without a vllm console script on PATH; use python3 -m vllm.entrypoints.cli.main for serve and bench.
Use in-container /models for HF_HUB_CACHE and changeme-models-mount:/models:ro so lab nodes can bind it-share without hardcoding /home/models.
Keep main's cell_build.py for the report cell-status loop.
prj2 rc3 loaded the tree as v3 and LLVM-aborted; atom-dev was never grepped green. Same bar as Mistral: do not ship a known-red config.
…fail calibrated gates.
An empty prefix made startswith always true, so structured-output checks skipped JSON validation for all models.
Skip the book JSON check only when the served model id matches a reasoning marker, not because the body looks like thinking.
Eager stays only when the variant JSON sets it. Drop the unused DeepSeek V4 helper.
solaiys
left a comment
solaiys
left a comment
There was a problem hiding this comment.
Review: blocking issues
Shipped native / vLLM-parity stems load and the serving-schema conversion matches the packaged vLLM JSON. Native ATOM no longer auto-injects --enforce-eager (covered by unit tests); vLLM parity configs still set it in serve_args. enforce_thresholds: false is consistent with record-only P2.
One blocker on the shipped SGLang distributed stem: container launch will crash before docker runs.
🔴 SGLang distributed container.runtime.args.env crashes Docker launch
serving_to_atom_variant_raw copies the serving container block through unchanged, including runtime.args.env. DockerRuntime._build_runtime_args then does for key, value in env_vars on that dict, which raises ValueError: too many values to unpack while building docker run.
I reproduced the unpack crash on the dict from mi3xx_atom_sglang_deepseek-r1_fp8_distributed.json. Native/vLLM stems do not hit this because they put env on roles.server.env / server_params.env only.
Even if that loop were fixed to .items(), the values ("<changeme> rdma0,...", "<changeme> eno0") must not be passed as docker -e — ATOM already strips NCCL_IB_HCA / socket IFNAME from roles.server.env and discovers fabric at runtime.
Fix: after lifting container.runtime.args.env into roles.server.env (already done), drop env from a deep-copied container runtime.args so it never reaches DockerRuntime. Move keepers (SGLANG_USE_AITER, NCCL_IB_GID_INDEX, NCCL_DEBUG) to server_params.env and delete the placeholder NCCL device/IFNAME entries. Add a unit test that loads the distributed SGLang JSON and asserts variant.container.runtime.args has no env.
Lift keepers onto server_params.env, drop placeholder NCCL device/IFNAME docker -e values, and treat empty runs as the full sweeps list.
amd-droy
left a comment
amd-droy
left a comment
There was a problem hiding this comment.
looks good to me. Thanks Humna.
solaiys
left a comment
solaiys
left a comment
There was a problem hiding this comment.
The SGLang distributed Docker-env crash is addressed on 65fb746:
serving_to_atom_variant_rawdeep-copies the container block, lifts env, then popsruntime.args.envso DockerRuntime never unpacks that dict- keepers (
SGLANG_USE_AITER,NCCL_IB_GID_INDEX,NCCL_DEBUG) are onserver_params.env; placeholder NCCL HCA/IFNAME docker-evalues andhca_id_prefixare gone test_load_atom_sglang_distributed_drops_container_runtime_envcovers the launch invariant; emptyruns: []expands to the 16 sweep cells
LGTM from my side.
Please also get approval from Ignatious (@cijohnson) before merging — his earlier CHANGES_REQUESTED is still on the PR.
| "sweep": { | ||
| "_comment": "Combo-key runs of ISL, OSL, TP, PP, and CONC, comma-separated. Each listed run must exist in the threshold JSON. If this field is empty, every run in the threshold JSON is used.", | ||
| "runs": [] | ||
| }, |
There was a problem hiding this comment.
After sweeps {}
why do we need sweep { } with runs[ ] list ?
any reason?
we can have direct runs[ ] list itself right?
There was a problem hiding this comment.
Agreed. Dropped the extra sweep wrapper. Sweeps holds the catalog and runs is a top level list. Empty runs means all sweeps cells.
4 participants
Summary
This is the ATOM P2 slice for
cvs run atom. I aligned the suite with how we actually run on MI325X (familymi3xxconfig stems, platformmi325xthresholds, models mounted at/models), then cut stems that could not finish/healthin lab so we do not ship configs we know will fail.Shipped ATOM JSON is MI325X-only on purpose; MI300X / MI355X stems come later.
Merged current
main(merge commit, not a rebase). Forcvs/lib/report/cell_build.pyI kept main.Shipped ATOM stems
atom_vllmR1 FP8 + GPT-OSS 120B MXFP4;atom_sglangR1 FP8 (single + distributed)Every shipped ATOM config has
enforce_thresholds: false— coverage, not a gate. Native ATOM still injects--enforce-eager(except DeepSeek V4), so we do not FAIL pytest on throughput numbers that were not taken under that flag. Threshold JSON is still present for reports.Not in this PR (failed
/healthin lab; not listed in ATOM docs)atom_sglang(aiter / gfx942 on the SGLang image)AutoConfigneedsmodel_type; vLLM 0.23 still dies inMistralCommonPixtralProcessorat init)deepseek_v4as v3 then LLVM abort; atom-dev/healthnever confirmed). Same bar as Mistral: stem waits.cvs run vllmV4 JSON underinference/vllm/is a different suite.Runtime / suite fixes that unblocked lab runs
gpu_archfrom the config filenamepython3 -m vllm.entrypoints.cli.main(novllmconsole script on ATOM images)ModuleNotFoundErrorTest plan
atom_vllmon MI325Xmake fmt-check lint uton the branch