Skip to content

Warn when training checkpoint helpers fall back to the global grid - #8758

Closed
Connor-XY wants to merge 142 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-f1b-checkpointing
Closed

Connor-XY wants to merge 142 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-f1b-checkpointing

Conversation

@Connor-XY

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

Copy link
Copy Markdown
Contributor

Depends on #8757 (which contains #8741, #8037 and #8049) and #8740. Please review the diff from their merge: scoped diff.

Why. save_checkpoint and load_checkpoint pass the model's groups to every helper they call. The helpers still read the global grid, silently, for any group a direct caller omits. save_grads always reads it, so the wgrad, param, dgrad and activation dumps of a model built on its own grid are named and written by the global ranks.

What.

  • These fallbacks now emit ProcessGroupFallbackWarning (deprecated in 0.21, removed in 0.23), naming the function and the omitted argument, and then read the same global groups as before:
    • get_checkpoint_name, for a rank or flag that is neither passed nor in a collection;
    • get_rng_state, for dp_group and dp_cp_group;
    • _build_sharded_state_dict_metadata and the default metadata of generate_state_dict;
    • maybe_save_dataloader_state, once per omitted group, before it branches on parallel ranks;
    • the restore group of a local non-persistent checkpoint with sharded state;
    • the fully parallel load group;
    • the collection of a model built without one, owned by save_checkpoint or load_checkpoint.
  • Paths that read nothing global stay silent: explicit ranks, a base-directory name and a disabled dataloader save.
  • save_grads takes a keyword-only pg_collection, which must set tp, pp, ep and expt_dp. The writers are the first ranks of expt_dp_gtp_remat, or of expt_dp for a collection without it. Without a collection it warns and reads the global grid as before.
  • DataGradLogger, ActivationLogger and their module-level wrappers forward the collection. train_step passes the model's collection to all three dumps, with owner="train_step".

Tests.

  • New tests/unit_tests/training/test_checkpointing_fallback_warnings.py:
    • on CPU, each fallback warns and calls the same accessor, and each explicit path neither warns nor reads the global grid;
    • on 4 GPUs, dumps on a TP2×PP2 grid unlike the global one;
    • on 4 GPUs, on the standard TP2×PP2×EP2 grid, checkpoint names, RNG shard keys, dump files and dataloader files match the fallback's.
  • In test_checkpointing_pg_collection.py, the model-grid save/load round trips now also fail on any fallback warning, and the collection-resolution test checks the warned path.
  • Under -W error::megatron.core.process_groups_config.ProcessGroupFallbackWarning, 7 existing maybe_save_dataloader_state tests in tests/unit_tests/test_checkpointing.py fail. They omit cp_group, which now warns. CI sets no such filter.

On 4 GPUs against its base, test_checkpointing_fallback_warnings.py passes 35 tests, all of them new, and test_checkpointing_pg_collection.py passes 5 here vs 5 on the base. In the integration run, which merges all #6307 PRs onto main and runs the full unit-test suite against main, this PR's test files show no regressions: 35 and 5 tests pass on 4 GPUs. End-to-end smokes on the same code, 22 training configurations on 4 GPUs including resumes, give bit-identical losses to main; resumed runs match their uninterrupted runs, and no process-group fallback warning is recorded.

Before the 2026-11-01 sync, the branch ran both test files on 4 CPU ranks with Gloo: 35 passed, the same per test as on the stacked version, and 33 of them are new. Two new 4-GPU tests and three existing ones fail there for lack of CUDA.

Compatibility.

  • Every new argument is keyword-only and defaults to the old behavior, which now warns. Stock training and the checkpoint converter pass their groups and do not warn.
  • On the standard grid, checkpoint names, writers and contents are unchanged.
  • generate_state_dict builds its default torch_dist metadata from the dp_cp_group it is given, instead of the global DP×CP group. That is the same group on the standard grid, and save_checkpoint and load_checkpoint do not use the default.
  • save_grads with a collection raises ValueError if tp, pp, ep or expt_dp is missing. That only matters when a dump is enabled, and stock collections set all four. A collection with expt_dp=None and no expt_dp_gtp_remat makes every data-parallel replica write the same file.
  • The multimodal example's dataloader restore calls get_checkpoint_name without tensor_rank or pipeline_parallel (examples/multimodal/dataloader_provider.py). Until Use the model's process groups in ModelOpt, multimodal and MIMO examples #8761 passes them, resuming its dataloader warns once, and raises under --strict-process-group-fallbacks.
  • Needs determinism-exempt only while Finish moving the training loop onto its process-group collection #8757 is unlanded. CI diffs against main, and Finish moving the training loop onto its process-group collection #8757 changes utils/common_utils.py, a registered kernel file. This PR's own diff passes the coverage check.

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>
save_grads names each dump file after the tensor, pipeline and expert
parallel ranks of its writer, and lets the first rank of each expert
data-parallel group write; it reads all of them from the global
parallel grid. The wgrad and param dumps of train_step, DataGradLogger
and ActivationLogger call it without groups, so a model built on its
own grid is dumped under the ranks of the global grid.

save_grads takes a keyword-only pg_collection, which must set tp, pp,
ep and expt_dp. The writers are the first ranks of expt_dp_gtp_remat,
the group the global fallback uses, or of expt_dp for a collection
without it. Without a collection, save_grads warns (deprecated in 0.21,
removed in 0.23) and reads the global grid as before.
DataGradLogger.save, ActivationLogger.save_activations and their
module-level wrappers forward the collection, and train_step passes the
model's collection to all three dumps, so training emits no fallback
warning.

On the standard grid the collection selects the same ranks as the
global accessors, so file names and writers are unchanged.

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-f1b-checkpointing branch from d977134 to 8efb33f Compare November 1, 2026 01:38
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
The warned fallbacks outside Megatron Core that follow rewrite the
global-grid reads in checkpointing.py that NVIDIA#8758 warns, and NVIDIA#8758 brings
NVIDIA#8757's training-loop helpers that the training files use.

megatron/training/initialize.py conflicts on the process_groups_config
import: NVIDIA#8733 imports enable_strict_process_group_fallbacks, NVIDIA#8031
(which NVIDIA#8758 brings through NVIDIA#8737) imports ProcessGroupCollection, and
NVIDIA#8757 (which NVIDIA#8758 brings) adds warn_global_process_group_fallback. The
import takes all three.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY and others added 22 commits October 31, 2026 23:00
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>
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>
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>
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:
- megatron/training/training.py: Main adds the tensor metric observer,
  the run_train_loop_without_updates argument, the batch-size transition
  helper, new train() docstring entries, and moves the RL inference-model
  setup out of pretrain() (NVIDIA#4264). The PR builds the process-group
  collection once right after initialization and passes it everywhere.
  The result keeps both sides. The setup_model_and_optimizer call passes
  pg_collection directly, without main's use_mpu_process_groups()
  fallback, and also passes run_train_loop_without_updates. The
  docstrings combine main's new entries with the PR's pg_collection and
  p2p_communicator wording.

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>
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>
…main

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

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>
Conflicts:
- megatron/training/training.py: Main factors the RL code out of
  training.py (NVIDIA#4264), adding the caller-managed-data branch in
  pretrain(), the keyword-only build_train_dataloader flag and
  broadcast_data_flags(). The PR passes a data pg_collection through
  the data loader and iterator builders. The result keeps main's
  structure and re-applies the PR's process-group changes on top of
  it. Both builders take (provider, pg_collection=None, *,
  build_train_dataloader=True), and the PR's docstrings gain an Args
  line for build_train_dataloader. The PR's inline flags broadcast
  gives way to main's broadcast_data_flags(), which still broadcasts
  over the default world group.

Adapted to main outside the conflicts:
- pretrain_gpt.py, pretrain_hybrid.py: The unused
  `from megatron.core import mpu` import is removed. The PR already
  drops the mpu reads from get_batch, and main moves
  is_dataset_built_on_rank into
  megatron/training/datasets/providers.py, so nothing in either file
  uses mpu any more.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- megatron/training/training.py: In train_step, the merge keeps the
  PR's model_pg_collection, which is computed once with the single-grid
  fallback. Before train(), it keeps the PR's DP world-size helper and
  main's new is_batch_size_transition. The train() docstring combines
  the PR's pg_collection text with main's new arguments. In the train()
  preamble, the merge takes main's removal of the RL reference-weights
  block and the GRPO microbatch reinit, which NVIDIA#4264 moved to
  megatron/rl/training_setup.py. That moved call still reads the global
  DP size, as main's RL path expects. The PR's sniff-test check and
  grid/sample collection setup replace main's local DP-size closure.

Adapted to main outside the conflicts:
- megatron/training/training.py: The PR and main (NVIDIA#7472) both add a
  _get_model_pg_collection helper with different signatures. They are
  now one helper at main's location: _get_model_pg_collection(
  model_chunk, pg_collection=None). It returns the model's collection
  first, then a single-grid run collection, then the global groups. The
  PR's call sites pass model[0]. Main's two tensor-metric observer calls
  in train_step now use the run's collection too, so they reduce over
  the same groups as the loss and grad-norm reductions.
- tests/unit_tests/training/test_training_loop_pg_collection.py: The
  tests pass a model chunk to the unified helper and call train() with
  main's new signature. A new test checks that the tensor-metric
  observers and the samples-seen count receive the run's collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- megatron/training/training.py: This PR builds pg_collection from
  parallel_state right after initialization and passes it on as is.
  Main adds the tensor metric observer, the
  run_train_loop_without_updates argument and the
  is_batch_size_transition helper. The result keeps both sides: the
  observer is built after pg_collection, and setup_model_and_optimizer
  gets the plain pg_collection plus main's new argument. Main's
  use_mpu_process_groups() fallback in that call is dropped because
  pg_collection is always set. The train_step and train docstrings
  combine this PR's pg_collection and p2p_communicator wording with
  main's new parameter entries.

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 (all in code that comes in with the base NVIDIA#6259):
- megatron/training/training.py: Main adds the tensor metric observer,
  run_train_loop_without_updates and is_batch_size_transition. The PR
  always builds the collection in pretrain() and passes it on. The
  result keeps both sides. The PR's plain pg_collection argument
  replaces main's use_mpu_process_groups() fallback, and the docstrings
  carry the entries from both sides.
- examples/t5/pretrain_t5.py: The result keeps the PR's collection
  fallback and main's get_run_config() and get_tokenizer(). It drops
  the build_tokenizer import, which main's tokenizer refactor left
  unused.
- megatron/core/models/common/embeddings/rotary_pos_embedding.py: The
  import block keeps the PR's warning helper and main's
  register_capture_constant. Main's embedding cache uses self.cp_group
  and adds no global group reads.
- pretrain_gpt.py, pretrain_hybrid.py: NVIDIA#8341 moves
  is_dataset_built_on_rank and core_gpt_dataset_config_from_args into
  megatron/training/datasets/providers.py, so the result takes main's
  deletions. The PR's pp_group argument to mtp_on_this_rank moves with
  the predicate into providers.py.

Adapted to main outside the conflicts:
- tests/unit_tests/training/test_training_pg_collection.py: NVIDIA#8213
  removes get_megatron_optimizer_config, and setup_model_and_optimizer
  now reads the optimizer config from the run config. The test sets
  run_config.optimizer to a distributed Adam config instead of patching
  the removed function.

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>
…NVIDIA#8047, NVIDIA#8043 and NVIDIA#8048 after their update to main

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- megatron/training/checkpointing.py: Main (NVIDIA#8213) moves the
  layer-wise distributed optimizer check in save_checkpoint to
  cfg.optimizer. The PR names the layer-wise optimizer file by the rank
  in the dp_gtp_remat group instead of the global data-parallel rank.
  The result keeps main's cfg.optimizer condition and the PR's
  group-based file index.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- megatron/training/training.py: Main moved the RL setup and the GRPO
  microbatch reinit out of train() into megatron/rl, and that code keeps
  main's global data-parallel world size. The result keeps main's data
  iterator structure and the PR's single run collection, its sample and
  grid collection split, and its data-parallel group helper. Both sides
  defined _get_model_pg_collection with different signatures (NVIDIA#7472 took
  one model chunk, the PR took the model list). They are unified as
  (model_chunk, pg_collection=None, *, owner), which falls back to the
  run's collection and then warns before using the global groups. Main's
  tensor-metric calls in train_step now pass the run's collection, so a
  model without groups emits a ProcessGroupFallbackWarning there.
- pretrain_gpt.py, pretrain_hybrid.py: Main (NVIDIA#8341) moved the dataset
  provider helpers out of the scripts, and the result takes that removal.
  The PR's get_batch changes apply on top.
- tests/unit_tests/test_training.py: Both sides' new tests are kept. The
  PR's test sets cfg.optimizer instead of patching
  get_megatron_optimizer_config, which main removed (NVIDIA#8213).
- tests/unit_tests/training/models/test_gpt_builder.py: The result keeps
  the PR's process-group imports and main's MLATransformerConfig import.

Adapted to main outside the conflicts:
- tests/unit_tests/training/test_training_loop_pg_collection.py,
  tests/unit_tests/training/test_training_fallback_warnings.py: The tests
  pass a model chunk to _get_model_pg_collection. They also call train()
  with main's keyword-only next_train_iterator and run_eval arguments.

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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- megatron/training/training.py: Main restructured pretrain(),
  setup_model_and_optimizer(), train() and the data loader builders.
  The result keeps main's new code and adds the PR's pg_collection
  plumbing back into it. Main's NVIDIA#7472 added its own
  _get_model_pg_collection(model_chunk), which shadowed the PR's helper
  of the same name. The two are now one helper,
  (model_chunk, pg_collection=None, *, owner=...), which falls back to
  the global groups with a warning. Every caller passes model[0],
  including main's tensor-metric observer calls and the save_grads,
  save_activations and save_dgrads calls from this PR.
- pretrain_gpt.py, pretrain_hybrid.py: Main moved the dataset helpers
  to megatron/training/datasets/providers.py, so the result takes the
  deletion. The PR's pp_group argument to mtp_on_this_rank is re-applied
  in the moved rank predicate in providers.py.
- examples/t5/pretrain_t5.py: Keeps the PR's pg_collection default and
  main's get_run_config() call, and drops the unused build_tokenizer
  import that main removed.
- megatron/core/models/common/embeddings/rotary_pos_embedding.py,
  tests/unit_tests/training/models/test_gpt_builder.py: Import-only
  conflicts. Both sides' imports are kept.
- tests/unit_tests/test_training.py: Keeps the new tests from both
  sides. The PR's test no longer patches get_megatron_optimizer_config,
  which main's OptimizerConfig migration (NVIDIA#8213) removed.

Adapted to main outside the conflicts:
- tests/unit_tests/training/test_checkpointing_fallback_warnings.py:
  Three tests take the run_config fixture, since main's OptimizerConfig
  migration (NVIDIA#8213) moved the settings they read into the run config.
- tests/unit_tests/training/test_training_fallback_warnings.py,
  test_training_loop_pg_collection.py, test_training_pg_collection.py:
  Updated for the single model-chunk helper, train()'s keyword-only
  next_train_iterator and run_eval, and the optimizer config read from
  run_config.optimizer.
- megatron/rl/training_setup.py: Main moved the GRPO microbatch re-init
  here, and it reads the global data-parallel world size. The PR's
  sample-collection read in train() no longer applies to it.

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 #8904, which carries this change as commit 1cba557 under the same title, with "Originally #8758." 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