Skip to content

DeepSpeed Comm. Backend v1 - #1985

Merged
jeffra merged 24 commits into
masterfrom
staging-comms-next
Jun 10, 2022
Merged

DeepSpeed Comm. Backend v1#1985
jeffra merged 24 commits into
masterfrom
staging-comms-next

Conversation

@awan-10

@awan-10 awan-10 commented May 27, 2022

Copy link
Copy Markdown
Contributor

This PR introduces the DeepSpeed Comm. Backend (v1).

Current advanced communication schemes rely on mixing python-level communication packages (e.g. torch.distributed, mpi4py for 1-bit Adam). In order to simplify comms prototypes, we're looking to add support for custom communication backends within DeepSpeed built directly on top of their respective libraries (e.g. NCCL, MPI, etc).

This PR completes the first phase towards this goal by introducing:

  • The new comms interface deepspeed.comms
  • A complete wrapper around torch.distributed called TorchBackend for backwards-compatibility
  • A rough skeleton for custom backends that we can use for phase 2

Co-authored-by: Quentin Anthony qganthony@yahoo.com
Co-authored-by: Ammar Ahmad Awan ammar.awan@microsoft.com
Co-authored-by: Jeff Rasley jerasley@microsoft.com

Co-authored-by: Quentin Anthony <qganthony@yahoo.com>
Co-authored-by: Ammar Ahmad Awan <ammar.awan@microsoft.com>
Co-authored-by: Jeff Rasley <jerasley@microsoft.com>
Comment thread deepspeed/__init__.py Outdated
Comment thread deepspeed/comm/backend.py Outdated
Comment thread deepspeed/moe/sharded_moe.py Outdated
Comment thread deepspeed/runtime/comm/coalesced_collectives.py Outdated
Comment thread deepspeed/runtime/comm/coalesced_collectives.py Outdated
Comment thread deepspeed/runtime/engine.py Outdated
Comment thread deepspeed/runtime/engine.py
Comment thread deepspeed/runtime/pipe/module.py Outdated
Comment thread deepspeed/runtime/zero/partition_parameters.py Outdated
Comment thread op_builder/comm.py Outdated
Comment thread tests/comm/test.py Outdated

@awan-10 awan-10 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR with Quentin. He will take care of minor comments. @jeffra, please review this one.

@jeffra

jeffra commented May 31, 2022

Copy link
Copy Markdown
Collaborator

@SeanNaren can you take a look at the lightning test failure? We're seeing this on other PRs and in master right now as well. It appears to be a protobuf issue, have you seen this on your side before?

@Quentin-Anthony

Copy link
Copy Markdown
Contributor

@jeffra and @tjruwase -- Any further comments?

Comment thread deepspeed/comm/comm.py Outdated
Comment thread deepspeed/comm/comm.py Outdated
Comment thread deepspeed/comm/comm.py Outdated
Comment thread deepspeed/comm/comm.py
Comment thread deepspeed/comm/comm.py Outdated
Comment thread deepspeed/comm/comm.py Outdated
Comment thread deepspeed/runtime/zero/stage_1_and_2.py Outdated
Comment thread deepspeed/comm/comm.py
@deepspeedai deepspeedai deleted a comment from rocm-mici Jun 9, 2022
Comment thread deepspeed/comm/comm.py Outdated
@jeffra
jeffra merged commit 36ad311 into master Jun 10, 2022
@jeffra
jeffra deleted the staging-comms-next branch June 10, 2022 23:47
@jeffra
jeffra restored the staging-comms-next branch June 10, 2022 23:47
@mrwyattii
mrwyattii deleted the staging-comms-next branch July 7, 2023 02:39
banxingmjj pushed a commit to openanolis/DeepSpeed that referenced this pull request Aug 24, 2026
…ync on MPS (deepspeedai#8303)

## Summary

Resolves @delock's review note on deepspeedai#8293
(deepspeedai#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,
deepspeedai#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 deepspeedai#8301

deepspeedai#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants