Skip to content

Hnimrama/ix atom multinode#247

Open
hnimra-amd wants to merge 49 commits into
dev/dtnifrom
hnimrama/ix-atom-multinode
Open

Hnimrama/ix atom multinode#247
hnimra-amd wants to merge 49 commits into
dev/dtnifrom
hnimrama/ix-atom-multinode

Conversation

@hnimra-amd

@hnimra-amd hnimra-amd commented Jul 6, 2026

Copy link
Copy Markdown

Add multinode ATOM inference support for InferenceX atom suites, including distributed serve, scaling-efficiency gates, and 2-node W1 perf sweep variants for MI300X and MI355X.

Motivation

Single-node inferencex_atom coverage exists on dev/dtni, but lab validation for multi-GPU / multi-node DeepSeek R1 runs requires a DTNI path that can launch ATOM across cluster ranks, assert scaling efficiency, and run a realistic multinode sweep matrix.

This PR adds that multinode layer: distributed ATOM serve, multinode config/schema fields, scaling metrics in gates and reports, 2-node cluster examples for MI300X and MI355X, and matching perf_multi variants with expanded sweep thresholds.

Merge order: Intended to land after hnimrama/refactor-ix-atom (moves shared suite helpers under inference/utils/). Expect a short rebase/conflict-resolution pass once that refactor merges.

Technical Details

Suite rename

  • inferencex_atom_singleinferencex_atom (test module, config directory, docs, CLI references)
  • Config moved: cvs/input/config_file/inference/inferencex_atom_single/inferencex_atom/
  • Report preset shim inferencex_atom_single.py removed (auto-register uses inferencex_atom stem)

Multinode orchestration (InferenceXAtomJob)

  • Distributed ATOM-native multinode serve via SPMD env vars (ATOM_DP_*, -dp), not vLLM --node-rank / PP flags
  • _atom_multinode_argv() isolates ATOM CLI tokens; vLLM distributed flags stripped from atom_args when present
  • parse_results accepts optional scaling_baseline_output_throughput + nnodes for efficiency derivation

Config & metrics

  • New params: nnodes, node_rank, master_addr, master_port, pipeline_parallel_size, scaling_baseline_output_throughput
  • New metric tier scaling with scaling.efficiency_pct (= actual / (single-node baseline × nnodes))
  • Loader validates multinode variants define scaling.efficiency_pct thresholds when baseline is set
  • Cell keys include PP= / NN= suffixes when multinode dimensions apply

Cluster & variants

GPU Cluster file Variant
MI300X cvs/input/cluster_file/mi300x_atom_multi.json mi300x_inferencex-atom-single_deepseek-r1_fp8_perf_multi
MI355X cvs/input/cluster_file/mi355x_atom_multi.json mi355x_inferencex-atom-single_deepseek-r1_fp8_perf_multi

Both variants use the same sweep matrix: 5 sequence shapes × concurrencies 16/64/128 (15 cells), nnodes=2, PP=2.

MI300X: rocm/atom-dev:latest, scaling_baseline_output_throughput=1500, enforce_thresholds: true.

MI355X: rocm/atom-dev:nightly_202606211542, scaling_baseline_output_throughput=4000 (seeded from single-node CI), enforce_thresholds: false until 2-node lab confirms.

Infrastructure

  • container.py: install openssh-server when image lacks sshd (multinode SSH setup)
  • Lifecycle/report fixes: CollectReport tolerance in HTML hooks, test_launch_container syntax repair

Key commits

  1. Multinode params + scaling efficiency metric
  2. Distributed ATOM serve across cluster ranks
  3. MI300X 2-node cluster + perf_multi variant
  4. Unit tests + docs for multinode routing/config
  5. Orchestrator openssh install
  6. ATOM-native multinode serve fixes + argv isolation
  7. Expanded multinode sweep matrix + scaling threshold enforcement
  8. Lifecycle/report fixes for multinode runs
  9. Suite rename inferencex_atom_singleinferencex_atom
  10. MI355X multinode cluster + perf_multi variant + config-loader test

Test Plan

  • Unit tests: multinode job routing, config loading, and orch parse (test_inferencex_atom_config_loader, test_inferencex_atom_orch_parse, test_inferencex_atom_parsing)
  • Unit test: test_load_w1_mi355x_multinode_variant
  • Lab smoke + perf: cvs run inferencex_atom with multinode cluster + perf_multi (2-node MI325X, W1)

Test Result

Check Result
Multinode unit tests (test_inferencex_atom_config_loader, test_inferencex_atom_orch_parse) Added
test_load_w1_mi355x_multinode_variant Added
Multinode lab run (2-node MI325X) Pass

@hnimra-amd
hnimra-amd force-pushed the hnimrama/ix-atom-multinode branch 6 times, most recently from 1ca54e2 to 3117e6d Compare July 8, 2026 18:33
@hnimra-amd
hnimra-amd force-pushed the hnimrama/ix-atom-multinode branch from 3117e6d to 33b0e4a Compare July 13, 2026 18:24
@hnimra-amd
hnimra-amd force-pushed the hnimrama/ix-atom-multinode branch from 480b192 to d4ef345 Compare July 15, 2026 17:54
…tric

Extend the variant schema with nnodes, pipeline parallelism, rendezvous
addresses, and optional single-node throughput baseline for
scaling.efficiency_pct. Multinode sweep cells include PP and NNODES in
the threshold cell key.
When params.nnodes is greater than one, launch one server per host with
per-rank distributed executor flags, poll all ranks for failures, and
run the benchmark client on the head node only. Single-node runs are
unchanged.
…riant

Ship example cluster JSON, config, and seeded thresholds for the W1
DeepSeek R1 FP8 multinode reference cell. Thresholds stay record-only
until lab confirmation.
Extend FakeOrch with exec_on_head and per-host exec tracking; add unit
tests for distributed server launch, head-only client, scaling metric,
and perf_multi variant schema.
Add lab run instructions for the 2-node W1 perf_multi stem and describe
nnodes, pipeline parallelism, rendezvous, and scaling baseline fields.
Multinode container runs need in-container sshd on port 2224; some ROCm
images ship without openssh-server. Fall back to apt install before start.
…cycle

The function body was left orphaned after the du_bytes import, causing an IndentationError when pytest collected inferencex_atom_single.
… PP flags

Replace unrecognized vLLM distributed CLI args with ATOM SPMD env and -dp when TP allows; TP8 two-node runs independent replicas per host. Harden multinode sshd setup (iproute2, pgrep fallback) and health-check all ranks.
…and report

Enable threshold enforcement for the 2-node perf variant, gate scaling.efficiency_pct as a dedicated metric tier, and surface it in the IX Run Deck preset.
Restore the newline between the docstring and timing call that was lost during rebase conflict resolution.
Collection failures emit CollectReport objects without user_properties; avoid AttributeError so pytest-html surfaces the real collection error.
Drop the misleading _single suffix so multinode runs use cvs run inferencex_atom; keep inferencex_atom_single as a deprecated config framework alias.
Clarify lab setup order so Makefile can manage .cvs_venv without an active shell inside it.
Apply ruff formatting and resolve lint issues in the 12 multinode-touched Python files so fmt-check and ruff check pass on this branch delta.
…riant

Add mi355x_atom_multi cluster file, perf_multi config/threshold scaffold, README runbook, and config-loader unit test mirroring the MI300X 2-node path.
DeepSeek R1 FP8 sweep over 1K/1K and 8K/1K at concurrency 4 through 256 with portable gated thresholds.
Replace four GPU/topology-specific cluster files with one inferencex_atom_cluster.json and document matching node_dict length to variant nnodes.
…ingle to inferencex-atom

Align shipped variant and threshold filenames with the inferencex_atom suite naming.
@hnimra-amd
hnimra-amd force-pushed the hnimrama/ix-atom-multinode branch from b11c5d6 to b71e9d8 Compare July 21, 2026 21:27

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For driver=atom with the configs (TP=8, nnodes=2), seems like the setup is not a coordinated 2-node distributed model. It is two independent full replicas, and the benchmark only uses the head. atom_spmd_dp_cli() returns [] because tp * nnodes > 8, so neither -dp nor ATOM_DP* env vars are applied. _atom_server_argv(rank) also ignores rank on the ATOM path.

Both nodes therefore start the same -tp 8 command with no rank-specific coordination, while configs/thresholds label cells as PP=2,NNODES=2.

After containers are up, the inference test runs the job:
test_inferencex_atom_inference() → InferenceXAtomJob.start_server()
For driver=atom, the serve command is built from:
roles.server.atom_args in config → ["-tp", "8", "--kv_cache_dtype", "fp8", ...]
_atom_server_argv() , that becomes roughly:
python -m atom.entrypoints.openai_server --model ... --server-port 8000 -tp 8 --kv_cache_dtype fp8 ...

Comment thread cvs/core/orchestrators/container.py Outdated
Remove runtime apt-get install of openssh-server and raise a clear error
when /usr/sbin/sshd is absent so multinode jobs fail at setup time.
…ultinode

Extend the orchestrator with framework-coordinator paths that preserve vLLM
distributed executor flags and SGLang PP launch, while standalone atom keeps
SPMD data parallel for scale-out without native pipeline parallel.
Add loader and orchestrator unit tests for PP=2 cell keys, vLLM executor
flags with headless workers, and SGLang dist-init launch wiring.
Replace driver=atom multinode configs with vllm_atom pipeline parallel,
serve_args, ib_netdev, and PP=2 threshold cell keys for W1 scaling runs.
Ship a SGLang pipeline-parallel multinode config and seed thresholds for
MI300X W1 scaling parity alongside the vLLM-ATOM path.
Clarify that true pipeline parallel requires a framework coordinator driver,
not standalone atom, and update variant tables for PP=2 multinode configs.
Document vllm_atom and sglang as multinode pipeline coordinators, refresh
the automation plan M5 status, and expand variant README lab prerequisites.
… labs

Clarify rank-0 GPU head IP vs jumphost and default cluster_id_ed25519 key path.

@atnair-amd atnair-amd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_tail_server_logs returns wrong log content per host in multinode runs — see inline comment below.

Comment on lines +512 to +519
def _tail_server_logs(self, lines=30):
if self.distributed:
out = {}
for rank in range(self.nnodes):
chunk = self._exec_all(f"tail -{lines} {shlex.quote(self._rank_server_log(rank))}")
out.update(chunk or {})
return out
return self._exec_all(f"tail -{lines} {shlex.quote(self.server_log)}")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_tail_server_logs returns wrong log content per host in multinode runs

def _tail_server_logs(self, lines=30):
    if self.distributed:
        out = {}
        for rank in range(self.nnodes):
            chunk = self._exec_all(f"tail -{lines} {shlex.quote(self._rank_server_log(rank))}")
            out.update(chunk or {})
        return out

_exec_all runs the command against all hosts (no hosts= filter), unlike start_server's equivalent per-rank call which does pass hosts=[host]. Each loop iteration tails rank N's log path on every host, and out.update(...) means the final iteration's paths win for every host key. With 2 nodes: after the loop, both host entries end up holding the result of tailing rank1's log path, evaluated on each host -- not each host's own server log.

This is called from wait_ready() for early-failure detection, so a crash on node0 can be masked by whatever rank1's log path resolves to on node0 -- multinode early-failure detection is effectively broken.

Proposed fix: filter by host, matching the pattern already used in start_server:

def _tail_server_logs(self, lines=30):
    if self.distributed:
        hosts = list(getattr(self.orch, "hosts", []) or [])
        out = {}
        for rank, host in enumerate(hosts):
            chunk = self._exec_all(
                f"tail -{lines} {shlex.quote(self._rank_server_log(rank))}", hosts=[host]
            )
            out.update(chunk or {})
        return out
    return self._exec_all(f"tail -{lines} {shlex.quote(self.server_log)}")

@atnair-amd atnair-amd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_deprecated_framework_alias_normalizes fails on current HEAD — see inline comment below.

self.assertEqual(variant.expected_cells(), ["ISL=7168,OSL=1024,TP=8,CONC=64"])
self.assertIn("enforce-eager", variant.roles.server.serve_args)

def test_deprecated_framework_alias_normalizes(self):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_deprecated_framework_alias_normalizes fails on current HEAD

This test builds a driver="atom" variant without roles.server.atom_args, but a validator (_atom_driver_requires_inline_server_args) now raises ValueError whenever driver=="atom" and atom_args is empty. The test wasn't updated alongside the new validator, so python -m unittest cvs.lib.inference.unittests.test_inferencex_atom_config_loader currently fails at this HEAD.

Proposed fix: add roles={"server": {"atom_args": ["-tp", "8"]}} to this test's InferenceXAtomVariantConfig(...) call (or switch the fixture to driver="vllm" if inline args aren't the point of the test).

@atnair-amd

Copy link
Copy Markdown
Collaborator

Two blockers remain before merge:

  1. python -m unittest discover -s cvs/lib/inference/unittests -p "test_inferencex_atom*.py" currently fails with 4 errors (see inline comments plus the note below) — none are exotic, all are stale test fixtures not updated alongside recent validator/key changes.
  2. _tail_server_logs has a real host-clobbering bug that breaks early-failure detection on multinode (see inline comment) — this is a correctness issue in shipped code, not just tests.

Recommend fixing both before merge; happy to send a small patch for all 4 if useful.


server_session_key tests use stale SimpleNamespace fixtures missing nnodes

server_session_key() reads p.nnodes, p.pipeline_parallel_size, p.master_addr, p.master_port (added for multinode reuse-key disambiguation). Three existing tests build params=SimpleNamespace(driver="atom", tensor_parallelism="8") without those fields, so they now fail with AttributeError: 'SimpleNamespace' object has no attribute 'nnodes' at this HEAD:

  • test_reuse_server_flag_and_session_key_helpers in cvs/lib/inference/unittests/test_inferencex_atom_config_loader.py:350 (predates this PR, unchanged — noting here since GitHub won't let me anchor a comment on an untouched line)
  • test_server_session_key_differs_for_model and test_server_session_key_differs_for_shape in cvs/lib/inference/unittests/test_inferencex_atom_server_reuse.py (also not part of this PR's diff)

This is purely a test-fixture staleness issue, not a production bug — the real InferenceXAtomParams.nnodes field has a pydantic default ("1"), so actual callers never hit this.

Proposed fix: add the missing fields to each fixture's params=SimpleNamespace(...), e.g. SimpleNamespace(driver="atom", tensor_parallelism="8", nnodes="1", pipeline_parallel_size="1", master_addr="", master_port="29501").

Drop dedicated smoke configs so multinode and single-node variants follow the same vLLM-style naming convention.
Check only rank-0 startup on the head node for PP runs, extend distributed poll timeouts for DeepSeek cold start, and update threshold_json paths plus lab docs for the renamed single/distributed configs.
Wire IB HCA discovery and GLOO_SOCKET_IFNAME like the vLLM suite so PP=2 lab runs get correct NCCL env exports.
…heck.

Single-node runs already skip sshd setup; remove the redundant openssh-server gate that blocked minimal images unnecessarily.
Resolve GLOO/NCCL socket interfaces from cluster IPs in test_discover_topology so labs no longer hand-enter ib_netdev each run.
Strip orchestrator-managed NCCL/Gloo env keys so old lab configs load without blocking socket netdev discovery.
Set ISL=512/OSL=512 CONC=16 gates to 80% of measured throughput and scaling efficiency, and 110% of measured TTFT/TPOT tails.
Raise mean_ttft and p99_ttft maxes to 110% of measured values (402ms and 15441ms).
Fixes vLLM Bad Request failures when random 2k/1k prompts exceed 4096 context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants