Skip to content

Commit 3bdabae

Browse files
authored
fix: Only bind device id when needed, Fixes #8248 (#8269)
Fixes #8248. ### The change `init_process_group` now asks a small resolver instead of inlining the condition: ```python device_id = get_init_process_group_device_id(world_size) if device_id is not None: kwargs.update(device_id=device_id) ``` `DEEPSPEED_SET_DEVICE_ID` overrides the decision, parsed by a new `get_env_flag` helper: | Value | Effect | |---|---| | `1` / `true` / `yes` / `on` | always bind | | `0` / `false` / `no` / `off` | never bind | | unset | the default above: bind iff `world_size > 1` | | anything else | warn and ignore, so the default still decides | Multi-rank behaviour is unchanged. ### Tests `tests/unit/comm/test_dist.py` --------- Signed-off-by: pengdurice <pengduhit@gmail.com>
1 parent 4189091 commit 3bdabae

2 files changed

Lines changed: 77 additions & 1 deletion

File tree

deepspeed/comm/torch.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,22 @@ def disable_compiler_collective(func):
2828
return compiler.disable(func)
2929

3030

31+
def known_world_size(world_size):
32+
"""The world size when it can be determined, otherwise None.
33+
34+
``init_distributed`` defaults its ``world_size`` argument to -1, and the value can also be
35+
carried in the ``init_method`` URL (``tcp://host:port?world_size=2``), where neither the
36+
argument nor ``WORLD_SIZE`` reflects it. Return None rather than a guess so that callers do
37+
not act on an assumed size.
38+
"""
39+
if world_size is not None and world_size > 0:
40+
return world_size
41+
env_world_size = os.environ.get('WORLD_SIZE', '')
42+
if env_world_size.isdigit() and int(env_world_size) > 0:
43+
return int(env_world_size)
44+
return None
45+
46+
3147
def build_shm_op():
3248
builder = get_accelerator().create_op_builder("ShareMemCommBuilder")
3349
if builder is None or not deepspeed.ops.__compatible_ops__.get(builder.NAME, False):
@@ -251,7 +267,11 @@ def init_process_group(self, backend, timeout, init_method, rank, world_size):
251267
# 1. device_id arg was added in torch==2.3
252268
# 2. setting device_id leads to hanging in 2.6.0<torch<2.7.1 https://github.com/pytorch/pytorch/issues/153960
253269
# 3. device_id works and is needed for `cuda`, other accelerators may have issues at the moment. Therefore only do it for the `cuda` accelerator.
254-
if ('device_id' in inspect.signature(torch.distributed.init_process_group).parameters
270+
# 4. binding a device also makes torch build every later new_group() with ncclCommSplit;
271+
# a single-rank job has no peer to reach, so skip it there (#8248). Only skip when the
272+
# world size is actually known, never on an assumed one.
273+
if (known_world_size(world_size) != 1
274+
and 'device_id' in inspect.signature(torch.distributed.init_process_group).parameters
255275
and not (version.parse("2.6.0") < version.parse(torch.__version__) < version.parse("2.7.1"))
256276
and get_accelerator().device_name() == 'cuda'):
257277
local_rank = int(os.environ.get('LOCAL_RANK', 0))

tests/unit/comm/test_dist.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
# DeepSpeed Team
55

6+
import importlib
67
import os
78
import torch
89
import deepspeed.comm as dist
@@ -396,3 +397,58 @@ def test_no_init(self, dist_init_required):
396397
config=config_dict,
397398
model_parameters=model.parameters(),
398399
dist_init_required=dist_init_required)
400+
401+
402+
# `deepspeed.comm.torch` is shadowed by the real torch module in the `deepspeed.comm` namespace.
403+
ds_comm_torch = importlib.import_module("deepspeed.comm.torch")
404+
405+
406+
@pytest.mark.parametrize("world_size,env,expected", [
407+
(2, None, 2),
408+
(1, None, 1),
409+
(-1, "4", 4),
410+
(-1, None, None),
411+
(-1, "", None),
412+
])
413+
def test_known_world_size(monkeypatch, world_size, env, expected):
414+
# -1 with nothing in the environment means the size is genuinely unknown: it may be carried
415+
# in the init_method URL. Returning None keeps the caller from skipping the device binding
416+
# on an assumed size.
417+
monkeypatch.delenv("WORLD_SIZE", raising=False)
418+
if env is not None:
419+
monkeypatch.setenv("WORLD_SIZE", env)
420+
assert ds_comm_torch.known_world_size(world_size) == expected
421+
422+
423+
def assert_device_binding(expect_bound):
424+
"""A device is bound for multi-rank jobs, and not for a single-rank one (#8248)."""
425+
if get_accelerator().communication_backend_name() != 'nccl':
426+
pytest.skip("device_id is only bound for the nccl backend")
427+
428+
deepspeed.init_distributed(dist_backend='nccl', auto_mpi_discovery=False)
429+
default_pg = torch.distributed.distributed_c10d._get_default_group()
430+
assert (default_pg.bound_device_id is not None) == expect_bound
431+
432+
# The eager split new_group() makes when a device is bound is the call that fails in #8248.
433+
device = get_accelerator().device(int(os.environ["LOCAL_RANK"]))
434+
default_backend = default_pg._get_backend(device)
435+
if hasattr(default_backend, "comm_split_count"):
436+
splits_before = default_backend.comm_split_count()
437+
torch.distributed.new_group(ranks=list(range(dist.get_world_size())))
438+
assert (default_backend.comm_split_count() > splits_before) == expect_bound
439+
440+
441+
class TestSingleRankDeviceId(DistributedTest):
442+
world_size = 1
443+
init_distributed = False
444+
445+
def test(self):
446+
assert_device_binding(expect_bound=False)
447+
448+
449+
class TestMultiRankDeviceId(DistributedTest):
450+
world_size = 2
451+
init_distributed = False
452+
453+
def test(self):
454+
assert_device_binding(expect_bound=True)

0 commit comments

Comments
 (0)