Repository navigation
Conversation
funasr/bin/train.py, funasr/bin/train_ds.py and funasr/train_utils/trainer_ds.py started every distributed process with torch.cuda.set_device(local_rank) / model.cuda(local_rank) and pinned the process-group backend to "nccl", and only released the allocator cache when device.type == "cuda". On a PyTorch build without CUDA (for example Ascend NPU) the first call raises AttributeError: module 'torch._C' has no attribute '_cuda_setDevice', and init_process_group(backend="nccl") fails with "Distributed package doesn't have NCCL built in", so DDP/FSDP training cannot start at all. Add device-agnostic helpers to funasr/train_utils/device_funcs.py and use them in the three entry points: the active accelerator (CUDA, Ascend NPU or XPU) is detected, the matching set_device is called, "hccl" becomes the default backend for NPU, and the end-of-epoch cache release dispatches to the accelerator that owns the device. CUDA behaviour is unchanged (nccl, cuda:<rank>, torch.cuda.empty_cache). Verified on Ascend 910B4 with a real 2-rank torchrun/HCCL run: before -> set_device AttributeError + nccl unavailable; after -> device_type=npu, resolved npu:0 / npu:1, set_device ok, backend=hccl, all_reduce sum 3.0 (expected 3.0), cache release ok. Not tested: CUDA/XPU hardware, FSDP/DeepSpeed end-to-end training. Signed-off-by: li-lizhe <147392333@qq.com>
LauraGPT
left a comment
There was a problem hiding this comment.
The CUDA compatibility path is sound in the scope I could independently exercise: at exact head ba0746849dde38ff8c115848165ebc700215a096, the six submitted device-helper tests pass, and a real H100 run with PyTorch 2.8.0+cu128 completed NCCL initialization, set_accelerator_device(0), an all-reduce, and DDP forward/backward on cuda:0.
One advertised accelerator path still needs a fix before merge. get_accelerator_device_type() can select "xpu", but get_default_dist_backend() returns "nccl" for every non-NPU device. Both training entry points therefore call init_process_group(backend="nccl") when XPU is active. In the same PyTorch 2.8 environment, torch.distributed.Backend.XCCL and is_xccl_available are present; a focused exact-head regression expecting the XPU default to be xccl fails with AssertionError: 'nccl' != 'xccl'.
Please select the repository-supported XPU backend (capability-aware if needed), retain the explicit backend= override, and add an XPU backend-selection case to tests/test_distributed_device_setup.py. The author's Ascend 2-rank evidence remains useful, but I have not independently reproduced NPU, XPU, full FSDP, or DeepSpeed hardware paths. Prepared with Codex assistance.
get_default_dist_backend() returned "nccl" for every non-NPU device, so an XPU run -- which get_accelerator_device_type() does select -- reached init_process_group(backend="nccl"). The default is now resolved per accelerator: HCCL on Ascend NPU, XCCL on XPU (the former oneCCL name "ccl" is kept for builds that confirm it through torch.distributed.is_ccl_available()), and the historical "nccl" elsewhere. Candidates are probed with torch.distributed.is_<backend>_available() where the build exposes it, and a non-CUDA accelerator never falls back to NCCL. An explicit backend=... still wins. tests/test_distributed_device_setup.py covers the XPU default and the capability-aware fallback (7 tests). Signed-off-by: li-lizhe <147392333@qq.com>
|
Thanks for the review and for independently exercising the CUDA path at the exact head. Fixed in
Candidates are probed with
Re-verified on the Ascend hardware after the change (2-rank XPU hardware is still not available here, so the XCCL selection itself is unit-tested only, not exercised on a device. |
LauraGPT
left a comment
There was a problem hiding this comment.
The backend-selection defect from the previous review is fixed at exact head da0e18cd9e3a4603562bb74bd66750d0d9022535: all seven submitted device-helper tests pass, including the XPU/XCCL selection and capability-aware fallback cases. I also reran the updated exact tree on a real NVIDIA H100 80GB HBM3 with PyTorch 2.8.0+cu128 / CUDA 12.8; NCCL initialization, set_accelerator_device(0), all-reduce, DDP forward/backward, and cache dispatch all completed on cuda:0.
The remaining merge gate is the newly changed XPU runtime path. The patch advertises XPU support and now chooses XCCL (or positively detected legacy CCL), but neither the author nor this review has exercised the exact head on Intel XPU hardware. Under the repository's current hardware-validation requirement, mock/unit backend selection and an unrelated CUDA GPU cannot substitute for the affected accelerator.
Please provide an exact-head Intel XPU run using the same entry-point order, preferably a 2-rank torchrun with XCCL, showing per-rank binding (xpu:0 / xpu:1), all-reduce, DDP forward/backward, and cache dispatch, plus exact PyTorch/oneAPI/device details. The reported exact-head 2-rank Ascend HCCL evidence is retained but was not independently reproduced here. This review does not cover full FSDP or DeepSpeed training. Prepared with Codex assistance.
Summary
funasr/bin/train.py,funasr/bin/train_ds.pyandfunasr/train_utils/trainer_ds.pybind every distributed process with CUDA-only calls:On a PyTorch build without CUDA — for example Ascend NPU — a distributed run cannot start:
torch.cuda.set_device(0)->AttributeError: module 'torch._C' has no attribute '_cuda_setDevice'init_process_group(backend="nccl")->RuntimeError: Distributed package doesn't have NCCL built indevice.type == "cuda", so on NPU/XPU it is a silent no-op.What changed
New helpers in
funasr/train_utils/device_funcs.py(no new dependency):get_accelerator_device_type()— probes CUDA, then Ascend NPU, then XPU,"cpu"otherwise (same backend orderAutoModelalready uses for its device fallback).accelerator_device(local_rank)/set_accelerator_device(local_rank)— bind the process tolocal_rankon the active accelerator.get_default_dist_backend()—"hccl"on NPU,"xccl"on XPU (legacy"ccl"reserved for builds that report it), the historical"nccl"elsewhere. Capability-aware throughtorch.distributed.is_<backend>_available(), still overridable viabackend=....clear_accelerator_cache(device)— dispatchempty_cache()to the accelerator that owns the device.The three entry points now call these. CUDA behaviour is unchanged (
nccl,cuda:<rank>,torch.cuda.empty_cache()).Verification (Ascend 910B4, real machine)
Real 2-rank
torchrun+ HCCL,torch 2.15.0.dev20260917+cpu/torch_npu 2.15.0.dev20260917, physical cards 6-7:set_deviceAttributeError: module 'torch._C' has no attribute '_cuda_setDevice'(both ranks)torch.npu.current_device()= 0 / 1init_process_groupRuntimeError: Distributed package doesn't have NCCL built inbackend=hcclall_reduce, 2 ranks3.0== expected3.0device.type != "cuda")Also adds
tests/test_distributed_device_setup.py(unittest, 7 cases, no accelerator needed): it stubs the accelerator modules and pins the resolution order, per-rank binding, backend choice (including the XPU default) and cache dispatch.python -m pytest tests/test_distributed_device_setup.py-> 7 passed on a CPU-only build.Re-checked at the current head
da0e18cd(2-ranktorchrunon 2x Ascend 910B4, torch 2.14.0 + torch_npu 2.14.0, cards 6-7):device_type=npu,backend=hccl, rank 0/1 onnpu:0/npu:1,all_reducesum3.0;python -m unittest tests.test_distributed_device_setup-> 7 passed. XPU hardware is still unavailable here, so the XCCL selection is unit-tested only.Not tested