FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix NameError: print_dist is called but never imported in auto_tp by delock · Pull Request #8310 · deepspeedai/DeepSpeed · GitHub

Fix NameError: print_dist is called but never imported in auto_tp - #8310

Closed
delock wants to merge 1 commit into
masterfrom
gma/fix-autotp-print-dist
Closed

Fix NameError: print_dist is called but never imported in auto_tp#8310
delock wants to merge 1 commit into
masterfrom
gma/fix-autotp-print-dist

Conversation

delock commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

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)

[rank0]:   File "/root/DeepSpeed/deepspeed/module_inject/auto_tp.py", line 620, in register_replicated_grad_hooks
[rank0]:     print_dist(
[rank0]: NameError: name 'print_dist' is not defined

Triggers e.g. via the AutoTP equivalence check from
deepspeedai/DeepSpeedExamples#1008:

cd training/autotp_equivalence
bash run_gpu.sh 500 0,1,2,3    # tp=3 and tp=4 both die at initialize

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:

-from deepspeed.utils.logging import log_dist
+from deepspeed.utils.logging import log_dist, print_dist

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:

  • Before: tp=3 / tp=4 both NameError at deepspeed.initialize().
  • After: both widths run to completion and agree with the AutoTP=1 baseline —
===== AutoTP=3 (uneven 6/6/4) vs AutoTP=1 =====
steps=500 first_rel=6.74e-08 mean_rel=7.45e-07 worst_rel=2.28e-04 at step 291
OK: 500 steps agree within 1e-02
===== AutoTP=4 (even 4/4/4/4, control) vs AutoTP=1 =====
steps=500 first_rel=0.00e+00 mean_rel=2.38e-06 worst_rel=7.68e-04 at step 291
OK: 500 steps agree within 1e-02

pre-commit passes on the changed file (yapf, flake8, codespell, check-torchdist,
check-license, check-torchcuda, check-extraindexurl).

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>

chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da4aa35130

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 👍 / 👎.

delock commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

close as #8311 will fix it.

delock closed this Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

1 participant


Back | FazBrowse Home | New Git URL