Skip to content

[kimi2.7] pass correct fsdp mesh for backend=spmd_types - #4063

Merged
weifengpy merged 7 commits into
gh/weifengpy/47/basefrom
gh/weifengpy/47/head
Aug 5, 2026
Merged

[kimi2.7] pass correct fsdp mesh for backend=spmd_types#4063
weifengpy merged 7 commits into
gh/weifengpy/47/basefrom
gh/weifengpy/47/head

Conversation

@weifengpy

@weifengpy weifengpy commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Stack from ghstack (oldest at bottom):

before: spmd_types exposes the dense DP axis as dp_shard, but Kimi 2.7 requested the legacy fsdp axis, causing Invalid mesh dim: 'fsdp' during initialization

after: explicitly tells FSDP that dp_shard or efsdp is the data-parallel shard axis

[ghstack-poisoned]
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 4, 2026
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@weifengpy weifengpy changed the title [Kimi] Resolve FSDP meshes for spmd_types [Kimi] pass correct fsdp mesh for backend=spmd_types Aug 5, 2026
@weifengpy
weifengpy marked this pull request as ready for review August 5, 2026 01:15
@weifengpy
weifengpy requested a review from shuhuayu August 5, 2026 01:16
@weifengpy weifengpy changed the title [Kimi] pass correct fsdp mesh for backend=spmd_types [kimi2.7] pass correct fsdp mesh for backend=spmd_types Aug 5, 2026

@shuhuayu shuhuayu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks for adding this. i did a debug test run and hit some errors, related to model.parallelize(). llama3 has done this check:

if parallelism.spmd_backend in ("full_dtensor", "spmd_types"):

can you do a test run for the debug variant to check further?

reduce_dtype=TORCH_DTYPE_MAP[training.mixed_precision_reduce],
reshard_after_forward_policy=parallelism.fsdp_reshard_after_forward,
pp_enabled=parallel_dims.pp_enabled,
dp_mesh_dims=None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why this change, maybe we can do this in a separate commit for qwen 3.5?

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.

from below, seems you are comfortable with dp_mesh_dims: DataParallelMeshDims | None, I can delete this qwen change

Comment thread torchtitan/distributed/fsdp.py Outdated
reshard_after_forward_policy: str = "default",
pp_enabled: bool = False,
*,
dp_mesh_dims: DataParallelMeshDims | None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we make it optional to be consistent with apply_fsdp_to_decoder, i.e., use default value None?

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.

done

@weifengpy
weifengpy marked this pull request as draft August 5, 2026 15:43
[ghstack-poisoned]
weifengpy added a commit that referenced this pull request Aug 5, 2026
Use the shared dense and sparse storage-mesh resolvers for the spmd_types backend and pass their explicit data-parallel dimensions to FSDP2. Preserve the existing mesh-selection path for the default backend.

Test Plan:
- Run Moonlight 16B-A3B for 6 steps on 8 B200 GPUs with DP8, EP4, spmd_types, local batch size 4, and sequence length 1024.


ghstack-source-id: 3d7cad7
Pull-Request: #4063
[ghstack-poisoned]
weifengpy added a commit that referenced this pull request Aug 5, 2026
Use the shared dense and sparse storage-mesh resolvers for the spmd_types backend and pass their explicit data-parallel dimensions to FSDP2. Preserve the existing mesh-selection path for the default backend.

Test Plan:
- Run Moonlight 16B-A3B for 6 steps on 8 B200 GPUs with DP8, EP4, spmd_types, local batch size 4, and sequence length 1024.


ghstack-source-id: 6ca5512
Pull-Request: #4063
[ghstack-poisoned]
weifengpy added a commit that referenced this pull request Aug 5, 2026
Use the shared dense and sparse storage-mesh resolvers for the spmd_types backend and pass their explicit data-parallel dimensions to FSDP2. Preserve the existing mesh-selection path for the default backend.

Test Plan:
- Run Moonlight 16B-A3B for 6 steps on 8 B200 GPUs with DP8, EP4, spmd_types, local batch size 4, and sequence length 1024.


ghstack-source-id: 3ab807e
Pull-Request: #4063
state_shardings={"weight": _REPLICATE_PARAM, "bias": _REPLICATE_PARAM},
in_src_shardings={"input": _REPLICATE_ACT},
in_dst_shardings={"input": _REPLICATE_ACT},
out_src_shardings=_REPLICATE_ACT,

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.

required once model.parallelize() runs under DP-only spmd_types

@weifengpy
weifengpy marked this pull request as ready for review August 5, 2026 17:00
@weifengpy

Copy link
Copy Markdown
Contributor Author

can you do a test run for the debug variant to check further?

fixed. added integration test to catch this but let me know if you want such test

@weifengpy
weifengpy requested a review from shuhuayu August 5, 2026 17:01

@shuhuayu shuhuayu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm! maybe @pianpwk can have a final check about the spmd type checks, is multimodal tensor check solved now?

@pianpwk

pianpwk commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

lgtm! maybe @pianpwk can have a final check about the spmd type checks, is multimodal tensor check solved now?

hmm, does this pass with spmd typechecking? or is it just enabling the spmd backend. I can take the typechecking portion if not.

@shuhuayu

shuhuayu commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

lgtm! maybe @pianpwk can have a final check about the spmd type checks, is multimodal tensor check solved now?

hmm, does this pass with spmd typechecking? or is it just enabling the spmd backend. I can take the typechecking portion if not.

no, spmd type checking is deliberately ignored, see here:

if backend == "spmd_types" and any(

so this is only for enablement.

OverrideDefinitions(
[
[
# Do not enable --debug.spmd_typechecking: multimodal pixel

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.

@pianpwk sorry, no spmd_typechecking this time

@weifengpy
weifengpy merged commit d73017a into gh/weifengpy/47/base Aug 5, 2026
12 of 13 checks passed
@weifengpy

Copy link
Copy Markdown
Contributor Author

oops, this wasn't merging into main
@shuhuayu I created a new PR with the same change for main, but need your approval: #4070

weifengpy added a commit that referenced this pull request Aug 5, 2026
clone of a previous approval
#4063

before: spmd_types exposes the dense DP axis as dp_shard, but Kimi 2.7
requested the legacy fsdp axis, causing Invalid mesh dim: 'fsdp' during
initialization

after: explicitly tells FSDP that dp_shard or efsdp is the data-parallel
shard axis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/8gpu CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants