Commit 9311fd5
authored
Route isend/irecv to nonblocking backend methods and stage them as async on MPS (#8303)
## Summary
Resolves @delock's review note on #8293
(#8293 (comment)):
`irecv` is asynchronous by contract and has no `async_op` parameter, so
the MPS CPU-staging wrapper must handle it explicitly.
Two fixes:
1. **`deepspeed/comm/comm.py`** — `isend`/`irecv` dispatched to the
*blocking* `cdb.send`/`cdb.recv` (since the original comm backend,
#1985). Callers got a blocking call and `recv`'s return value (the
source rank `int`) instead of a waitable handle, so
`dist.irecv(...).wait()` raised `AttributeError`. This affects every
backend, not just MPS — e.g. the 1-bit comm helpers
(`runtime/comm/{compressed,hccl,nccl}.py`) call
`dist.isend/irecv(...).wait()`. They now route to
`cdb.isend`/`cdb.irecv`.
2. **`deepspeed/comm/torch.py`** — with the routing fixed, the MPS
staging wrapper's copy-back decision (keyed on an `async_op` argument)
ran immediately for `irecv`, before the transfer completed. A new
`always_async` flag on `stage_on_cpu` defers the copy-back to the
handle's `wait()` for `isend`/`irecv`. `StagedWork.wait()` now also
returns the underlying work's wait result.
### Verified (M5 Max, macOS 26.3, torch 2.13)
- Real two-process gloo run with MPS tensors: on master, `dist.irecv`
returns an `int` and `.wait()` crashes; with this PR it returns a handle
and the buffer holds the correct payload after `wait()`.
- `DS_ACCELERATOR=mps pytest unit/comm/test_dist.py`: 10 passed
(multi-rank cases skip on 1 device).
- ZeRO-2/3 smoke training unaffected.
### Test
Adds `TestDistIsendIrecv` (world size 2) to the existing
`tests/unit/comm/test_dist.py`: rank 0 `isend`s, rank 1 `irecv`s, both
assert a waitable handle and verify the payload after `wait()`.
Backend-agnostic, so it exercises the routing fix on CUDA/CPU CI as
well.
### Relation to #8301
#8301 addresses the same note with a more extensive `StagedWork`
(futures, result identity restoration, weakref buffer tracking). This PR
makes the fix more concise and accurate: no current DeepSpeed users
calls `Work.result()`/`get_future()` on staged P2P ops, and the staged
CPU buffer for `isend` is kept alive by the deferred copy-back closure
until `wait()`. Huge Credit to @FU-max-boop for the thorough analysis of
the Work semantics and fixes.
---------
Signed-off-by: PKUWZP <zhipeng.rainbowserie@gmail.com>1 parent da3ca68 commit 9311fd5
4 files changed
Lines changed: 106 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
384 | 384 | | |
385 | 385 | | |
386 | 386 | | |
387 | | - | |
| 387 | + | |
388 | 388 | | |
389 | 389 | | |
390 | 390 | | |
391 | 391 | | |
392 | 392 | | |
393 | | - | |
| 393 | + | |
394 | 394 | | |
395 | 395 | | |
396 | 396 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
103 | 103 | | |
104 | 104 | | |
105 | 105 | | |
| 106 | + | |
106 | 107 | | |
107 | | - | |
| 108 | + | |
108 | 109 | | |
109 | | - | |
| 110 | + | |
110 | 111 | | |
111 | 112 | | |
112 | 113 | | |
113 | 114 | | |
114 | 115 | | |
115 | 116 | | |
116 | 117 | | |
117 | | - | |
| 118 | + | |
118 | 119 | | |
119 | 120 | | |
120 | 121 | | |
121 | 122 | | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
122 | 126 | | |
| 127 | + | |
| 128 | + | |
123 | 129 | | |
124 | 130 | | |
125 | 131 | | |
| |||
147 | 153 | | |
148 | 154 | | |
149 | 155 | | |
150 | | - | |
| 156 | + | |
151 | 157 | | |
152 | 158 | | |
153 | 159 | | |
| |||
427 | 433 | | |
428 | 434 | | |
429 | 435 | | |
430 | | - | |
| 436 | + | |
431 | 437 | | |
432 | 438 | | |
433 | 439 | | |
434 | 440 | | |
435 | | - | |
| 441 | + | |
436 | 442 | | |
437 | 443 | | |
438 | 444 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| 7 | + | |
7 | 8 | | |
8 | 9 | | |
9 | 10 | | |
| |||
65 | 66 | | |
66 | 67 | | |
67 | 68 | | |
68 | | - | |
69 | | - | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
70 | 72 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
128 | 128 | | |
129 | 129 | | |
130 | 130 | | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
131 | 219 | | |
132 | 220 | | |
133 | 221 | | |
| |||
0 commit comments