[https://nvbugs/6503293][fix] Restore whole-node GPU visibility for default packing - #16893
Merged
zongfeijing merged 1 commit intoJul 28, 2026
Merged
Conversation
…efault packing Commit db347a9 changed the slurm benchmark workers so each rank only sees a single GPU (CUDA_VISIBLE_DEVICES=SLURM_LOCALID) in the default, non-compact packing path. That is safe for DEP (enable_attention_dp=true), which has no intra-node TP all-reduce, but it breaks TEP (enable_attention_dp=false): the custom all-reduce topology check calls cudaDeviceCanAccessPeer() over the node-local ranks, and with only one visible device the peer ordinals are invalid, so every rank aborts at executor initialization with CUDA runtime error in cudaDeviceCanAccessPeer(...): invalid device ordinal (tensorrt_llm/thop/allreduceOp.cpp) before any KV-cache transfer happens. Restricting visibility to one GPU also collapses the auto-detected gpus_per_node to 1. Restore the pre-db347a94847 behaviour for default packing only: submit.py computes the node's full GPU list again and passes it to start_worker.sh, which exports it as CUDA_VISIBLE_DEVICES. Each rank still binds to its own device through mapping.local_rank (= rank % gpus_per_node). The compact packing path, added for non-divisible EP, is unchanged and keeps deriving CUDA_VISIBLE_DEVICES from the per-worker gpu_map. Verified on GB200 with qwen3-235b-fp4 tep4 (tp4 + ep4, enable_attention_dp=false): before the fix all 4 gen ranks crashed at executor init; after it gpus_per_node is detected as 4, the worker serves, and greedy generation returns correct output. A dry run confirms the compact path still emits its hostfile/gpu_map unchanged. Signed-off-by: JacobHu-NV <266902545+JacobHu-NV@users.noreply.github.com>
JacobHu-NV
force-pushed
the
fix/nvbug6503293-tep-gpu-visibility
branch
from
July 27, 2026 07:37
3fd3be7 to
aa28471
Compare
JacobHu-NV
marked this pull request as ready for review
July 27, 2026 07:42
Collaborator
Author
|
/bot run --disable-fail-fast |
Contributor
WalkthroughChangesCUDA device propagation
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant submit.py
participant srun
participant start_worker.sh
submit.py->>submit.py: Derive cuda_devices
submit.py->>srun: Pass cuda_devices
srun->>start_worker.sh: Provide ninth argument
start_worker.sh->>start_worker.sh: Set CUDA_VISIBLE_DEVICES
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/disaggregated/slurm/benchmark/start_worker.sh`:
- Line 14: Add the repository-standard NVIDIA copyright header immediately after
the shebang in start_worker.sh, using the current copyright year and preserving
the existing script content.
In `@examples/disaggregated/slurm/benchmark/submit.py`:
- Around line 635-640: Update the GPU visibility construction near node_list and
cuda_devices to use the full GPU list for each allocated node, rather than the
rank-assigned subset in allocation["nodes"].values(). Preserve whole-node
ownership semantics and ensure cuda_devices exposes every GPU on the node,
including partial and non-divisible allocations. Add coverage for both
allocation cases and the start_worker.sh full-node visibility contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5e41677d-3f37-49d2-b155-0e3243fd8071
📒 Files selected for processing (2)
examples/disaggregated/slurm/benchmark/start_worker.shexamples/disaggregated/slurm/benchmark/submit.py
Collaborator
|
PR_Github #61856 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #61856 [ run ] completed with state
|
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #61925 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #61925 [ run ] completed with state
|
Collaborator
Author
|
/bot run --disable-fail-fast |
Collaborator
|
PR_Github #62071 [ run ] triggered by Bot. Commit: |
Collaborator
|
PR_Github #62071 [ run ] completed with state |
chuangz0
approved these changes
Jul 28, 2026
qiaoxj07
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request updates the logic for setting
CUDA_VISIBLE_DEVICESin the Slurm-based benchmarking scripts to better support both default and compact GPU packing modes. The main improvement is that, in default packing mode, each worker is now exposed to the node’s full list of GPUs, which is necessary for certain intra-node communication patterns. The change also ensures that the appropriate GPU list is passed fromsubmit.pytostart_worker.sh.GPU device visibility and environment variable handling:
start_worker.sh, the script now accepts acuda_devicesargument and setsCUDA_VISIBLE_DEVICESto this value in default packing mode, rather than just usingSLURM_LOCALID. This exposes the full node GPU list to each worker, which is required for intra-node tensor parallelism with custom all-reduce operations. [1] [2]Job submission and argument propagation:
submit.py, the script constructs the correctcuda_devicesstring (a comma-separated list of GPU IDs) for default packing and passes it as an argument tostart_worker.sh. In compact packing mode, it setscuda_devicesto"none"as the GPU mapping is handled differently. [1] [2]Documentation and code comments:
These changes improve the flexibility and correctness of GPU assignment in multi-GPU, multi-node training scenarios.
Dev Engineer Review
CUDA_VISIBLE_DEVICESvisibility for default GPU packing, enabling intra-node tensor parallelism and custom all-reduce topology checks."none"and retaining the existing GPU-map-based assignment.submit.pytostart_worker.sh, with rank-to-device binding still handled throughmapping.local_rank.QA Engineer Review
No test changes.