Skip to content

Skip ZeRO DeepCompile backward hooks for AutoSP - #8667

Merged
tohtana merged 1 commit into
deepspeedai:masterfrom
jinyouzhi:fix-autosp-deepcompile-hooks
Sep 27, 2026
Merged

tohtana merged 1 commit into
deepspeedai:masterfrom
jinyouzhi:fix-autosp-deepcompile-hooks

Conversation

@jinyouzhi

Copy link
Copy Markdown
Contributor

What breaks

DeepSpeedEngine._backward_prologue() / _backward_epilogue() enter the ZeRO DeepCompile backward hooks whenever DeepCompile is active, exempting only the AutoTP compile pass:

if self.is_deepcompile_active() and not self.compile_autotp():
    deepcompile_backward_prologue(self.is_gradient_accumulation_boundary())

With compile.passes: ["autosp"], engine.compile() installs the AutoSP backend and marks DeepCompile active, so every backward calls deepcompile_backward_prologue(). That calls get_deepcompile_handle(), which loads (and JIT-builds on first use) the DeepCompile native extension, then calls start_backward() on it. AutoSP never initializes that runtime (dc.init() is only called on the ZeRO DeepCompile path), so on CUDA this silently loads an unused extension, and on accelerators that cannot build it, backward fails.

allreduce_gradients() already uses uses_parallelization_pass_only() (AutoSP or AutoTP) for the same decision; the backward hooks were missed when that helper was introduced.

Fix

Use uses_parallelization_pass_only() in both the backward prologue and epilogue checks. The AutoTP behavior is unchanged.

Tests

Added TestAutoSPEngineBackward::test_backward_skips_zero_deepcompile_hooks in tests/unit/v1/compile/test_compile_autosp.py. It builds a ZeRO-0 engine with compile.passes: ["autosp"], marks DeepCompile active as engine.compile() would, makes get_deepcompile_handle() and the post-backward hooks raise, and runs forward/backward/step.

result
master FAILED (RuntimeError: AutoSP backward must not load the ZeRO DeepCompile runtime)
this PR PASSED

Also ran locally: tests/unit/compile/test_zero3_grad_dtype.py, tests/unit/compile/test_backend.py, and the rest of tests/unit/v1/compile/test_compile_autosp.py (all IR-level tests pass). The end-to-end TestAutoSPCompile cases fail identically on master and this branch in my environment (torch 2.13, transformers 5.17) during Dynamo tracing of the forward pass, before backward is reached, so they are unrelated to this change.

The engine's backward prologue and epilogue only exempted the AutoTP
compile pass from the ZeRO DeepCompile hooks. With the AutoSP pass,
backward therefore called deepcompile_backward_prologue(), which loads
(and JIT-builds on first use) the DeepCompile native extension and calls
start_backward() on it, although AutoSP never initializes that runtime.
On accelerators that cannot build the extension, backward fails.

Use uses_parallelization_pass_only() for both checks, as
allreduce_gradients() already does, so neither parallelization pass
enters the ZeRO DeepCompile hooks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Jin, Youzhi <youzhi.jin@intel.com>
@tohtana tohtana mentioned this pull request Sep 27, 2026
10 of 22 tasks

@tohtana tohtana left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @jinyouzhi, this looks good to me. Thank you for the improvement!

@tohtana
tohtana enabled auto-merge September 27, 2026 21:23
@tohtana

tohtana commented Sep 27, 2026

Copy link
Copy Markdown
Collaborator

@jinyouzhi By the way, I noticed that this PR, #8674, and #8675 include some of the same changes as #8457. Sorry I wasn't able to review #8457 sooner. Are these PRs intended to supersede #8457, or are there remaining changes in #8457 that still need review?

@tohtana
tohtana added this pull request to the merge queue Sep 27, 2026
Merged via the queue into deepspeedai:master with commit 016fb76 Sep 27, 2026
13 of 15 checks passed
@jinyouzhi

jinyouzhi commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor Author

@jinyouzhi By the way, I noticed that this PR, #8674, and #8675 include some of the same changes as #8457. Sorry I wasn't able to review #8457 sooner. Are these PRs intended to supersede #8457, or are there remaining changes in #8457 that still need review?

Thank you very much! I split #8457 into separate individual PRs to make review easier! I will remove the duplicate parts in #8457.

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.

2 participants