Skip to content

Validate DistMuon parameter assignments before optimizer construction - #4816

Draft
weifengpy wants to merge 1 commit into
pr/dist-muon-stacked-shard0from
pr/dist-muon-assignment-validation
Draft

weifengpy wants to merge 1 commit into
pr/dist-muon-stacked-shard0from
pr/dist-muon-assignment-validation

Conversation

@weifengpy

Copy link
Copy Markdown
Contributor

A DistMuon selection regex can match some parameters while silently leaving others with compute layouts assigned to AdamW. The existing parameter-coverage check accepts that configuration because every parameter still has an optimizer.

Require every locally trainable parameter with a DistMuon compute layout to be assigned to DistMuon. Otherwise, raise ValueError naming the parameter and its assignment before constructing optimizers for that model part. Also catch mapped parameters omitted from every group, including when no DistMuon group exists. Canonicalize checkpoint-wrapped names and ignore frozen parameters and entries for other pipeline stages.

The optimizer container collects assignments and dispatches registered validators; the DistMuon-specific rule lives beside DistMuon. Keep existing string identifiers, taking DistMuon's identifier from DistMuon.__name__.

Stacked on #4811 to review the optimizer-container validation separately from Kimi's parameter selection and Shard(0) compute changes.

Validation:

  • All 23 existing optimizer parameter-group tests pass on the parent and this change. No test files are modified.
  • Standalone checks reject the old w[123] regex, metadata with only AdamW groups, and mapped unassigned parameters before optimizer construction. Wrapped valid assignments and frozen/off-stage entries are accepted.
  • Ten seed-42 deterministic CPU steps with mixed Adam/AdamW match the parent bitwise for loss, gradient norm, parameters, and optimizer state.
  • A previous eight-GPU deterministic C4 comparison of the valid Muon configuration with and without this startup validation matched all 10 full-precision training losses, gradient norms, and seven held-out losses.
  • Full pre-commit reports the same 30 existing Pyrefly errors; all other hooks pass. Focused type checks pass.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 21, 2026
@weifengpy
weifengpy force-pushed the pr/dist-muon-stacked-shard0 branch from 35e26a8 to 90ed00f Compare September 21, 2026 05:27
@weifengpy
weifengpy force-pushed the pr/dist-muon-assignment-validation branch from 5152b20 to 0e81a9f Compare September 21, 2026 05:27
@weifengpy
weifengpy force-pushed the pr/dist-muon-stacked-shard0 branch from 90ed00f to a13c8f7 Compare September 21, 2026 05:33
@weifengpy
weifengpy force-pushed the pr/dist-muon-assignment-validation branch from 0e81a9f to 55f9e03 Compare September 21, 2026 05:33
@weifengpy
weifengpy force-pushed the pr/dist-muon-assignment-validation branch from 55f9e03 to 4fd21a0 Compare September 21, 2026 07:23
@weifengpy
weifengpy force-pushed the pr/dist-muon-stacked-shard0 branch from a13c8f7 to 0a2a8a8 Compare September 21, 2026 07:23
@weifengpy
weifengpy force-pushed the pr/dist-muon-assignment-validation branch from 4fd21a0 to aa37c86 Compare September 21, 2026 19:43
A DistMuon selection regex can match some parameters while silently leaving others with compute layouts assigned to AdamW. The existing parameter-coverage check accepts that configuration because every parameter still has an optimizer.

Require every locally trainable parameter with a DistMuon compute layout to be assigned to DistMuon. Otherwise, raise ValueError naming the parameter and its assignment before constructing optimizers for that model part. Also catch mapped parameters omitted from every group, including when no DistMuon group exists. Canonicalize checkpoint-wrapped names and ignore frozen parameters and entries for other pipeline stages.

The optimizer container collects assignments and dispatches registered validators; the DistMuon-specific rule lives beside DistMuon. Keep existing string identifiers, taking DistMuon's identifier from `DistMuon.__name__`.

Stacked on #4811 to review the optimizer-container validation separately from Kimi's parameter selection and Shard(0) compute changes.

Validation:

- All 23 existing optimizer parameter-group tests pass on the parent and this change. No test files are modified.
- Standalone checks reject the old w[123] regex, metadata with only AdamW groups, and mapped unassigned parameters before optimizer construction. Wrapped valid assignments and frozen/off-stage entries are accepted.
- Ten seed-42 deterministic CPU steps with mixed Adam/AdamW match the parent bitwise for loss, gradient norm, parameters, and optimizer state.
- A previous eight-GPU deterministic C4 comparison of the valid Muon configuration with and without this startup validation matched all 10 full-precision training losses, gradient norms, and seven held-out losses.
- Full pre-commit reports the same 30 existing Pyrefly errors; all other hooks pass. Focused type checks pass.
@weifengpy
weifengpy force-pushed the pr/dist-muon-assignment-validation branch from aa37c86 to b31e4bf Compare September 21, 2026 19:46
@weifengpy
weifengpy force-pushed the pr/dist-muon-stacked-shard0 branch from 11dd361 to 074332c Compare September 21, 2026 19:46
weifengpy added a commit that referenced this pull request Sep 21, 2026
…#4811)

w13 didn't get any fqn match after splitting w123 into w13 and w2. this
PR fixes it:
  * #4535
  * https://github.com/pytorch/torchtitan/pull/4676/changes#r4058499709
* same problem first raised in
#4531 by @WhoisZihan

the next follow up is adding validation prevent future cases like above
in optimizer init: #4816


Co-authored-by: Zihan Yang <whois.zihan.yang@gmail.com>

Co-authored-by: Zihan Yang <whois.zihan.yang@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

1 participant