Skip to content

Restore BC for the tensor-parallel API - #48300

Merged
ArthurZucker merged 28 commits into
mainfrom
fix-tp-plan-bc
Aug 26, 2026
Merged

ArthurZucker merged 28 commits into
mainfrom
fix-tp-plan-bc

Conversation

@ArthurZucker

@ArthurZucker ArthurZucker commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

CPU CI GPU run-slow

Restore tp_plan from from_pretrained() and add deprecation cycle to it
More guarding regarding qk_rope_head_dim attributes in MLA + expert dim floor-divided by mesh size
Doc updates

`from_pretrained` accepts `tp_plan` and `tp_size` again, a supplied plan is
applied to the model instead of being ignored, embeddings are shardable when
the weights are untied, `DistributedConfig` is exported publicly, the renamed
parallel-style classes keep their old names, and an mps host falls back to
cpu/gloo rather than refusing to run.
An unsupported style in a config-derived plan was skipped instead of raising,
the expert dimension was floor-divided by the mesh size with no divisibility
check, and MLA lost its actionable error for a config without
`qk_rope_head_dim`.
Comment thread src/transformers/distributed/configuration_utils.py Outdated
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Comment thread src/transformers/distributed/mixin.py Outdated
Comment on lines +75 to +87
# Shard input embeddings for both tied and untied models. Note that architectures use different names for input embeddings.
if self._tp_plan:
try:
input_embeddings = self.get_input_embeddings()
except NotImplementedError:
input_embeddings = None
if isinstance(input_embeddings, nn.Embedding):
input_embedding_name = next(
(name for name, module in self.named_modules() if module is input_embeddings), None
)
if input_embedding_name:
self._tp_plan.setdefault(input_embedding_name, "embedding_rowwise")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is ai slope

Comment thread src/transformers/distributed/mixin.py Outdated
Comment on lines +135 to +145

def _matches(pattern: str) -> bool:
regex_pattern = pattern.replace("*", r"\d+")
return any(re.match(regex_pattern, name) for name in model_param_names)

prefix = getattr(self, "base_model_prefix", "")
if prefix:
plan = {
f"{prefix}.{k}" if not _matches(k) and _matches(f"{prefix}.{k}") else k: v for k, v in plan.items()
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is also ai slope

Comment on lines +792 to +796
if style not in ALL_PARALLEL_STYLES:
raise ValueError(
f"Unsupported tensor parallel style '{style}' for layer '{layer_pattern}'. "
f"Supported styles are {list(ALL_PARALLEL_STYLES.keys())}"
)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

no, collect all errors, then value error with all unrecognized

Comment on lines +74 to +76
logger.warning_once(
"PyTorch's built-in DeviceMesh/DTensor stack does not support an MPS mesh. Falling back to CPU."
)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ok

Comment on lines -351 to -354
if getattr(self, "tie_word_embeddings", False) and self.base_model_tp_plan is not None:
self.base_model_tp_plan = {
**self.base_model_tp_plan,
"embed_tokens": "embedding_rowwise",

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This was good, if not present and tie, keep it

Comment on lines +4198 to +4202
warnings.warn(
"Passing `tp_plan` directly to `from_pretrained` is deprecated and will be removed in v5.18. "
"Pass it in `distributed_config=DistributedConfig(tp_plan=...)` instead.",
FutureWarning,
stacklevel=2,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

nice

@github-actions

Copy link
Copy Markdown
Contributor

CI recap

Dashboard: View test results in Grafana
Latest run: 32953452821:2
Result: failure | Jobs: 1 | Tests: 35 | Failures: 0 | Duration: 7m 13s

model = AutoModelForImageTextToText.from_pretrained(
"baidu/ERNIE-4.5-VL-28B-A3B-PT",
device_map="auto", # Use tp_plan="auto" instead to enable Tensor Parallelism!
device_map="auto", # Pass distributed_config=DistributedConfig(tp_size=N) instead to enable TP!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is not ideal. It's annoying to write.
Can we either:

  • keep tp plan auto
  • auto detect torch run no device map -> tp plan auto?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah but your suggestions dont take into account other parallelism. I agree that having tp_plan=auto works wonder but if you want to combine with fsdp or pp, then DistributedConfig(tp_size=2, fsdp_size=2, pp_size=2) makes more sense

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

what I am saying and what you are saying are not against one another! its more about good defaults! DP is never used in inference, PP can be yes -> we would ideally want the torch run command to be caught and auto infer tp size and pp size (inter / intra)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

okay let's say we go for DistributedConfig(tp_plan=auto, pp_plan=auto) and WORLD_SIZE=16,

we would intuitively infer tp_size=8 and pp_size=2 but if some reasons, someone would want to do tp_size=4 and pp_size=4, we loose the ability to do so since we auto infer.

On top of that, it's heavily dependant on the infra but sometimes PP can be faster than TP. Here some benchmark Ive ran back in the days (Same DP, batch settings, nodes, and GPU count. PP is 10.8% faster)

PP: dp16_tp1_pp4_mbs1_ga64 — 5211 tok/s/GPU (source: https://huggingface.co/datasets/nanotron/picotron_bench/tree/main/result_blog_posts/saturate_dp_7b_8_node_pp/dp16_tp1_pp4_mbs1_ga64_sl4096)
TP: dp16_tp4_pp1_mbs1_ga64 — 4705 tok/s/GPU (source: https://huggingface.co/datasets/nanotron/picotron_bench/tree/main/result_blog_posts/saturate_dp_7b_8_node_tp/dp16_tp4_pp1_mbs1_ga64_sl4096)

Comment thread docs/source/en/model_doc/llama4.md Outdated
Comment on lines +372 to +375
distributed_config = DistributedConfig(tp_size=int(os.environ["WORLD_SIZE"]))
model = Llama4ForConditionalGeneration.from_pretrained(
model_id,
tp_plan="auto",
device_map="auto",
distributed_config=distributed_config,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

not a good default at all. why force the use to compute world size, import os etc when a single simple tp plan was enough before

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can use DistributedConfig(tp_plan=auto) if you want but with other parallelism, that would be weird to doDistributedConfig(tp_plan=auto, fsdp_size=2, pp_size=2) for example

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

absolutely, but we need to remove friction for the more common use case : inference!

@ArthurZucker
ArthurZucker merged commit f0e42ef into main Aug 26, 2026
108 of 109 checks passed
@ArthurZucker
ArthurZucker deleted the fix-tp-plan-bc branch August 26, 2026 11:54
@vasqu vasqu added the for patch Tag issues / labels that should be included in the next patch label Aug 26, 2026
vasqu pushed a commit that referenced this pull request Aug 26, 2026
* Restore backward compatibility for the tensor-parallel API

`from_pretrained` accepts `tp_plan` and `tp_size` again, a supplied plan is
applied to the model instead of being ignored, embeddings are shardable when
the weights are untied, `DistributedConfig` is exported publicly, the renamed
parallel-style classes keep their old names, and an mps host falls back to
cpu/gloo rather than refusing to run.

* Restore the plan validation the refactor turned into silent skips

An unsupported style in a config-derived plan was skipped instead of raising,
the expert dimension was floor-divided by the mesh size with no divisibility
check, and MLA lost its actionable error for a config without
`qk_rope_head_dim`.

* tp_plan deprecation warning

* warning

* better

* bc modeling utils

* bette rnaming

* doc

* doc

* let mixin handle the tied untied of embedding

* revert ai slop

* revert

* collect then raise

* update doc

* todo

* revert

* revert doc

---------

Co-authored-by: Ferdinand Mom <47445085+3outeille@users.noreply.github.com>
Co-authored-by: 3outeille <ferdinand.mom@epita.fr>
sbucaille pushed a commit to sbucaille/transformers that referenced this pull request Sep 16, 2026
* Restore backward compatibility for the tensor-parallel API

`from_pretrained` accepts `tp_plan` and `tp_size` again, a supplied plan is
applied to the model instead of being ignored, embeddings are shardable when
the weights are untied, `DistributedConfig` is exported publicly, the renamed
parallel-style classes keep their old names, and an mps host falls back to
cpu/gloo rather than refusing to run.

* Restore the plan validation the refactor turned into silent skips

An unsupported style in a config-derived plan was skipped instead of raising,
the expert dimension was floor-divided by the mesh size with no divisibility
check, and MLA lost its actionable error for a config without
`qk_rope_head_dim`.

* tp_plan deprecation warning

* warning

* better

* bc modeling utils

* bette rnaming

* doc

* doc

* let mixin handle the tied untied of embedding

* revert ai slop

* revert

* collect then raise

* update doc

* todo

* revert

* revert doc

---------

Co-authored-by: Ferdinand Mom <47445085+3outeille@users.noreply.github.com>
Co-authored-by: 3outeille <ferdinand.mom@epita.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

for patch Tag issues / labels that should be included in the next patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants