intel / intel/torch-xpu-ops

[distributed] ProcessGroupXCCL does not implement the c10d reconfigure API, so test_c10d_fault_tolerance.py has zero XPU coverage

Open
#5,409 0 comments 0 reactions 0 assignees View on GitHub
bug module: distributed
Dominant language
Python
Stars
113
Forks
128
Avg merge
5d 13h
Merged PRs (30d)
107

Description

### Describe the bug

`ProcessGroupXCCL` does not implement the c10d fault-tolerance / reconfigure API, so it
inherits `return false` from `Backend.hpp:232`:

```cpp
// supportsReconfigure advertises support; get_reconfigure_handle returns an
// opaque handle that peers exchange out-of-band; reconfigure (re)initializes
// the communicator with a new set of peers.
virtual bool supportsReconfigure() const {
return false;
}
```

Only `ProcessGroupGloo.hpp:278` and `nccl2/ProcessGroupNCCL.hpp:302` override it.
`grep -rniE "reconfigure" src/xccl/` in this repo returns nothing.

Worse, `enable_reconfigure=True` is **silently accepted** on XPU -- no error, no warning,
the capability simply reports false:

```bash
python - <<'EOF'
import torch, torch.distributed as dist, tempfile
f = tempfile.NamedTemporaryFile(delete=False)
dist.init_process_group("xccl", world_size=1, rank=0,
store=dist.FileStore(f.name, 1), enable_reconfigure=True)
pg = dist.distributed_c10d._get_default_group()
b = dist.get_backend_impl(pg, torch.device("xpu"))
print("backend:", type(b).__name__)
print("dist._supports_reconfigure():", dist._supports_reconfigure())
print("backend.supports_reconfigure:", b.supports_reconfigure)
EOF
```

```
backend: ProcessGroupXCCL
dist._supports_reconfigure(): False
backend.supports_reconfigure: False
```

Arguably a smaller bug in its own right: asking for `enable_reconfigure=True` on a backend
that cannot honour it should raise (or at least warn) rather than hand back a process group
that quietly lacks the capability.

### What XPU needs

1. `supportsReconfigure()` returning `true`, plus `getReconfigureHandle()` and
`reconfigure()` -- the handle is an opaque blob peers exchange out-of-band, and
`reconfigure()` must re-initialize the communicator against a new peer set (including
shrinking, merging, and recovering after `abort()`).
2. `get_error()` reporting `ErrorType::COMM_ERROR` after an `abort()` and
`ErrorType::TIMEOUT` after a collective times out (the tests assert both).
3. Work-result reporting (`Work::get_future_result()` -> `WorkResult::TIMEOUT` /
`COMM_ERROR`), which is what `FaultToleranceBackend.supports_work_result` gates.

### Test impact

`test/distributed/test_c10d_fault_tolerance.py` builds one test class per entry in

```python
FAULT_TOLERANCE_BACKENDS = [
FaultToleranceBackend("gloo", "cpu"),
FaultToleranceBackend("nccl2", "cuda", supports_work_result=True),
]
```

so on XPU (`python test/distributed/test_c10d_fault_tolerance.py -v`) the picture is:

```
Ran 34 tests in 42.110s
OK (skipped=19)
```

| Class | Collected | Pass | Skip |
|---|---|---|---|
| `GlooFaultToleranceTest` (cpu) | 16 | 13 | 3 (2x "gloo does not report work results", 1x nonblocking-NCCL-init) |
| `Nccl2FaultToleranceTest` (cuda) | 16 | 0 | 16 -- "fault tolerance CUDA tests require at least 3 GPUs" (`not TEST_CUDA`, line 392) |
| `ReconfigureContractTest` / `BackendCapabilityContractTest` | 2 | 2 | 0 (device-free) |

So all 16 accelerator-path tests are dead on XPU, and the only thing exercising reconfigure
is the CPU/gloo class.

Note this file is *less* forgiving than `test_c10d_window.py` (see #5407): it has no
capability-based self-skip. Adding `FaultToleranceBackend("xccl", "xpu",
supports_work_result=True)` today would make all 16 tests **hard-fail**, not skip, at the
two assertions in `_init_reconfigurable_pg` (lines 81-82):

```python
self.assertTrue(dist._supports_reconfigure())
self.assertTrue(self.backend.supports_reconfigure)
```

### What must pass to close this

With `("xccl", "xpu")` added to `FAULT_TOLERANCE_BACKENDS`, the generated
`XcclFaultToleranceTest` class must run all 16 tests green with no skips:

```bash
python test/distributed/test_c10d_fault_tolerance.py -v
```

- `XcclFaultToleranceTest::test_reconfigure_basic`
- `XcclFaultToleranceTest::test_reconfigure_then_all_reduce`
- `XcclFaultToleranceTest::test_reconfigure_then_send_recv`
- `XcclFaultToleranceTest::test_reconfigure_identity`
- `XcclFaultToleranceTest::test_reconfigure_scale_down_up`
- `XcclFaultToleranceTest::test_reconfigure_single_to_all`
- `XcclFaultToleranceTest::test_reconfigure_late_join`
- `XcclFaultToleranceTest::test_reconfigure_merge_split`
- `XcclFaultToleranceTest::test_shrink_exclude_last_rank`
- `XcclFaultToleranceTest::test_shrink_exclude_middle_rank`
- `XcclFaultToleranceTest::test_reconfigure_after_abort`
- `XcclFaultToleranceTest::test_reconfigure_after_timeout`
- `XcclFaultToleranceTest::test_reconfigure_rejects_reused_uuid`
- `XcclFaultToleranceTest::test_reconfigure_timeout_is_retryable`
- `XcclFaultToleranceTest::test_work_explicit_timeout_includes_prelaunch_stall`
- `XcclFaultToleranceTest::test_work_reports_communicator_error`

The last two are the ones needing item 3 above; the class-level gate at lines 390-393
(`not TEST_CUDA or torch.cuda.device_count() < 3`) also has to become accelerator-generic,
and the suite needs 3 devices.

### Note: PyTorch-side prep work

Independently of XCCL, the test file itself has to be made device-generic before an XPU
entry can work. These are PyTorch changes (part of pytorch/pytorch#114850), listed so the
two halves are not fixed in isolation:

- `device` property hardcodes `f"cuda:{self.rank}"` (lines 50-51) and `_init_reconfigurable_pg`
calls `torch.cuda.set_device` (lines 72-73)
- `test_work_explicit_timeout_includes_prelaunch_stall` uses `torch.cuda.synchronize()`,
`torch.cuda._sleep()` and `torch.cuda.current_stream().query()` (lines 154-166)
- `test_work_reports_communicator_error` uses `torch.cuda.synchronize()` (line 173) and
asserts the literal backend string `"NCCL operation failed"` (line 186)
- backend-name branches at lines 293, 336 and 358

### Versions

```
torch 2.15.0a0+git3c3da2a (source build, commit 3c3da2a5e8b)
device 4x Intel(R) Data Center GPU Max 1100
backend xccl
```

Contributor guide

Open the contributing guide

Research direction

Read Backend.hpp:232 and the XCCL sources under src/xccl/ to map the missing reconfigure, error, and work-result interfaces. Then run test/distributed/test_c10d_fault_tolerance.py -v and review the listed PyTorch-side device-generic changes and XPU test requirements. Done means an XcclFaultToleranceTest class runs all 16 tests green with no skips.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, python
Domain
distributed-systems, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.