[PR #727] utils: only reuse PyTorch's NCCL comm when _comm_ptr() is non-null #734

New issue

Open

opened 2026-10-27 21:05:43 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-27 21:05:43 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/deepseek-ai/DeepEP/pull/727
Author: @dmvevents
Created: 8/14/2026
Status: 🔄 Open

Base: main ← Head: fix/null-guard-reused-comm-ptr

📝 Commits (4)

13dab41 utils: only reuse PyTorch's NCCL comm when _comm_ptr() is non-null
0ef4e11 utils: make the PyTorch-comm reuse decision group-uniform
9cad80b tests: cover the reuse escape hatch and the cache-hit path
080984a utils: make comm.py yapf-clean so the format check can pass

📊 Changes

2 files changed (+167 additions, -5 deletions)

View changed files

📝 deep_ep/utils/comm.py (+30 -5)
➕ tests/utils/test_comm_reuse.py (+137 -0)

📄 Description

Fixes #726.

get_nccl_comm_handle's reuse branch trusts backend._comm_ptr() blindly, but PyTorch creates NCCL communicators lazily — _comm_ptr() is a passive accessor that returns 0 when the ProcessGroup+device has no communicator yet (comms materialize at the first collective on that group+device; eager creation only happens when init_process_group received device_id=...).

A consumer that constructs ElasticBuffer before any collective has run on the EP group gets NCCLCommHandle(0), and calculate_elastic_buffer_size reaches get_physical_domain_size → ncclTeamWorld(nullptr) → comm->nRanks null-deref. vLLM's expert-parallel serve path is a live reproduction: deterministic segfault on all 16 ranks (DP16/EP16) at serve init — full chain in #726.

Change: only take the borrowed-comm path when the pointer is non-null; otherwise fall through to the existing create-own-comm path. On a cold group the fallback's all_gather_object is itself a collective on that group, so it lazily materializes torch's comm as a side effect and every rank takes the same branch — no rank divergence on the cold-boot case.

Why the test suite doesn't catch it: tests/utils init_dist passes device_id=... to init_process_group, so torch eager-creates comms and _comm_ptr() is always valid there. The null case is only reachable from lazy-init consumers.

Validated E2E on the vLLM path (Qwen3-30B-A3B-FP8, DP16/EP16, 2 nodes over EFA): with the create-own-comm path taken (behaviorally identical to this guard's fall-through on a cold group), serve comes up and sustains a c=1→64 load sweep with 0 errors.

Update: the guard is now an explicit all-rank-consistent decision — every rank gathers every rank's communicator-nullness before any rank branches, so reuse happens only when all ranks hold a live communicator and no rank can be left behind in a collective. Covered by tests/utils/test_comm_reuse.py.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/deepseek-ai/DeepEP/pull/727 **Author:** [@dmvevents](https://github.com/dmvevents) **Created:** 8/14/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/null-guard-reused-comm-ptr` --- ### 📝 Commits (4) - [`13dab41`](https://github.com/deepseek-ai/DeepEP/commit/13dab41dafa68cb66aa8754ea275db1663647397) utils: only reuse PyTorch's NCCL comm when _comm_ptr() is non-null - [`0ef4e11`](https://github.com/deepseek-ai/DeepEP/commit/0ef4e1157b365f5a323330537729ee0fd0b6696c) utils: make the PyTorch-comm reuse decision group-uniform - [`9cad80b`](https://github.com/deepseek-ai/DeepEP/commit/9cad80ba06727002837ae64e48acbd598ea59d24) tests: cover the reuse escape hatch and the cache-hit path - [`080984a`](https://github.com/deepseek-ai/DeepEP/commit/080984a530e2fdf2cab4739fd84a007cef94b070) utils: make comm.py yapf-clean so the format check can pass ### 📊 Changes **2 files changed** (+167 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `deep_ep/utils/comm.py` (+30 -5) ➕ `tests/utils/test_comm_reuse.py` (+137 -0) </details> ### 📄 Description Fixes #726. `get_nccl_comm_handle`'s reuse branch trusts `backend._comm_ptr()` blindly, but PyTorch creates NCCL communicators **lazily** — `_comm_ptr()` is a passive accessor that returns **0** when the ProcessGroup+device has no communicator yet (comms materialize at the first collective on that group+device; eager creation only happens when `init_process_group` received `device_id=...`). A consumer that constructs `ElasticBuffer` before any collective has run on the EP group gets `NCCLCommHandle(0)`, and `calculate_elastic_buffer_size` reaches `get_physical_domain_size` → `ncclTeamWorld(nullptr)` → `comm->nRanks` null-deref. vLLM's expert-parallel serve path is a live reproduction: deterministic segfault on all 16 ranks (DP16/EP16) at serve init — full chain in #726. **Change:** only take the borrowed-comm path when the pointer is non-null; otherwise fall through to the existing create-own-comm path. On a cold group the fallback's `all_gather_object` is itself a collective on that group, so it lazily materializes torch's comm as a side effect and every rank takes the same branch — no rank divergence on the cold-boot case. **Why the test suite doesn't catch it:** `tests/utils` `init_dist` passes `device_id=...` to `init_process_group`, so torch eager-creates comms and `_comm_ptr()` is always valid there. The null case is only reachable from lazy-init consumers. Validated E2E on the vLLM path (Qwen3-30B-A3B-FP8, DP16/EP16, 2 nodes over EFA): with the create-own-comm path taken (behaviorally identical to this guard's fall-through on a cold group), serve comes up and sustains a c=1→64 load sweep with 0 errors. Update: the guard is now an explicit all-rank-consistent decision — every rank gathers every rank's communicator-nullness before any rank branches, so reuse happens only when all ranks hold a live communicator and no rank can be left behind in a collective. Covered by `tests/utils/test_comm_reuse.py`. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-27 21:05:43 +00:00

bsakaguchi referenced this issue

2026-10-27 21:05:46 +00:00

[PR #734] bug fix: probe RDMA link rate via sysfs, survive probe failure #740