Skip to content

Use the owner's DP x CP group for replica ids in SSM, MoE and TE layers - #8732

Closed
Connor-XY wants to merge 10 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-g9-ckpt-helper-groups
Closed

Connor-XY wants to merge 10 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-g9-ckpt-helper-groups

Conversation

@Connor-XY

@Connor-XY Connor-XY commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Depends on #8044. Please review the diff from the merge of #8044 into main: scoped diff.

Why. Called without metadata, sharded_state_dict() of these modules filled in the global DP x CP group:

  • the Mamba, GDN and GDP mixers and HybridStack;
  • the MoE expert classes TEGroupedMLP and SequentialMLP;
  • TELayerNormColumnParallelLinear.

A module on its own grid then computed replica ids over another grid's DP ranks, so several ranks, or none, wrote a shard. A model built from explicit groups could not save these modules without a global grid. #8044 lets modules pass their own group to ensure_metadata_has_dp_cp_group and uses it for the language model; these modules still used the default.

What.

  • Each module passes its collection's dp_cp_gtp_remat, or dp_cp when that is unset, to ensure_metadata_has_dp_cp_group. A group already in the metadata still wins.
  • HybridStack fills the metadata before it visits its layers. The GDP mixer hands its group to its gated norm, as it does the TP group.
  • The experts fill in the dense group, the one a model passes down. Their replica ids still take the DP entry from the expert data-parallel group.
  • TEGroupedMLP, SequentialMLP and TELayerNormColumnParallelLinear keep the collection they are built with.

Tests. With the global DP x CP accessor patched to raise, new tests compare (key, offset, replica_id) against the module's own group for a Mamba mixer, a hybrid stack, both expert classes and the TE layer:

  • tests/unit_tests/ssm/test_ssm_sharded_state_dict_dp_cp_group.py and tests/unit_tests/transformer/moe/test_experts_sharded_state_dict_dp_cp_group.py are new.
  • test_te_layernorm_column_parallel_linear.py also checks the TE layer on a TP=1 grid that differs from the global TP=2 grid, and without a collection. The no-collection test guards the global default and passes on Compute checkpoint replica ids over the module's own DP x CP group #8044 too.
  • test_gdp_tp_checkpoint.py covers the GDP mixer and its norm on CPU, including the dp_cp fallback. Its stand-in collection is now a ProcessGroupCollection, because the mixer reads both DP x CP fields.

The _GDNBase change has no test of its own; it is the same one-line change as the Mamba and GDP mixers.

On 4 GPUs against its base (main with #8044 merged): test_te_layernorm_column_parallel_linear.py: 8 passed here vs 5 on the base, and the 3 new tests pass. #8044's test_sharded_state_dict_dp_cp_group.py passes 6 tests on both sides. An earlier version of the experts test failed on both sides, because its config used a top-1 softmax router, which TransformerConfig rejects. It now routes each token to two experts and passes. Before the sync, the branch also ran on 4 CPU gloo ranks with no regressions; the 3 new GDP cases pass, the SSM tests need CUDA, and the TE and MoE tests skip without Transformer Engine. In the integration run (all #6307 PRs merged onto main), this PR's test files show no regressions against main, and every test in the four files passes on 4 GPUs: test_gdp_tp_checkpoint.py 8 passed vs 5 on main, test_te_layernorm_column_parallel_linear.py 8 vs 5, and the new SSM and experts files 2 each.

Compatibility.

  • Replica ids on the standard grid do not change. Modules built without a collection keep the global default. No new global reads.
  • The experts fill the dense DP x CP group, not expt_dp. This keeps standard-grid replica ids as they are and matches what a model passes down. The DP entry of each expert replica id already comes from expt_dp.
  • dp_cp_gtp_remat or dp_cp is evaluated even when the metadata already carries a group, and the GDP mixer reads both fields at construction. A duck-typed collection without these attributes, such as a SimpleNamespace, now raises AttributeError. No in-repo or Megatron-Bridge caller builds one.
  • Pass the owner's collection to FP8/FP4 quantization contexts #8035 adds the same self._pg_collection = pg_collection line to TELayerNormColumnParallelLinear.__init__, a few lines away. Git merges both without a conflict, so whichever PR lands second should drop its copy.
  • Merges cleanly with Give SequentialMLP experts the expert groups of their collection #8724, which edits the expert MLP construction next to this change in experts.py.
  • Not covered here: the MambaLayer, MoELayer and Router entry points without a collection, the MLA fused q/kv-down split and the global TP read in the Mamba mixer's ExtendedRMSNorm. Warn when checkpoint helpers fall back to the global process groups #8753 handles them. SharedExpertMLP.sharded_state_dict with metadata=None and the shared-expert gate still raises TypeError. No PR in [TRACKING] Deprecate parallel_state: explicit process groups in megatron/core #6307 fixes that yet.
  • Needs determinism-exempt: experts.py, transformer_engine.py, mamba_mixer.py, gated_delta_product.py and gated_delta_net/common.py are registered kernel files, and only checkpoint metadata groups change. While Compute checkpoint replica ids over the module's own DP x CP group #8044 is unmerged, its edit to the registered transformer/utils.py needs the label too.

Synced with main 77391e7 on 2026-11-01 by a merge commit that lists each conflict and its resolution, so existing commits and review links stay valid.

Part of #6307.

🤖 Generated with Claude Code

Connor-XY and others added 4 commits October 26, 2026 10:58
sharded_state_dict() called without metadata filled in the global
get_data_parallel_group(with_context_parallel=True), and replica ids
were computed over it. A model on its own grid, saved by a tool, a
converter or a test that passes no metadata, then claimed replicas by
another grid's DP ranks: several ranks or none wrote each shard.

ensure_metadata_has_dp_cp_group takes an optional dp_cp_group that it
uses instead of the global group when the metadata has none; a group
already in the metadata still wins. MegatronModule, MLP and
TransformerBlock pass their collection's dp_cp_gtp_remat (dp_cp when
the collection has no GTP-inclusive group). That is the group the
global default stands for, so replica ids on the standard grid do not
change. MLP now keeps its pg_collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
LanguageModule.sharded_state_dict fills missing metadata for GPT,
hybrid and BERT models before their submodules can, so a model built
on its own collection still got the global DP x CP group for every
replica id below it. Pass the model collection's dp_cp_gtp_remat
(dp_cp without GTP), the group the global default stands for.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
make_sharded_tensors_for_checkpoint, make_tp_sharded_tensor_for_checkpoint
and make_sharded_tensor_for_checkpoint fell back to the global groups
only when both groups were None. A tp_group passed without its
dp_cp_group made the DP x CP group a singleton, so every data-parallel
rank claimed to be the main replica of each shard. That is now a
ValueError. No in-repo caller passes a TP group alone.

A dp_cp_group without a tp_group still means the tensors are not
tensor-parallel: the GTP checkpoint helper, MIMO's sharded_state_dict
for plain submodules and the GTP checkpoint tests rely on it, so the
docstrings now say so instead of raising.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 30, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

Connor-XY and others added 4 commits October 30, 2026 13:14
The SSM, MoE and TE layers that follow pass their own DP x CP group to
ensure_metadata_has_dp_cp_group, which takes that argument from NVIDIA#8044,
and their tests reuse NVIDIA#8044's sharded-state DP x CP test helpers, so
merge NVIDIA#8044 here. NVIDIA#8044's head already contains main, so the merge is
clean and its tree is NVIDIA#8044's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Called without metadata, the Mamba, GDN and GDP mixers and the hybrid
stack filled in the global data-parallel group with context
parallelism. A layer on its own grid then computed replica ids over
another grid's DP ranks, so several ranks or none wrote each shard.

Each of them now fills the metadata with its collection's GTP-inclusive
DP x CP group (dp_cp_gtp_remat, or dp_cp when that field is unset), the
group the global default stands for. A group already in the metadata
still wins, and a collection without either field keeps the global
default. The hybrid stack fills the metadata before visiting its
layers, so a stack-level call reaches no global group either. The GDP
mixer hands its group to its gated norm, as it does with the TP group.

The GDP checkpoint test stubs now use a ProcessGroupCollection, whose
unset fields read as None, as the mixer expects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Called without metadata, TEGroupedMLP and SequentialMLP filled in the
global data-parallel group with context parallelism, so a model built
on explicit process groups could not checkpoint its experts without a
global grid.

Both now keep their collection and fill in its GTP-inclusive dense
DP x CP group (dp_cp_gtp_remat, or dp_cp when that field is unset):
the group the global default stands for and the one a model passes
down to its experts, so the result does not depend on whether the
call starts at the model or at the experts. The DP entry of each expert
replica id still comes from the expert data-parallel group.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Called without metadata, TELayerNormColumnParallelLinear filled in the
global data-parallel group with context parallelism, so a layer on its
own grid computed replica ids over another grid's DP ranks.

The layer now keeps the collection it is built with and fills in its
GTP-inclusive DP x CP group (dp_cp_gtp_remat, or dp_cp when that field
is unset). A layer built without a collection keeps the global
default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
@Connor-XY
Connor-XY force-pushed the yx/pstate-g9-ckpt-helper-groups branch from 66d0848 to a0b5b21 Compare November 1, 2026 01:37
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
The one-warning tests that follow check that a MoELayer built without a
collection gives its experts the GTP groups of the global grid, and
read them from experts.pg_collection. NVIDIA#8732's 645cd0d makes
SequentialMLP and TEGroupedMLP keep their collection; without it that
test case fails with "'SequentialMLP' object has no attribute
'pg_collection'". NVIDIA#8732 brings NVIDIA#8044.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY and others added 2 commits November 1, 2026 09:00
The process-group allowlist records the reads main added since this PR
was written.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- tests/unit_tests/extension/test_te_layernorm_column_parallel_linear.py:
  Main (NVIDIA#8246) adds a multi-line import of HAVE_TE,
  TELayerNormColumnParallelLinear and TENorm and a new parametrized
  test_rmsnorm_gamma_marker. The PR adds imports for parallel_state,
  ProcessGroupCollection and the dp/cp test helpers, plus the
  _all_ranks helper and TestShardedStateDictDpCpGroup. The result keeps
  both sides unchanged, with the imports in isort order and the PR's
  class after main's new test.

The process-group allowlist records the reads main added since this PR
was written.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
@Connor-XY

Copy link
Copy Markdown
Contributor Author

Superseded by #8897, which carries this change as commit 74f8cb4 under the same title, with "Originally #8732." in its message. Closing so review happens in one place. The branch stays on the fork.

@Connor-XY Connor-XY closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant