Skip to content

feat: multi-node comm data (world 8/16, gb200) + data-driven beyond-node custom-AR fan-out - #1593

Open
Arsene12358 wants to merge 1 commit into
mainfrom
yimingl/aic-1808-multi-node-comm
Open

feat: multi-node comm data (world 8/16, gb200) + data-driven beyond-node custom-AR fan-out#1593
Arsene12358 wants to merge 1 commit into
mainfrom
yimingl/aic-1808-multi-node-comm

Conversation

@Arsene12358

@Arsene12358 Arsene12358 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

What

Closes the sglang multi-node TPOT under-prediction on gb200 (Linear AIC-1808). One rust change plus measured multi-node comm data; one new parity golden pins the repriced path end-to-end.

  1. query_custom_allreduce_scaled honors recorded beyond-node fan-outs. The query previously hard-clamped tp to num_gpus_per_node and extrapolated beyond it with a p2p-bandwidth ratio, because comm collection was single-node and no table recorded larger fan-outs. On gb200 (modeled as 4-GPU nodes; the NVL72 reroute is gated on num_gpus_per_node == 72 and never fires) that priced tp16 allreduces at ~2.7µs while 16-rank serving measures ~48–50µs per AR across the NVL72 segment (~185 ARs per decode step). Now, when the custom-AR table records fan-outs beyond the node size, the query uses the largest recorded fan-out ≤ tp directly and only bandwidth-scales beyond it. Tables that stop at or below the node size keep the legacy clamp+scale behavior bit-for-bit (pinned by a new unit test; the legacy formula is algebraically unchanged in that case).
  2. gb200 comm/nccl/2.23: +336 rows — nccl-tests sweeps (all_reduce, all_gather, reduce_scatter, alltoall × half/int8) at num_gpus 8 and 16, 512B–512MB, collected on oci-hsg GB200 NVL72 (4 nodes × 4 GPUs, segment-pinned, srun --mpi=pmix, NCCL 2.27.7). Feeds query_nccl_scaled consumers (alltoall/AG/RS at >4 ranks) which previously bandwidth-scaled from the 4-rank curves.
  3. gb200 comm/sglang/0.5.14: +92 rowscollector/network/collect_all_reduce.py --backend sglang --use-slurm at world 8/16 on the sglang v0.5.17 image. At beyond-node fan-outs sglang custom AR disables itself ("process group spans across nodes"), so the rows measure the pynccl fallback — the exact serving lane. Three-way agreement at 16 ranks: 48.8µs (this collector, graph @128b) ≈ ~50µs (nsys-audited serving decode) ≈ ~48µs (nccl-tests).

Validation

Stacked on the Qwen3.8 support branch (#1583) and compared against measured serving (GB200 NVL72 tp16 fp8, direct engine, jobs 6491036/6506559/6512879 — details in the AIC-1782/AIC-1808 Linear trail):

point before after
sglang TPOT err @ conc 1 / 8 / 32 −54% / −37% / −18% −12.7% / −14.6% / −5.7%
sglang static per-step err @ bs1 / bs32 −57% / −26% −17.5% / −11.7%
vLLM TPOT (all conc) 11.1% median byte-identical

Suites: cargo 515 passed; parity 451 passed (4 new golden records for the added tp16 gb200/sglang case, append-only pin); sdk pytest 2027 passed.

Deliberately not included

  • vLLM multi-node AR rows (collected in the same jobs, banked): the vllm 0.27.1 benchmark measures ~47µs at 16 ranks (its custom-AR and symm-mem lanes both refuse cross-node groups), yet vllm tp16 serving is fast (8.96 ms/step bs1) and accurately predicted under the legacy pricing. Repricing its ARs to the benchmarked value would break a validated arm; how vllm serving avoids that cost is an open question tracked in AIC-1808.
  • gb300 rows: needs GB300 silicon access; follow-up on the same issue.
  • New rows carry their true measurement-stack versions in the version column (nccl 2.27.7, sglang 0.5.17) while living in the resolution-keyed dirs (2.23, 0.5.14) — same precedent as the existing 2.23 dir whose rows self-report 2.29.2; loaders ignore the column.

Linear: AIC-1808

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Improvements

    • Improved communication performance estimates for multi-node tensor-parallel workloads.
    • Custom all-reduce calculations now use the closest available recorded fan-out before scaling remaining capacity, improving accuracy for larger configurations.
    • Preserved legacy behavior when detailed multi-node measurements are unavailable.
  • Validation

    • Added coverage for Qwen3.5-397B-A17 deployments using 16-way tensor parallelism across multiple nodes.

… multi-node comm rows (world 8/16)

query_custom_allreduce_scaled previously hard-clamped tp to
num_gpus_per_node and extrapolated beyond it with a p2p-bandwidth ratio,
because comm collection was single-node and no table recorded larger
fan-outs. On gb200 (modeled as 4-GPU nodes) that priced tp16 allreduces
at ~2.7us while 16-rank serving (sglang pynccl ring across the NVL72
segment) measures ~48-50us per AR (~185 ARs/decode step), driving a 37%
median TPOT under-prediction for Qwen3.8-Max sglang fp8 tp16.

Change: when the custom_allreduce table records fan-outs beyond the node
size (multi-node collection), use the largest recorded fan-out <= tp
directly and only bandwidth-scale beyond it. Tables that stop at or
below the node size keep the legacy clamp+scale behavior bit-for-bit
(pinned by the new unit test; full rust suite and sdk pytest unchanged).

Data (collected on oci-hsg GB200 NVL72, 4 nodes x 4 GPUs, segment-pinned,
slurm 6515005/6516705):
- gb200 comm/nccl/2.23: +336 rows — nccl-tests sweeps (all_reduce,
  all_gather, reduce_scatter, alltoall x half/int8) at num_gpus 8 and 16
  (512B..512MB), NCCL 2.27.7. Consumed by query_nccl_scaled users
  (alltoall/AG/RS at >4 ranks) which previously bandwidth-scaled from
  the 4-rank curves.
- gb200 comm/sglang/0.5.14: +92 rows — collect_all_reduce.py --backend
  sglang under srun (--use-slurm) at world 8/16 on the sglang v0.5.17
  image; at >node fan-outs sglang custom AR disables itself and the rows
  measure the pynccl fallback, i.e. the exact serving lane.

vLLM rows at world 8/16 were collected in the same jobs but deliberately
NOT landed: the vllm 0.27.1 benchmark measures ~47us at 16 ranks (its
custom AR and symm-mem lanes both refuse cross-node groups), yet vllm
serving at tp16 measures 8.96 ms/step bs1 with 11% prediction accuracy
under the legacy pricing — repricing its ARs to the benchmarked value
would break a validated arm. Evidence banked in AIC-1808.

Validation (Qwen3.8-Max fp8 tp16 gb200 vs measured serving, stacked on
the qwen3.8 support branch): sglang TPOT errs move from -54/-37/-18% to
-12.7/-14.6/-5.7% (conc 1/8/32); static per-step bs1 -57% -> -17.5%,
bs32 -26% -> -11.7%; vllm predictions byte-identical.

Linear: AIC-1808

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Yiming Liu <yimingl@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the feat label Aug 25, 2026
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 7d4cf9d6-32ff-4cd1-a32b-8cba8d7488da

📥 Commits

Reviewing files that changed from the base of the PR and between 231c1e4 and aa4848d.

⛔ Files ignored due to path filters (2)
  • aic-core/src/aiconfigurator_core/systems/data/gb200/comm/nccl/2.23/nccl_perf.parquet is excluded by !**/*.parquet and included by aic-core/**
  • aic-core/src/aiconfigurator_core/systems/data/gb200/comm/sglang/0.5.14/custom_allreduce_perf.parquet is excluded by !**/*.parquet and included by aic-core/**
📒 Files selected for processing (3)
  • aic-core/rust/aiconfigurator-core/parity_tests/goldens/engine_step.json
  • aic-core/rust/aiconfigurator-core/parity_tests/test_engine_step_parity.py
  • aic-core/rust/aiconfigurator-core/src/perf_database/communication.rs

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout. (16)
  • GitHub Check: Collect snapshot (old)
  • GitHub Check: Collect snapshot (new)
  • GitHub Check: aic-core public API contract
  • GitHub Check: Build and Test (unit)
  • GitHub Check: Rust/Python engine-step parity
  • GitHub Check: Python 3.12 compatibility
  • GitHub Check: Build and Test (e2e)
  • GitHub Check: Cargo Deny
  • GitHub Check: Perf data sanity (informational)
  • GitHub Check: Build wheels (macosx_arm64)
  • GitHub Check: codeowners
  • GitHub Check: Check collector data
  • GitHub Check: Build wheels (manylinux_2_28_x86_64)
  • GitHub Check: create-charts
  • GitHub Check: Build wheels (manylinux_2_28_aarch64)
  • GitHub Check: parquet-diff
🧰 Additional context used
📓 Path-based instructions (1)
- Prefer applicable inline comments. When the correct fix is clear, small, and limited to the commented diff hunk, include it as a GitHub Suggested Change so the author can apply it with one click.

⚙️ CodeRabbit configuration file

Files:

  • aic-core/rust/aiconfigurator-core/parity_tests/goldens/engine_step.json
  • aic-core/rust/aiconfigurator-core/src/perf_database/communication.rs
  • aic-core/rust/aiconfigurator-core/parity_tests/test_engine_step_parity.py

Walkthrough

Changes

The custom-allreduce scaling query now uses recorded beyond-node fan-outs before extrapolating overflow. Regression and parity coverage add TP16 Qwen3.5-397B-A17B execution modes.

Custom Allreduce Scaling

Layer / File(s) Summary
Recorded fan-out lookup and scaling
aic-core/rust/aiconfigurator-core/src/perf_database/communication.rs
The scaled query selects the largest recorded beyond-node fan-out within the requested TP range. Regression tests cover exact lookup, partial scaling, and legacy behavior.
TP16 parity case coverage
aic-core/rust/aiconfigurator-core/parity_tests/test_engine_step_parity.py, aic-core/rust/aiconfigurator-core/parity_tests/goldens/engine_step.json
The parity suite adds the Qwen3.5-397B-A17B TP16 scan-coverage case. All four execution modes record ValueError results and Rust post-freeze pins.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to aa484

This change updates multi-node communication pricing and adds measured GB200 data with parity coverage; no actionable merge-blocking risk remains based on the supplied evidence, so it is merge-ready after normal checks and review.

Poem

Recorded fan-outs guide the way
Overflow scales only what must sway
TP16 joins the parity line
Four ValueErrors mark the sign
Rust pins keep the result in time

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main changes: multi-node communication data and data-driven beyond-node custom-AR fan-out handling.
Description check ✅ Passed The description is detailed and covers the change scope, implementation details, validation results, exclusions, and related Linear issue. It does not use the template headings and does not explicitly…
Full details: Description check

Explanation

The description is detailed and covers the change scope, implementation details, validation results, exclusions, and related Linear issue. It does not use the template headings and does not explicitly identify where reviewers should start, but it is mostly complete.


Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Perf Parquet Diff Report

Compared origin/main to HEAD for aic-core/src/aiconfigurator_core/systems/data.

  • Parquet files changed: 2
  • CSV-to-parquet conversions checked: 0
  • Conversions with matching columns and rows: 0
  • New parquet files without a base CSV/parquet counterpart: 0
  • Modified or renamed parquet files: 2
  • Deleted parquet files: 0
  • Legacy *_perf.txt files added or modified: 0
  • Row-level changes: +428 / -0 / ~0
  • Full per-file diff artifacts: 2 files under parquet-diff-details/diffs/

Per-File Row Diff Preview

Showing the first 3 rows per diff kind for each changed parquet file. Full exact CSVs are in parquet-diff-details/.

aic-core/src/aiconfigurator_core/systems/data/gb200/comm/nccl/2.23/nccl_perf.parquet
  • Rows: +336 / -0 / ~0
  • Key columns: framework, version, device, op_name, kernel_source, nccl_dtype, num_gpus, message_size
  • Note: duplicate keys; unmatched rows paired within key

added rows - full CSV: parquet-diff-details/aic-core/src/aiconfigurator_core/systems/data/gb200/comm/nccl/2.23/nccl_perf.parquet.added.csv

framework,version,device,op_name,kernel_source,nccl_dtype,num_gpus,message_size,latency
TRTLLM,2.27.7,NVIDIA GB200,all_gather,NCCL,half,16,1024,0.02846
TRTLLM,2.27.7,NVIDIA GB200,all_gather,NCCL,half,16,1048576,0.03176
TRTLLM,2.27.7,NVIDIA GB200,all_gather,NCCL,half,16,131072,0.03131
aic-core/src/aiconfigurator_core/systems/data/gb200/comm/sglang/0.5.14/custom_allreduce_perf.parquet
  • Rows: +92 / -0 / ~0
  • Key columns: framework, version, device, op_name, kernel_source, allreduce_dtype, num_gpus, message_size, backend

added rows - full CSV: parquet-diff-details/aic-core/src/aiconfigurator_core/systems/data/gb200/comm/sglang/0.5.14/custom_allreduce_perf.parquet.added.csv

framework,version,device,op_name,kernel_source,allreduce_dtype,num_gpus,message_size,latency,backend
SGLang,0.5.17,NVIDIA GB200,all_reduce,SGLang_CustomAllReduce_eager,bfloat16,16,1024,0.1678281593322754,sglang_eager
SGLang,0.5.17,NVIDIA GB200,all_reduce,SGLang_CustomAllReduce_eager,bfloat16,16,1048576,0.1297692775726318,sglang_eager
SGLang,0.5.17,NVIDIA GB200,all_reduce,SGLang_CustomAllReduce_eager,bfloat16,16,128,0.1257126426696777,sglang_eager

Artifact Contents

  • Full per-file unified diffs: perf-parquet-diff artifact under parquet-diff-details/diffs/ (2 files)
  • Exact row-level CSVs: perf-parquet-diff artifact under parquet-diff-details/ (listed in summary.csv)

@github-actions

Copy link
Copy Markdown
Contributor

Sanity Check Chart Generation Report

📥 Download all sanity charts from workflow artifacts

New perf data files were detected in this PR. Please use the link above to
download sanity check charts for the new perf data to compare the collected
perf data vs SOL (theoretical max performance).

Below is a report of whether the chart generation was successful for each op.
If doesn't validate whether the perf data itself is sane.

Chart Generation Report for system: gb200, backend: sglang, backend_version: 0.5.14

  • allreduce
  • CLI smoke test ✅

@Arsene12358
Arsene12358 marked this pull request as ready for review August 26, 2026 05:39
@Arsene12358
Arsene12358 requested review from a team as code owners August 26, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant