Skip to content

Warn when the optimizer falls back to the global process groups - #8752

Closed
Connor-XY wants to merge 101 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-c5-optimizer-fallbacks
Closed

Connor-XY wants to merge 101 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-c5-optimizer-fallbacks

Conversation

@Connor-XY

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

Copy link
Copy Markdown
Contributor

Depends on #6259, #6303 (which contains #6293), #8017, #8043 and #8049. Please review the diff from the merge of their heads: scoped diff. The base takes #8017's update that merges #6259 and sets the Gloo fields of build_test_pg_collection. #6303 is in the base because the test-caller commit uses ProcessGroupCollection in tests/unit_tests/dist_checkpointing/utils.py and tests/unit_tests/training/test_param_norm.py, which import it only with #6303. The base's process-group allowlist records the 172 reads of its tree, without the T5Model shim read that #6303 removes.

Land this after #8738. With Gloo groups on, the default, setup_model_and_optimizer passes no collection to get_megatron_optimizer. Without #8738, stock training would warn once there, and so would test_training.py, test_fp8_param.py, test_fp4_param.py and test_cuda_graphs.py.

Why. Three optimizer paths read the global parallel_state groups without a warning when the caller omits them:

  • get_megatron_optimizer without pg_collection builds one with use_mpu_process_groups();
  • clip_qk without dp_cp_group reduces the attention logits over the global DP×CP group;
  • Float16OptimizerWithFloat16Params.sharded_state_dict rebuilds the checkpoint entry of a GTP-remat shard that has no model entry, such as the Mamba and GDP in_proj weights under distributed Muon. The rebuild read the global TP and DP groups, even for an optimizer built from a collection.

A model on its own grid, such as a MIMO module, then sharded its optimizer state, clipped its weights or described its shards with another model's ranks. Without a global grid, the rebuild hit an assertion.

What.

  • get_megatron_optimizer and clip_qk fall back through resolve_process_groups. It warns once with a ProcessGroupFallbackWarning (deprecated in 0.21, removed in 0.23) and returns the same groups as before:
    • get_megatron_optimizer builds every collection field, as use_mpu_process_groups() did. Several of the optimizers' readers treat an unset field differently from one set to None.
    • clip_qk resolves dp_cp_gtp_remat, and only when a layer has logits to reduce.
  • Float16OptimizerWithFloat16Params takes a keyword-only pg_collection, which get_megatron_optimizer and LayerWiseDistributedOptimizer pass. The GTP rebuild takes tp from it, plus dp_cp_gtp_remat or, when that is unset, dp_cp, as the model's own entries do. A collection without tp, or with neither DP×CP group, raises ValueError. Without a collection, the rebuild warns and uses the global groups. It still resolves groups only when a shard needs a rebuild.
  • 46 unit-test calls to get_megatron_optimizer and get_megatron_muon_optimizer pass ProcessGroupCollection.use_mpu_process_groups(). That is the collection the fallback builds, so these tests run on the same groups without the warning. Two of these calls are in main's new test_shortcut_moe_training_and_clipping, which the sync merge adapts so that it does not hit the warned fallback. The initializer-cleanup tests pass an empty collection instead of stubbing use_mpu_process_groups().
  • The checker goes from 172 reads to 171. The three fallbacks become guarded reads, and the rebuild reads one collection instead of two accessors.

Tests.

  • New tests/unit_tests/optimizer/test_optimizer_process_groups.py:
    • the factory builds on the collection it is given, with global reads forbidden and the warning made an error;
    • the layer-wise optimizer hands its collection to the optimizers it wraps;
    • without a collection, the factory warns and builds the same collection as use_mpu_process_groups();
    • the GTP rebuild uses the collection's groups for four field combinations and rejects a collection without a DP×CP group;
    • the rebuild warns without a collection, and needs no groups when every shard has a model entry.
  • test_qk_clip.py: a passed group is used with no global read. An omitted group warns and reduces over the same global group. A model without logits needs no group.

On 4 GPUs against its base, 22 of the 23 test files that exercise this change finish. In those 22, no test that passes on the base fails here. Across the 19 files with a pytest summary, 642 tests pass here vs 628 on the base. The 12 tests of the new test_optimizer_process_groups.py pass. test_qk_clip.py renames one test and adds one: 3 pass here vs 2 on the base. One test in test_layer_wise_optimizer.py fails on the base and passes here. The three generalized_tensor_parallel test files run no tests on either side; they skip because the container's Transformer Engine is older than GTP needs. distributed/mfsdp_v2/test_mcore_hybrid.py timed out on both sides. 78 tests in dist_checkpointing/test_optimizer.py fail on both sides because they need 8 ranks; they pass on 8 GPUs (two nodes) in the integration run. The two CUDA-graph end-to-end cases in distributed/mfsdp_v1/test_mcore_fully_sharded_data_parallel.py fail on both sides, the same way as on main.

In the integration run, which merges all #6307 PRs onto main 77391e7, this PR's test files show no regressions against main on 4 GPUs and 8 GPUs (two nodes). test_mcore_hybrid.py timed out there on both sides as well. End-to-end training smokes on that tree (22 configurations on 4 GPUs) give bit-identical losses to main and record no process-group fallback warning. Before the sync, the branch also ran on 4 CPU gloo ranks, one test file at a time, with no regressions. On CPU, 9 tests in the new file pass and 3 need CUDA. The checker tests pass.

Compatibility.

  • On the standard grid the groups are the same, so checkpoints do not change. The new constructor argument is optional and keyword-only. GraphedGradCopyFloat16Optimizer forwards it.
  • The rebuild checks its groups at the first checkpoint save that needs a rebuild, not at construction. Such a save needs GTP, distributed Muon and Mamba or GDP in_proj. setup_process_groups_for_optimizer accepts a collection with neither dp_cp nor dp_cp_gtp_remat when CP is 1, and the rebuild then raises. The DP×CP group does not change the rebuilt entry, because a GTP shard elects its writer over the group stamped on the parameter.
  • Without a collection, the rebuild resolves dp_cp along with tp and dp_cp_gtp_remat. That is one global group more than before, and the standard grid never uses it.
  • The rebuild warns under the name Float16OptimizerWithFloat16Params, also for GraphedGradCopyFloat16Optimizer.
  • Not changed here: LayerWiseDistributedOptimizer reads expt_dp through getattr(..., None). A collection without it treats the expert data-parallel axis as size 1 and passes None, which torch.distributed reads as WORLD, to the expert all-gather. That predates this PR, and every in-repo collection sets expt_dp.
  • The test-caller commit touches 19 test files across several CODEOWNERS groups. Should it move into its own PR?
  • Needs determinism-exempt: optimizer/__init__.py and optimizer/optimizer.py are registered kernel files. The changes there are group plumbing and warnings; numerics do not change.

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 30 commits October 7, 2026 16:02
Carry the optional dense and expert optimizer Gloo groups in explicit
collections and resolve the legacy global collection at the public optimizer
factory boundary. Preserve the disabled-Gloo path and permit collection
construction when Gloo groups were not created.

Refresh against current main, preserve the nested optimizer regressions,
and cover optional accessor checks plus Gloo presence and enablement.

Signed-off-by: Yan Xu <yxu1@nvidia.com>
…o groups

With use_gloo_process_groups=True, a process-group collection without Gloo
groups used to fall back silently to running the distributed optimizer without
Gloo. The global path raised in that case before this change. Raise ValueError
instead, except under GTP_remat, which never builds Gloo groups.

Signed-off-by: Yan Xu <yxu1@nvidia.com>
get_megatron_optimizer resolves the collection before calling its helpers, so a
missing collection there is a caller error. Report it with ValueError instead of
an assert, which is skipped under python -O. The previous hasattr(tp) check
could never fail, because ProcessGroupCollection returns None for unset fields,
so it is dropped.

Signed-off-by: Yan Xu <yxu1@nvidia.com>
get_megatron_optimizer now resolves a missing pg_collection with
ProcessGroupCollection.use_mpu_process_groups() up front, so the default path
goes through the explicit-collection branch of
setup_process_groups_for_optimizer. That branch returned no intra_dist_opt
group unless ddp_config.use_distributed_optimizer was set. Megatron-FSDP
leaves that flag False while OptimizerConfig enables the distributed
optimizer, so its gradient-statistics group became None
(TestMegatronFsdpGradStatsGroup in mfsdp_v1). Pass intra_dist_opt through
from the collection whenever it is present, as the parallel_state branch
always did.

The initializer-cleanup factory tests patch every optimizer helper and never
initialize model parallelism; stub the MPU fallback there instead of
reaching the global grid.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
With Muon and the distributed optimizer, the emerging-optimizer path builds
a separate DistributedOptimizer for the parameters Muon does not manage, and
it set up that optimizer's process groups with Gloo hard-coded off. The
collection now carries the Gloo groups, so honor the caller's
use_gloo_process_groups instead. Without a Gloo group, the first
memory-efficient fully reshardable checkpoint save failed the
data_parallel_group assert in get_parameter_state_dp_zero.

Tests: the layer-wise optimizer's DistributedOptimizer gets the collection's
Gloo group (and none when Gloo is off), and the Muon + Lion checkpoint
round trip also covers the memory-efficient fully reshardable format.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Without the distributed optimizer, the emerging-optimizer path reduced the
gradient norm and zero count of expert parameters over tp_ep_pp, which
leaves out the EGTP_remat axis. EGTP_remat peers hold distinct shards of
each expert weight, so each peer computed the norm of its own shards only,
and with gradient clipping the peers applied different clip factors and
their replicas drifted apart. Use tp_ep_pp_with_egtp_remat, as the standard
optimizer path does, falling back to tp_ep_pp for collections without it
(the same group when EGTP_remat is off).

Test: Muon with EP=2 and EGTP_remat=2 reports the norm over every expert
shard on every rank. Before this change the two EGTP_remat peers reported
53.1 and 73.3 on 4 ranks, against a true norm of 89.1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The distributed optimizer shards its state over the collection's
data-parallel group (dp_cp, or intra_dp_cp with more than one optimizer
instance) and gathers that state over the matching Gloo group for checkpoint
I/O. Taking the Gloo groups from the collection means they can now disagree:
for example, a caller who starts from use_mpu_process_groups() and replaces
dp_cp with their own group keeps parallel_state's Gloo group, and the save
would then gather over other ranks, failing on a size mismatch or silently
scrambling the optimizer checkpoint. Compare the rank lists of each Gloo
group and the group it mirrors (dense and expert) and raise ValueError when
they differ.

Only expert parameters use the expert Gloo group, so require
intra_expt_dp_gloo only when the collection has an expert data-parallel
group. A dense model on its own grid can then enable Gloo by supplying
intra_dp_cp_gloo alone.

Tests: groups built by parallel_state pass with one and two optimizer
instances; a Gloo group over other ranks raises, dense and expert; replacing
dp_cp in an MPU collection with a same-size group over other ranks raises; a
dense collection with only intra_dp_cp_gloo is accepted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The Gloo-collection test built its collection with use_mpu_process_groups(),
so its Gloo groups were the parallel_state globals and the identity checks
also passed for code that read the globals directly. Swap freshly created
Gloo groups into the collection instead, and add an expert parameter so the
expert distributed optimizer's Gloo group is checked too. The case where Gloo
is requested in a job built without Gloo groups is now its own test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The CI ratchet from NVIDIA#6258 landed on main after this branch was written, so
its allowlist still lists the two optimizer-helper fallbacks this change
removes and lacks the one boundary read it consolidates them into. Refresh
it: 210 -> 209 grandfathered reads.

The addition was reviewed by hand: get_megatron_optimizer resolves a
missing pg_collection with use_mpu_process_groups() once, at the entry
point, replacing the fallbacks in _get_megatron_emerging_optimizer and
_get_megatron_optimizer_based_on_param_groups that were already
grandfathered. After adding it, `python tools/check_process_group_usage.py
--update` dropped the two stale entries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
A collection without an embd field, or tied pipeline embedding/output stages
without a usable embedding group, is a caller error. Report it with
ValueError instead of an assert, which is skipped under python -O.

Signed-off-by: Yan Xu <yxu1@nvidia.com>
Six sites in megatron/core called parallel_state accessors even though the
correct group was already stored on the object (self.tp_group / self.cp_group).
Pure substitution: no signature changes, no new arguments, no caller impact.

  SelfAttention.run_realtime_tests       3 sites -> self.tp_group
  SelfAttention.get_query_key_value_tensors  1 site -> self.tp_group
  MultiTokenPredictionLayer              1 site -> self.tp_group
  LLaVAModel                             1 site -> self.cp_group

Global process-group reads in megatron/core: 163 -> 157.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
…scope

Two more sites where the correct group was reachable without any API change.

transformer_engine.py: in the cp_comm_type == 'a2a+p2p' branch the code read the
global hierarchical CP groups, even though the caller-supplied branch directly
above asserts hasattr(pg_collection, 'hcp') for exactly that case. The assert
was already guaranteeing the group the code then declined to use.

language_module.py: the word-embedding replica id derived its DP-CP rank from
the global grid while the sharded tensor nine lines later is built against
metadata['dp_cp_group']. Now both use the same group, so replica id and
sharding agree under a caller-supplied grid.

Global process-group reads in megatron/core: 157 -> 155.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
megatron/core/models/multimodal/context_parallel.py hard-coded the global CP
grid in 11 places, across module-level helpers with no group parameter. This is
the vision-encoder path, so it is exactly where an independent encoder grid
silently used the LLM's groups instead.

cp_group is now a required argument on the public helpers and is threaded
through the private ones and the autograd Function. get_pg_size/get_pg_rank
preserve the previous behaviour when CP is disabled (size 1, rank 0).

The call graph is closed inside this repo: LLaVAModel already holds
self.cp_group, and the only other callers are unit tests. The module is not
re-exported and has no users in Megatron-Bridge or nemo-rl.

Global process-group reads in megatron/core: 155 -> 144.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
_gather_from_tensor_parallel_region read the global TP grid for its size, rank
and both collectives. Both callers are subclasses that already hold the group
(self.tp_group on ColumnParallelLinear, self._tp_group on the TE variant), and
the mapping helpers it calls already accept an explicit group.

Global process-group reads in megatron/core: 144 -> 141.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
… in tests

Caught by a distributed run: changing the autograd Function signature
missed one direct .apply() call site in the unit tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
hybrid_context_parallel_forward_backward read the global TP and DP-CP grids for
its broadcast source rank, broadcast group, rank checks and barrier. Its only
caller is forward_backward_no_pipelining, which already holds pg_collection.

dp_cp is asserted in the hybrid-CP branch rather than unconditionally, matching
how TEDotProductAttention asserts hcp only for cp_comm_type == 'a2a+p2p'; the
legacy schedule collection already requests dp_cp.

get_tensor_model_parallel_src_rank() becomes
torch.distributed.get_global_rank(tp_group, 0), which is its definition.

hybrid_cp_schedule.py no longer imports parallel_state at all.

Global process-group reads in megatron/core: 141 -> 136.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
run_realtime_tests compared Q/K layernorm parameters across the global DP grid.
The DP group is required only by this debug check, so it is asserted there
rather than in __init__, which asks use_mpu_process_groups for tp/cp only --
the same idiom TEDotProductAttention uses for hcp.

megatron/core/transformer/attention.py no longer references parallel_state.

Global process-group reads in megatron/core: 136 -> 133.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
mtp_on_this_rank read the global PP rank to decide whether MTP layers live on
this stage. Both model callers (GPTModel, HybridModel) already hold self.pp_group
from LanguageModule.__init__, which runs before the call.

Global process-group reads in megatron/core: 125 -> 124.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
RotaryEmbedding and MultimodalRotaryEmbedding fell back to the global CP grid
via a ternary when cp_group was not passed. 10 of the 22 constructor calls in
this repo already passed one; the remaining 12 (3 in core, 9 in tests) now do
too, so the fallback is gone rather than merely unused.

megatron/core/models/common/embeddings/rotary_pos_embedding.py no longer
references parallel_state.

Global process-group reads in megatron/core: 124 -> 122.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
…nd T5Model

Both classes fell back to ProcessGroupCollection.use_mpu_process_groups() when
no collection was passed. The fallback is now an assertion naming the missing
argument, which is the 'fails loudly rather than silently' half of the goal.

DotProductAttention: already dead in-repo -- Attention always passes
pg_collection when building core_attention, and the one direct test
construction passes it positionally.

T5Model: 4 of 5 constructors did not pass one (3 tests, 1 example); they now do.

This is a deliberate breaking change for external callers that construct these
directly without a collection, which is the intent of the migration.

Global process-group reads in megatron/core: 122 -> 120 (shims 20 -> 18).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
…lock

The use_mpu_process_groups fallback becomes an assertion naming the missing
argument. All 7 constructors without one live in a single test module and now
pass a collection.

Global process-group reads in megatron/core: 120 -> 119 (shims 18 -> 17).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
LLaVAModel is the case the migration exists for: a vision encoder and an LLM may
run on independent parallel grids, so falling back to the global grid silently
picks the wrong one. The fallback becomes an assertion that says exactly that.

All 8 constructors without a collection (7 tests, 1 example) now pass one.

Global process-group reads in megatron/core: 119 -> 118 (shims 17 -> 16).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Attention, MultiLatentAttention (x2) and AbsorbedMLASelfAttention each fell back
to use_mpu_process_groups when no collection was passed. All are dead in-repo:
TransformerLayer sets attention_optional_kwargs['pg_collection'] unconditionally
before build_module, and every direct constructor already passes one (verified
by AST across megatron/, tests/ and examples/).

Global process-group reads in megatron/core: 118 -> 114 (shims 16 -> 12).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Global process-group reads in megatron/core: 114 -> 113 (shims 12 -> 11).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
These were based on a faulty inventory. My constructor scan filtered on
len(node.args) < 2, which silently skipped every call site passing arguments
positionally, so 'no caller is missing pg_collection' was wrong.

A paired run showed 85 real failures:
  63x AssertionError: MultiLatentAttention requires an explicit pg_collection
  22x AssertionError: Attention requires an explicit pg_collection
  10x TypeError: DSAIndexer.__init__() got multiple values for 'pg_collection'
      (my inserted keyword duplicated an existing positional argument)

Reverting to the last validated state. The earlier fallback removals
(DotProductAttention, T5Model, MultiTokenPredictionBlock, LLaVAModel) were each
validated against a pristine baseline and are retained.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Redo of the reverted attempt, this time with a signature-aware inventory: both
DSAIndexer constructors already pass pg_collection positionally, so the fallback
is dead and the tests need no change. The previous attempt added a redundant
keyword and produced 'got multiple values for argument pg_collection'.

Global process-group reads in megatron/core: 118 -> 117 (shims 16 -> 15).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Second attempt, this time driven by the signature-aware scanner: 15 of 16
SelfAttention constructors were missing pg_collection (all in test modules) and
now pass one. The earlier attempt used a scan that skipped positional call
sites and had to be reverted.

Global process-group reads in megatron/core: 117 -> 116 (shims 15 -> 14).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
…sses

Second attempt, driven by the signature-aware scanner. MultiLatentAttention (x2)
and AbsorbedMLASelfAttention no longer fall back to use_mpu_process_groups.
18 of 19 MLASelfAttention constructors were missing pg_collection, all in one
test module, and now pass one; AbsorbedMLASelfAttention's single constructor
already did.

Global process-group reads in megatron/core: 116 -> 113 (shims 14 -> 11).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Paired run: 24 failed / 43 passed against a pristine 20 / 57, with 110
'requires an explicit pg_collection' assertions. The failures are concentrated
in TestFusedMLA*, i.e. a MultiLatentAttention subclass my scan never covered --
I scanned MLASelfAttention and AbsorbedMLASelfAttention by name and assumed
those were the only constructors reaching the base __init__.

The Attention removal in the preceding commit validated clean (5F/121P on both
trees) and is retained.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY and others added 8 commits October 30, 2026 15:23
NVIDIA#8043 lets clip_qk reduce over the caller's data-parallel group and
passes it the model's DP x CP group. The clip_qk fallback warning that
follows changes the same code, so merge NVIDIA#8043 here. The merge is clean.

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


NVIDIA#8049 adds ProcessGroupFallbackWarning, resolve_process_groups and
require_process_groups, on top of NVIDIA#6293's deprecated global fallbacks,
which this branch already carries through NVIDIA#6303. clip_qk, the GTP shard
description and get_megatron_optimizer resolve their groups through
these helpers, so merge NVIDIA#8049 here. The merge is clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The allowlist merged without conflict, but the merged file keeps a
total of 172 while the merged tree has 171 reads (NVIDIA#6303 removes the
T5Model shim read). Regenerate it from the merged tree; no entry
changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
clip_qk reduces the maximum attention logits over the group its caller
passes as dp_cp_group. When the caller omits it, clip_qk reads
parallel_state.get_data_parallel_group(with_context_parallel=True),
with no warning and no removal date. For a model on its own grid, the
logits are then reduced, and the weights clipped, over another model's
data-parallel ranks.

The fallback now goes through resolve_process_groups, which warns once
(deprecated in 0.21, removed in 0.23) and returns the same global group:
dp_cp_gtp_remat is the DP x CP group that includes the GTP-remat peers.
It still resolves the group only when a layer has logits to reduce, so a
model without them neither warns nor reads the grid. The training loop
already passes the model's group.

The tests check that a passed group is used without any global read,
that an omitted one warns and reduces over the same global group as
before, and that a model without logits needs no group.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Float16OptimizerWithFloat16Params.sharded_state_dict rebuilds the
checkpoint entry of a GTP_remat shard that has no matching model entry,
such as the gathered-and-split Mamba and GDP in_proj weights under
distributed Muon. The rebuild read the global tensor-parallel and
data-parallel groups from parallel_state, with no warning. A model on
its own grid, such as a MIMO module, then got TP offsets from another
model's grid, or an assertion where no global grid exists.

Float16OptimizerWithFloat16Params takes a keyword-only pg_collection,
the process groups of the model whose parameters it updates.
get_megatron_optimizer and LayerWiseDistributedOptimizer pass the
collection they already hold. The rebuild takes tp and the DP x CP
group from it, dp_cp_gtp_remat when the collection sets it and dp_cp
otherwise, as the model's own entries do. A collection without tp, or
with neither a dp_cp_gtp_remat nor a dp_cp group, raises ValueError.
An optimizer built without a collection falls back through
resolve_process_groups, which warns once (deprecated in 0.21, removed
in 0.23) and builds the same global groups as before. The groups are
still resolved only when a shard needs a rebuild, so shards that match
their model entries need neither.

On the standard grid the groups are the same as before, so checkpoints
do not change. The DP x CP group does not change the rebuilt entry on
any grid: a GTP shard elects its writer over the group stamped on the
param.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
44 calls to get_megatron_optimizer in the unit tests omit pg_collection,
so the optimizer builds its groups from parallel_state on its own. Pass
ProcessGroupCollection.use_mpu_process_groups() instead: it builds the
same collection the optimizer builds without one, so every test runs on
the same groups, and the tests keep working without a warning once that
fallback warns.

The initializer-cleanup tests stub out the builders and never initialize
model parallelism. They replaced use_mpu_process_groups() with a stub
that returned an empty namespace; they now pass an empty
ProcessGroupCollection, and the stub goes.

The MIMO dispatch test keeps omitting the collection: a MimoModel builds
its optimizers per module before the fallback is reached.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
get_megatron_optimizer is the boundary where the optimizer takes its
process groups: every helper below it receives the collection. A caller
that passes no collection got one from
ProcessGroupCollection.use_mpu_process_groups(), with no warning and no
removal date, so a model on its own grid silently sharded its optimizer
state and reduced its gradient statistics over another model's ranks.

The fallback now goes through resolve_process_groups, which warns once
(deprecated in 0.21, removed in 0.23). It builds every field, as
use_mpu_process_groups() does by default, so the optimizers get the same
groups as before: they read most fields, and several of their readers
treat an unset field differently from one set to None. A collection the
caller passes is used as before. A MimoModel still builds one optimizer
per module, on that module's collection, before the fallback is reached.

The training loop passes None while Gloo process groups are enabled,
the default, so stock training warns until it passes its collection
with the Gloo groups; that is a separate change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The silent fallbacks of clip_qk, the GTP shard rebuild in
_backfill_gtp_sharded_param_map and get_megatron_optimizer now go
through resolve_process_groups. The checker records each as a guarded
read, and the rebuild reads one collection instead of two accessors:
171 reads (152 accessor, 19 shim) become 170 (149 accessor, 18 shim,
3 guarded).

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-c5-optimizer-fallbacks branch from ce4a8fc to 687f10e Compare November 1, 2026 01:38
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
NVIDIA#8752 gives Float16OptimizerWithFloat16Params a keyword-only
pg_collection and makes get_megatron_optimizer resolve its collection
once, through resolve_process_groups, before it builds the optimizers.
NVIDIA#8752 also carries NVIDIA#6303, whose imports its tests use. The config-logger
change that follows forwards that collection to the optimizers' config
log, so merge it here.

Two files conflict with NVIDIA#8042, which this branch carries:
- optimizer/__init__.py: keep both import changes, NVIDIA#8752's
  resolve_process_groups and NVIDIA#8045's get_ep_rank_and_size; in
  _get_megatron_optimizer_based_on_param_groups keep NVIDIA#8752's ValueError
  for a missing collection and NVIDIA#8042's set_duplicate_filter_groups call,
  which replaces the tp_group and expert_tp_group setattrs that NVIDIA#8752's
  side still read.
- optimizer.py: import the union of both sides from
  process_groups_config (ProcessGroupCollection,
  require_process_groups, resolve_gtp_remat_group,
  resolve_process_groups).

The process-group usage allowlist conflicts on NVIDIA#8042's entries; it is
regenerated on the merged tree and equals NVIDIA#8752's file (170 reads: 149
accessor, 18 shim, 3 guarded).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
NVIDIA#8755 lets log_config_to_disk take the logging owner's process groups,
through its pg_collection keyword, and adds
test_config_logger_process_groups.py. The config-logger change that
follows passes the optimizers' collection to that keyword and extends
that test, so merge it here.

Only the process-group usage allowlist conflicts. It is regenerated on
the merged tree: 167 reads (136 accessor, 16 shim, 15 guarded).
Against NVIDIA#8755's file, NVIDIA#8752's guarded reads in get_megatron_optimizer,
_backfill_gtp_sharded_param_map and clip_qk replace their global reads.
The merged .py files show no new undefined names or redefinitions (ruff
F821/F811). Both branches carry NVIDIA#6303.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
The checker classification that follows requires every allowlisted
read in megatron/core to be warned or guarded, and its test lists the
only two reads allowed to stay unwarned. NVIDIA#8752 warns before
get_megatron_optimizer's use_mpu_process_groups() fallback, before
clip_qk's global data-parallel group (with NVIDIA#8043, which NVIDIA#8752 brings)
and describes the GTP shards in _backfill_gtp_sharded_param_map with the
optimizer's own groups. Without it those four reads are unwarned and the
classification test fails.

megatron/core/optimizer/__init__.py conflicts on its imports: NVIDIA#8045's
e09f5de (which NVIDIA#8749 brings) imports get_ep_rank_and_size next to
get_global_unique_param_name, and NVIDIA#8752's 2d4d3cc adds
resolve_process_groups to the process_groups_config import. The import
takes both.

NVIDIA#8752 also carries NVIDIA#6303, which this branch does not have yet, so
tests/unit_tests/export/trtllm/test_trtllm_distributed_gpu_converter.py
conflicts on its imports: NVIDIA#6303 imports ProcessGroupCollection and NVIDIA#8059
adds ProcessGroupFallbackWarning to the same import. The import takes
both. The process-group usage allowlist is regenerated on the merged
tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
The checker classification that follows requires every allowlisted
read in megatron/core to be warned or guarded. NVIDIA#8755 warns the global
fallbacks of the log and param-hash helpers, RoPE, MIMO partitioning,
batch-invariant unpermute, the FP8/FP4 contexts and the CUDA-graph
helpers, and brings NVIDIA#8017, NVIDIA#8032, NVIDIA#8035, NVIDIA#8047, NVIDIA#8049 and NVIDIA#8053 (NVIDIA#6303
already came in with NVIDIA#8752).

megatron/core/extensions/transformer_engine.py conflicts on its
parallel_state import: NVIDIA#8038 drops get_tensor_model_parallel_world_size
and keeps model_parallel_is_initialized, and NVIDIA#8755's eac0b1c drops
get_amax_reduction_group and model_parallel_is_initialized. The merged
file calls none of the three, so the import keeps only
get_context_parallel_group, get_hierarchical_context_parallel_groups and
get_tensor_model_parallel_group. The process-group usage allowlist is
regenerated on the merged tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
The checker classification that follows requires every allowlisted
read in megatron/core to be warned or guarded. NVIDIA#8769 warns before
resolve_gtp_remat_group reads the global grid, and brings NVIDIA#8734
(optimizer, duplicate-filter and tensor-parallel seeding fallback
warnings), NVIDIA#8057, NVIDIA#8060 and NVIDIA#8073.

Five files conflict. Each is resolved as follows:
- optimizer/optimizer.py, process_groups_config import: take NVIDIA#8752's
  require_process_groups and resolve_process_groups (92d2030) and
  NVIDIA#8734's _FALLBACK_WARNING_SKIP_PREFIXES, ProcessGroupFallbackWarning,
  resolve_gtp_remat_group and warn_global_process_group_fallback
  (ec801ee).
- generalized_tensor_parallelism.py: keep NVIDIA#7029's TeardownStage and
  register_model_parallel_teardown import, which NVIDIA#8767 brings, and
  NVIDIA#8734's resolve_process_groups import.
- inference_layers.py: InferenceLinear keeps NVIDIA#8035's comment and
  pg_collection=pg_collection, which NVIDIA#8755 brings, and passes NVIDIA#8734's
  owner=type(self).__name__ to resolve_gtp_remat_group.
- tensor_parallel/layers.py: NVIDIA#8734 stops calling the four parallel_state
  rank and size getters, so their import goes; NVIDIA#8734's
  process_groups_config import stays, and get_global_memory_buffer comes
  from megatron.core.utils as NVIDIA#8730, which NVIDIA#8767 brings, has it.
- tests/unit_tests/ssm/test_hybrid_block.py get_pg_collection: the
  required groups are the union of NVIDIA#8035's embd, tp_cp and tp_dp_cp and
  NVIDIA#8073's ep, expt_tp and tp_ep, with both comments.
The process-group usage allowlist is regenerated on the merged tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
This base holds every other PR of the parallel_state deprecation series
(NVIDIA#6307): the end-to-end test that follows asserts that no process-group
fallback or deprecation warning fires anywhere on a collection that is
not the global grid, and the strict-fallback functional tests turn
every remaining fallback into an error, so both need the whole series
below them. NVIDIA#8763 brings NVIDIA#8752, NVIDIA#8755 (with NVIDIA#6303), NVIDIA#8042 and NVIDIA#8045.

Four files conflict:
- extensions/transformer_engine.py: the process_groups_config import
  adds NVIDIA#8755's resolve_process_groups (eac0b1c).
- models/T5/t5_model.py: drop the self.tp_group line that NVIDIA#6293 kept;
  NVIDIA#6303's b732c67, which NVIDIA#8763 brings, removes it because
  LanguageModule already sets self.tp_group from the collection.
- optimizer/fully_sharded_optimizer.py: keep NVIDIA#8763's
  ProcessGroupCollection import (c17120b) and NVIDIA#8731's
  get_model_ep_rank_and_size import (3877940).
- parallel_state.py, get_data_parallel_group_gloo and
  get_expert_data_parallel_group_gloo: take NVIDIA#6259's check_initialized
  argument and Args section, which NVIDIA#8763 brings, with NVIDIA#7991's
  deprecation paragraph before the Args section.
The process-group usage allowlist is regenerated on the merged tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY and others added 8 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:
- examples/t5/pretrain_t5.py: Main adds `cfg = get_run_config()` to
  model_provider and drops the `build_tokenizer` import in favour of
  `get_tokenizer()`. The PR moves that import for isort and passes a
  ProcessGroupCollection into T5Model. The result keeps main's `cfg`
  line and the PR's pg_collection fallback and argument. It drops the
  `build_tokenizer` import, which would otherwise be unused.
- megatron/core/models/common/embeddings/rotary_pos_embedding.py: Main
  imports `register_capture_constant` for its shared rotary cache. The
  PR imports `warn_global_process_group_fallback`. Both imports are
  kept, along with main's cache refactor and the PR's warning
  cp_group fallback.
- pretrain_gpt.py, pretrain_hybrid.py: Main (NVIDIA#8341, NVIDIA#8592) moves
  `is_dataset_built_on_rank` into megatron/training/datasets/providers.py
  and the dataset config builder into argument_utils. The PR adds
  `pp_group` to the `mtp_on_this_rank` call in the local predicate. The
  result takes main's layout and keeps the PR's `pp_group` in get_batch.

Adapted to main outside the conflicts:
- megatron/training/datasets/providers.py: The PR's predicate edit is
  re-applied in main's shared `is_dataset_built_on_rank_explicit`. It
  passes `pp_group=mpu.get_pipeline_model_parallel_group()` to
  `mtp_on_this_rank`, so middle pipeline stages do not hit the
  fallback warning. The function's signature is unchanged.

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>
Adapted to main outside the conflicts:
- tests/unit_tests/training/test_train_step_qk_clip_group.py: Main's
  NVIDIA#8149 makes train_step read the run config first, which asserts when
  none is set. The PR's new tests now use an autouse training_config
  fixture that sets micro_batch_size to 1. This is the same fixture
  main added to test_train_step_schedule_plumbing.py, so the tests
  reach clip_qk again. No production code changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
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>
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:
- examples/bert/pretrain_bert.py: The PR adds a ProcessGroupCollection
  import for the pg_collection default in model_provider, and main's
  tokenizer refactor (NVIDIA#8391) swaps build_tokenizer for get_tokenizer.
  The result keeps the PR's import list, imports get_tokenizer from
  global_vars, and drops the unused build_tokenizer import.
- tests/unit_tests/pipeline_parallel/test_fine_grained_activation_offloading.py:
  The PR imports ProcessGroupCollection for its pg_collection arguments,
  and main splits the tensor_parallel.random import to add
  CheckpointWithoutOutput. The result keeps both imports.

Adapted to main outside the conflicts:
- tests/unit_tests/ssm/test_hybrid_block.py: Main's MTP overlap test
  now also requires the embd group. Without it, the PR's LanguageModule
  check raises before HybridModel reaches the guard the test expects.
- tests/unit_tests/inference/coordinator_pairwise_utils.py,
  tests/unit_tests/inference/engines/batch_invariant_test_utils.py,
  tests/unit_tests/inference/engines/test_batch_invariant_models.py,
  tests/unit_tests/models/test_pos_embedding_cuda_graph.py,
  tests/unit_tests/models/test_hybrid_mhc.py: The GPTModel and
  HybridModel constructions that main added now pass
  pg_collection=ProcessGroupCollection.use_mpu_process_groups(). This
  keeps them off the deprecated fallback, as the PR does elsewhere.

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>
…ir update to main

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- tests/unit_tests/dist_checkpointing/utils.py: Main (NVIDIA#8213) adds a
  config.finalize_optimizer_config() call before the optimizer is built
  in setup_model_and_optimizer and setup_moe_model_and_optimizer. The PR
  passes pg_collection=ProcessGroupCollection.use_mpu_process_groups()
  to get_megatron_optimizer in both helpers. The result runs finalize
  first, then the PR's pg_collection call.

Adapted to main outside the conflicts:
- tests/unit_tests/distributed/mfsdp_v2/test_mcore_moe.py: Main (NVIDIA#8312)
  adds test_shortcut_moe_training_and_clipping, which calls
  get_megatron_optimizer without pg_collection and so hits the PR's
  warned fallback. Both calls now pass
  pg_collection=ProcessGroupCollection.use_mpu_process_groups(), as the
  PR does in test_standard_moe_training_and_clipping. The groups are the
  same as before.

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 #8903, which carries this change as commit c388533 under the same title, with "Originally #8752." 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.

2 participants