Repository navigation
Conversation
…educe Decline node-local custom all-reduce and graph IPC registration for MNNVL-only groups, retaining their symmetric-memory all-gather and reduce-scatter paths. Add CPU-level regressions for dispatch and graph registration. Related issue: vllm-project#52778 GPU startup and model validation remain pending; this commit is shared for hardware validation before any upstream PR. Co-authored-by: OpenAI Codex Signed-off-by: Silu Panda <31051721+SiluPanda@users.noreply.github.com>
|
TLDR; We have validated that this patch resolves the abort. Setup
vllm serve nvidia/NVIDIA-Nemotron-3-Super-120B-A12B-FP8 \
--quantization modelopt \
--tensor-parallel-size 2 \
--max-model-len 4096 \
--kv-cache-dtype fp8_e4m3The essential ingredient is that the two TP ranks land on different MNNVL-capable nodes. A same-node TP=2 run does not reproduce it. Unpatched behaviourThe server dies during startup inside the Ray workers. It gets through and then both ranks die with no capture progress and no This matches your reading of the mechanism: With your patchSame commit, same base image, patch applied as the only difference: Startup completes, CUDA-graph capture completes, and a four-point concurrency sweep (1 / 16 / 64 / 128) runs to completion and reports metrics. Zero occurrences of ControlsWe pinned the exact aborting commit and ran candidate patches as sibling container layers over it, so every arm shares one base image and one vLLM wheel and the applied source patch is the only difference. Two further data points on the same baseline:
Worth stressing, because it speaks to the shape of the fix: appears on both workers in both arms — unpatched and patched alike, and in the unpatched arm it appears before the abort. Your patch leaves MNNVL allreduce setup completely untouched; it only stops the CUDA-IPC custom all-reduce from claiming a group it cannot serve. This is a correctness restoration, not a disablement. |
|
@tlrmchlsmth @jeejeelee, could you review this cross-node MNNVL/CUDA-IPC fix? The reporter validated it on two GB200 NVL72 nodes: the unpatched control aborts, while the patch completes CUDA-graph capture, startup, and all four serving-concurrency runs with MNNVL enabled. The PR description now includes these results and the remaining validation gaps. |
Thank you so much @janbernloehr for your help in validating the change! Appreciate your time and work. |
|
@tlrmchlsmth @jeejeelee, gentle follow-up: the reporter has confirmed the fix with matched failing-unpatched/passing-patched GB200 runs; could you review this PR when you have a chance? Thank you! |
|
This pull request has merge conflicts that must be resolved before it can be |
Preserve upstream docstring formatting while retaining the validated MNNVL locality explanation and both CUDA IPC guards. No executable logic or regression tests changed from the original PR patch. Validation: 16 custom-all-reduce CPU tests and 20 adjacent tests passed; 5 GPU tests skipped and 10 multi-process tests deselected. Changed-file pre-commit checks passed. No GPU rerun was performed for this update. Co-authored-by: OpenAI Codex Signed-off-by: Silu Panda <31051721+SiluPanda@users.noreply.github.com>
Pr is rebased and updated now, no more merge conflicts |
|
This pull request has merge conflicts that must be resolved before it can be |
Resolve the shared insertion-point conflict by retaining the PR's CPU fixture and the upstream SM100 fused mHC helper and GPU regression. The production file and all existing PR test functions are unchanged. Validation: 16 custom-all-reduce CPU tests and 20 adjacent tests passed; 6 GPU tests skipped and 10 multi-process tests deselected. Changed-file pre-commit and independent resolution review passed. No GPU rerun. Co-authored-by: OpenAI Codex Signed-off-by: Silu Panda <31051721+SiluPanda@users.noreply.github.com>
|
@tlrmchlsmth @jeejeelee, gentle follow-up: the latest merge conflict is resolved and 36 CPU tests pass; with the reporter's GB200 validation above, could you review or advise if anything else is needed? Thank you! |
|
This pull request has merge conflicts that must be resolved before it can be |
|
@tlrmchlsmth @jeejeelee, could you review the reporter-validated GB200 fix when convenient? The branch still needs rebasing and its CI requirements cleared before merge. Thank you! |
Purpose
Addresses #52778.
Reporter-provided multi-node GPU validation is available below; same-node GPU controls and model-output evaluation remain pending. Local validation uses CPU tests only; this PR does not claim submitter-run GPU tests.
Cross-node MNNVL-capable groups remain enabled for custom all-gather/reduce-scatter, but cannot exchange node-local CUDA IPC handles.
should_custom_ar()currently admits TP=2 cross-node groups; CUDA-graph teardown can subsequently exchange their IPC handles and terminate the process insidecudaIpcOpenMemHandle.This patch:
mnnvl_only=True, allowing the existing caller dispatch to try another backend;No CUDA kernels or arithmetic implementations are changed.
Why this is not duplicate work
Before starting, #52778 had no assignee or visible work claim. Issue/timeline checks and open-PR searches for the issue number, cross-node IPC, MNNVL custom allreduce,
mnnvl_only, andregister_graph_buffersfound no open implementation of this specific fix. The issue-number andmnnvl_onlysearches were repeated before opening this PR.Related changes address different mechanisms:
Test Plan
.venv/bin/python -m pytest tests/distributed/test_custom_all_reduce.py -v .venv/bin/python -m pytest \ tests/distributed/test_custom_all_gather_reduce_scatter.py \ tests/distributed/test_comm_ops.py -k 'not multi_process' -v .venv/bin/pre-commit run --files \ vllm/distributed/device_communicators/custom_all_reduce.py \ tests/distributed/test_custom_all_reduce.py git diff --checkTest Result
Executed locally on Apple Silicon macOS with Python 3.12.11 and PyTorch 2.13.0:
The three baseline failures cover eager/captured cross-node all-reduce dispatch and cross-node IPC graph registration. Positive controls cover same-node registration and MNNVL collective eligibility.
These tests exercise real production Python methods and CPU tensors, with CUDA operations and process-group calls mocked at the boundaries. They do not execute CUDA IPC, NCCL, or MNNVL kernels. The precompiled editable install was unavailable on this host; the Python tests ran through normal source imports after installing dependencies with
uv.Outstanding validation
External GPU validation: the issue reporter confirmed the fix on September 8, 2026. See the full validation report. On two GB200 NVL72 nodes with one GPU per node, cross-node TP=2, and custom all-reduce left enabled, the unpatched control aborts with
custom_all_reduce.cuh:143. Applying this patch as the only change allows CUDA-graph capture, server startup, and serving benchmarks at concurrency 1/16/64/128 to complete. The reporter also confirmed that MNNVL setup remains active.The submitter still has no GPU environment. Same-node GPU controls, GPU all-gather/reduce-scatter regression runs, model-output accuracy evaluation, and the remaining human-review requirements have not been reported as complete.
AI assistance
OpenAI Codex assisted with issue triage, investigation, patch preparation, and execution of the local CPU tests and lint checks. The patch was approved for publication by the submitter. The outstanding checklist is intentionally incomplete. External GPU results above are attributed to the issue reporter; they do not attest to submitter-run GPU tests, model-output accuracy evaluation, or completion of the remaining human-review requirements. The commit includes AI attribution and sign-off.
Main synchronization — September 23, 2026
Merged upstream
main(0f2a15c9) without rewriting the original validated commit. Resolved the test-file insertion conflict by retaining both the PR's CPU fixture/regressions and upstream's new SM100 fused-mHC GPU regression.-k 'not multi_process': 20 passed, 1 GPU test skipped, 10 distributed tests deselected.CI status (original PR head)
The failing
pre-run-checkis the repository's CI authorization gate, not a lint/test result. It requires one ofverified,ready, orready-run-all-tests, or at least four merged PRs from the author; the log reports one merged PR and no qualifying label. The actualpre-commitjob was therefore skipped. GitHub's ready-for-review status is distinct from those authorization labels. Local changed-file pre-commit checks passed as reported above; upstream CI remains unexecuted behind that gate. No CI policy or authorization controls were modified.