| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
register_replicated_grad_hooks() logs its summary via print_dist, which exists in deepspeed/utils/logging.py but was not imported in deepspeed/module_inject/auto_tp.py. Any model whose HuggingFace tp_plan contains replicated_with_grad_allreduce entries (e.g. Qwen3's q_norm / k_norm with recent transformers) hits NameError at deepspeed.initialize() whenever tensor-parallel size > 1. Verified with the AutoTP equivalence check from deepspeedai/DeepSpeedExamples#1008 (Qwen3-0.6B, tp=3 uneven / tp=4 even, 500 steps each): tp=3 and tp=4 both crashed at initialize before this fix and complete with agreeing loss curves after it. Same missing import is fixed in passing on the #8241 branch; this is the minimal standalone hotfix so the crash is not blocked on that refactor. Signed-off-by: Guokai Ma <guokai.ma@intel.com>
There was a problem hiding this comment.
Here are some automated review suggestions for this pull request.
Reviewed commit: da4aa35130
ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Sorry, something went wrong.
| from deepspeed.module_inject.tp_shard import get_shard_size, get_shard_size_list | ||
| from deepspeed.utils import groups | ||
| from deepspeed.utils.logging import log_dist | ||
| from deepspeed.utils.logging import log_dist, print_dist |
There was a problem hiding this comment.
Add the required Signed-off-by trailer
This is a non-merge commit, but its commit message has no Signed-off-by trailer, so it does not meet the repository's mandatory DCO requirement and may be rejected by commit/CI policy. Recreate the commit with git commit --signoff using the configured author identity.
AGENTS.md reference: AGENTS.md:L6-L8
Useful? React with 👍 / 👎.
Sorry, something went wrong.
|
close as #8311 will fix it. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Motivation
register_replicated_grad_hooks() in deepspeed/module_inject/auto_tp.py logs its
summary through print_dist, which exists in deepspeed/utils/logging.py but was
never imported in this module. As a result, any model whose HuggingFace tp_plan
contains replicated_with_grad_allreduce entries crashes with NameError at
deepspeed.initialize() whenever tensor-parallel size > 1 — before a single
training step runs.
This regressed in #8185, which introduced the grad-allreduce hooks path. With recent
transformers, Qwen3's tp_plan marks q_norm / k_norm as
replicated_with_grad_allreduce, so it is hit by simply launching a TP run of Qwen3.
Reproducer (before this fix)
Triggers e.g. via the AutoTP equivalence check from
deepspeedai/DeepSpeedExamples#1008:
Description of changes
Add print_dist to the existing logging import — one line, matching the fix already
carried in passing by the #8241 refactor branch, so the crash is not blocked on that
larger review:
print_dist (rather than log_dist) matches the call site's intent: its docstring
says to use it when log level should not decide whether the message is printed, which
fits this one-line setup diagnostic.
Validation
Ran the Examples#1008 equivalence suite (Qwen3-0.6B, fp32, 500 steps per width,
4x RTX 4080, transformers 5.14.0.dev0) against this branch:
pre-commit passes on the changed file (yapf, flake8, codespell, check-torchdist,
check-license, check-torchcuda, check-extraindexurl).