Skip to content

Align the agent and review rules with the process-group checks - #8776

Closed
Connor-XY wants to merge 481 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-h4-review-rules
Closed

Connor-XY wants to merge 481 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-h4-review-rules

Conversation

@Connor-XY

Copy link
Copy Markdown
Contributor

Depends on #8774, which contains #8773, #8772 and #7991. Please review the diff from #8774's head: scoped diff.

Why. The process-group section of AGENTS.md (which CLAUDE.md links to) predates the deprecation of the parallel_state accessors. It allows "migration fallbacks with an explicit comment" and says not to apply the rules to megatron/training. The pr-review rubric carries its own copy of that text, and docs/developer/contribute.md calls the process-group job report-only. An agent or reviewer following them would add or pass a commented, silent read of the global grid, skip the training code and functional tests that the guard covers, accept a silent catch_warnings() block or a private write that CI rejects, and assume that nothing blocks an unwarned core read.

What. Docs and agent guidance only.

  • AGENTS.md: the current rules.
    • Code takes its groups from the owning model's collection or an explicit group from its caller. New code requires its groups.
    • What is deprecated, in which releases, and what is not: bootstrap, teardown and the registry stay.
    • Compatibility fallbacks go only through the warned helpers, dated 0.21 and 0.23 by default. A comment does not make a read a fallback.
    • What the checker counts and allows in megatron/core, and that its tests fail CI on a read that is neither warned nor guarded.
    • What the guard covers outside megatron/core (the rest of megatron/, examples/, tools/, tasks/, scripts/, docs/, tests/functional_tests/ and the top-level scripts) and what it allows.
    • What the pytest filters turn into errors, how to find fallbacks with strict mode, and what reviewers flag.
  • skills/pr-review/references/light.md and strict.md state the same rules. The strict rubric gives 0.21 and 0.23 as defaults: a fallback that first warns in a later release passes its own dates and is not a finding for them.
  • The guide's "Reviewing?" paragraph no longer lists bootstrap code and warned fallbacks as allowed in megatron/core, and covers the code outside it.
  • docs/developer/contribute.md says which reads in megatron/core fail the process-group job. Only its allowlist ratchet is report-only now.
  • Every text states the rules the same way and points to the guide's guard paragraph for the details:
    • the bootstrap rule: a statement that is only a call of warnings.warn(..., ParallelStateDeprecationWarning) or warn_global_process_group_fallback() comes before the with, in the block that holds it or in a block that contains that block;
    • the filter the guard accepts: a body call of warnings.simplefilter("ignore", ParallelStateDeprecationWarning) or the matching filterwarnings();
    • private parallel_state attributes are never allowed. The memory buffer comes from megatron.core.utils, since test_global_memory_buffer_imports.py rejects its aliases from parallel_state even in a fallback;
    • only the unit tests may use the global grid.

Tests. Docs only. The guard, checker, removal-version and Testmon mapping tests pass on 4 CPU ranks with Gloo before and after (116 passed). The bootstrap rule, the accepted filter forms and the memory-buffer rule were checked by running the guard, the checker and test_global_memory_buffer_imports.py on small examples of each form.

Compatibility.

  • No runtime change.
  • AGENTS.md says that under pytest pyproject.toml turns the deprecation warning into an error for this repository's modules. That describes Make the process-group checks enforce what the deprecation notes claim #8770's error filter. If maintainers drop the filter, this sentence needs the matching edit.
  • The skills/pr-review files may need review from their code owners.
  • Not touched, and already on main: the adjust_batch_dimensions_for_expert_parallelism docstring, which says that ep_group=None uses the global grid, and the @deprecated helpers in moe_utils.py that are past their removal versions.
  • determinism-exempt is needed only while the PRs below this one are unlanded. CI diffs against main, and the stack contains registered kernel files. This PR's own diff passes the coverage check.

Part of #6307.

🤖 Generated with Claude Code

Connor-XY and others added 30 commits October 29, 2026 13:00
A training run cannot show today that it passes its process groups
explicitly: each fallback to the global groups only warns once, and
the warning is easy to miss among hundreds of ranks. CI needs a run
that fails at the first fallback instead.

--strict-process-group-fallbacks makes initialize_megatron call
enable_strict_process_group_fallbacks() right after the logging setup.
The entry point's rank-0 log suppression ran earlier, on import, and
the strict filter takes precedence over it, so every rank that
reaches a fallback raises. The filter is in place before distributed
initialization, seeding and model construction. The flag is off by
default, so runs that omit it behave as before.

Stock training reaches fallbacks that later changes remove, so no
functional test enables the flag yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
When the caller omits its collection, resolve_process_groups builds
one through use_mpu_process_groups, which reads most groups with
check_initialized=False. If parallel_state is not initialized, those
groups come back as None, require_process_groups accepts None as an
axis that is off, and torch.distributed then treats each None group as
WORLD: a tensor-parallel all-reduce would sum over every rank. The
fallback warning did not say so.

Without a grid the fallback still builds the collection as before,
because code that works without a grid today moves onto the resolver
during the deprecation window:

- get_fp8_context, get_fp4_context and the Transformer Engine
  quantization contexts pass no amax group without a grid, which
  Transformer Engine reads as its default group, the right domain for
  pure data parallelism.
- The batch-invariant MoE unpermute runs in a single process without
  torch.distributed.
- Callers that check the returned groups themselves, such as the MoE
  default collection, get_ep_layer_offset and the TensorRT-LLM
  converter, raise their own, more specific errors.

The global accessors return None for most groups, and an accessor that
needs an initialized grid, such as the one for dp, still raises, so no
new None group reaches torch.distributed. A ProcessGroupFallbackWarning
now names the groups that are None and says that torch.distributed
treats a None group as the default (WORLD) group. It is recorded apart
from the warning for an initialized grid, so a process that reaches
both hears about both. Strict mode, through
enable_strict_process_group_fallbacks(),
--strict-process-group-fallbacks or pytest's -W error filter, turns it
into an error like any other fallback.

On an initialized grid nothing changes: the warning still comes before
any group is read, and its text is the same. An explicit collection is
returned as before, without reading parallel_state. The warning still
reaches every rank under the rank-0 log suppression.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#8037 builds one ProcessGroupCollection in pretrain() and passes it to
the model, the optimizer and the schedules. get_batch and the data
loaders are to read their groups from that collection, so merge NVIDIA#8037
into this branch, which carries NVIDIA#6303's explicit process-group
collections for the language models.

The only conflict is the allowlist's total; keep NVIDIA#6303's allowlist,
which matches the merged tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
forward_step passes the model's ProcessGroupCollection to get_batch in
pretrain_gpt.py and pretrain_hybrid.py, but only the sequence-packing
path used it. The default path read the global grid on every
micro-batch: the TP rank, source rank and group of the batch
broadcast, the pipeline group of the MTP placement, the first/last
stage checks and the CP group that shards the batch. A model built on
groups other than the global ones therefore received batches read,
broadcast and sharded over the wrong ranks.

Read all of them from the collection. is_first_or_last_pipeline_stage
takes an optional pipeline group and virtual pipeline size; without a
group it keeps reading the global state, so its other callers are
unchanged. get_batch raises ValueError when the collection lacks a
group it needs, instead of passing None (the WORLD group) to
torch.distributed. Callers that pass no collection keep the global
groups through the existing compatibility fallback, now shared by both
paths.

On the standard grid the collection holds the global groups, so the
batches are identical.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
pretrain_vlm.py's model provider receives the model's process-group
collection but took the pre_process, post_process and add_encoder
stages from the global pipeline grid, and get_batch broadcast every
micro-batch over the global tensor-parallel group through the
tensor_parallel.broadcast_data fallback.

Take the stages from the collection's pipeline group, and have
forward_step pass the model's collection to get_batch, which
broadcasts over its tensor-parallel group. get_batch keeps a
compatibility fallback to the global groups for callers that pass no
collection. On the standard grid the groups are the same objects as
before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
pretrain() builds one process-group collection for the run, but the
data loaders it builds still read the global grid: the samplers took
their data-parallel rank and size from parallel_state, the TP-rank-0
check that decides which ranks build the data pipeline read the global
TP rank, and full validation agreed on the number of evaluation
iterations over the global DP x CP group.

Pass the run's collection to build_train_valid_test_data_iterators,
build_train_valid_test_data_loaders and build_pretraining_data_loader.
The samplers shard over its GTP-inclusive data-parallel group
(dp_gtp_remat, or dp for collections without the GTP fields), the
pipeline is built on the first rank of its tensor-parallel group, and
the evaluation iterations are reduced over dp_cp_gtp_remat, matching
the default group of get_data_parallel_group(with_context_parallel=
True). A MIMO run keeps the global defaults because its collection has
one collection per module and its providers build their own loaders.
The new pg_collection arguments default to None, which keeps the
global groups for existing callers.

On the standard grid the collection holds the global groups, so the
samplers and reductions are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The warnings that follow extend the TP and GTP duplicate filters that
NVIDIA#8042 moves onto the optimizer's collection, and they reuse the
fallback warning helpers of NVIDIA#8049, which this branch is based on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The gradient norm, zero count and found-inf flag are reduced over the
optimizer's grad-stats group. When that group is None, torch.distributed
uses WORLD. That silently counts gradients replicated across optimizer
instances or data-parallel ranks, mixes in other models of the job, and
hangs when some ranks run no optimizer step. It happens when:

- a DistributedOptimizer has no grad_stats_parallel_group, for example
  one built from a collection with intra_dist_opt=None;
- a non-distributed optimizer has grad_stats_parallel_group=None, for
  example from a collection with mp=None;
- a LayerWiseDistributedOptimizer gets a collection without
  intra_dist_opt.

A non-distributed optimizer that never got the attribute also falls
back, silently, to the global model-parallel group.

Raising is not possible yet. Megatron-Bridge sets intra_dist_opt=None
on purpose for a single optimizer instance, where WORLD holds the right
ranks, and core's own setup error invited None. So every one of these
paths now emits a ProcessGroupFallbackWarning, deprecated in 0.21 and
removed in 0.23, and returns the same group as before:

- DistributedOptimizer.get_grad_stats_parallel_group and
  MegatronOptimizer.get_grad_stats_parallel_group warn on a None group;
- MegatronOptimizer.get_grad_stats_parallel_group warns through
  warn_global_process_group_fallback before reading the global
  model-parallel group;
- LayerWiseDistributedOptimizer warns when it is built, because it
  reads the attribute directly.

ChainedOptimizer delegates to its children, so they warn for it. The
WORLD warning has its own message, because the fallback is the default
group, not a parallel_state group. It warns once per optimizer class.
Like warn_global_process_group_fallback, it does not record a warning
that an error filter raised, so strict mode raises every time.

The setup error for a collection without intra_dist_opt no longer says
to set it to None. It asks for the ranks of one optimizer instance.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The gradient-norm and zero-count duplicate filters,
param_is_not_tensor_parallel_duplicate and param_is_not_gtp_duplicate,
read the global tensor-parallel or GTP rank when the caller gives no
group and keeps use_global_fallback=True. The optimizers take that path
when they were not built through set_duplicate_filter_groups, for
example when constructed by hand, and so do direct callers such as
count_zeros_fp32 with its default arguments. For a model on its own
grid, the global rank drops or double-counts replicated parameters,
and nothing reports it.

Both filters now emit a ProcessGroupFallbackWarning, deprecated in 0.21
and removed in 0.23, right before the global read. The warning names
the missing argument: tp_group or expert_tp_group, gtp_group or
expert_gtp_group. The result is unchanged. Optimizers that
get_megatron_optimizer builds take their groups from the collection
and pass use_global_fallback=False, so they do not warn.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#8033 honors the caller's pipeline rank in custom pipeline layouts and
sets the checkpoint converter's pipeline rank on its fake group. The
converter's models are to take their process groups from that fake
grid, on top of NVIDIA#6303's explicit process-group collections for the
language models, so merge NVIDIA#8033 into this branch.

tools/checkpoint/utils.py merges cleanly. The only conflict is the
allowlist's total; keep NVIDIA#6303's allowlist, which matches the merged
tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
tools/checkpoint built every model on the global parallel_state grid.
It wrote fake groups into private parallel_state globals, set the
_MPU_* rank and world-size overrides, and called the model provider
without a collection, so each model resolved its groups through the
deprecated global fallback. The loader and the saver also selected
stages through mpu.get_*_group().set_rank() and
mpu.is_pipeline_first_stage()/is_pipeline_last_stage().

Build the fake groups as a ProcessGroupCollection
(build_checkpoint_converter_process_groups) and pass it to the model
provider as pg_collection. The loader sets the tensor- and
pipeline-parallel rank of each model on that collection and derives
pre_process/post_process from the stage it loads; the saver selects
its stages on the collection too. Both pass the collection's groups to
load_checkpoint() and save_checkpoint(), as training does.

megatron.training.checkpointing still names legacy checkpoint files
after the global ranks, and model builders that take no collection
still read the global pipeline rank. For those reads the converter
mirrors the same group objects into parallel_state
(install_checkpoint_converter_process_groups) and keeps one override,
the pipeline rank: get_pipeline_model_parallel_rank() asks
torch.distributed for the rank of its group, which a fake group cannot
answer. The tensor-, pipeline- and expert-parallel world-size overrides
and the tensor- and expert-parallel rank overrides only repeated what
the mirrored groups report, and nothing on the converter path depends
on the virtual-pipeline rank or size globals, so those writes go.

initialize_checkpoint_converter_fake_process_groups() keeps its
behavior and now returns the collection it mirrors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The warnings that follow cover the global-group fallbacks that remain in
finalize_model_grads once NVIDIA#8027 reads the caller's groups, and they use
the fallback warning helpers of NVIDIA#8049, which this branch is based on.
The process-group usage allowlist is regenerated on the merged tree, so
it records the reads of both sides.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Without a pg_collection, finalize_model_grads read seven groups from
parallel_state up front, and two more (the TP x DP x CP group for the
router expert bias and for global-batch quantile balancing) further
down, behind their own "pg_collection is None" checks. None of these
reads warned, so a caller on another grid silently reduced over the
global groups, and the fallback had no removal date.

The no-collection branch now builds its groups with one
resolve_process_groups call, which emits a ProcessGroupFallbackWarning
(deprecated in 0.21, removed in 0.23) before it reads parallel_state.
It requests the same groups the accessors returned: tp, pp, embd,
pos_embd, the GTP-inclusive dp_cp_gtp_remat, tp_dp_cp and the two GTP
groups. The later reads use the resolved tp_dp_cp, so the function
reads parallel_state in one place only. The collection path and its
validation are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Given no collection, SimplifiedDSGQAIndexer built its tp and cp groups
with use_mpu_process_groups and did not warn. Given a collection
without cp, it read cp as None, and its RoPE then fell back to the
global context-parallel group.

Its owner, DSGQACoreAttention, already passes the collection it gets
from the attention layer. The indexer now resolves its groups through
resolve_process_groups. Like DSAIndexer, it warns when the collection
is missing: a ProcessGroupFallbackWarning (deprecated in 0.21, removed
in 0.23), followed by the same global tp and cp groups as before. A
collection without tp or cp raises ValueError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
finalize_model_grads and SimplifiedDSGQAIndexer read the global groups
only through resolve_process_groups. The nine accessor reads in
finalize_model_grads and the use_mpu_process_groups call in
SimplifiedDSGQAIndexer become one guarded read each: 166 reads to 158.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The simple training loop, the two TRT-LLM export examples and the MIMO
AVLM inference example build the standard grid themselves, yet each
later step read it back from parallel_state. The model provider called
use_mpu_process_groups() again and model_parallel_cuda_manual_seed
read the TP, EP, expert-TP and GTP ranks. In the training loop, DDP,
the schedule lookup, the no-pipelining schedule and
finalize_model_grads fell back to the global groups because no
collection was passed. The MIMO example gathered its logits over the
global TP group.

Each example now collects the groups once, right after
initialize_model_parallel, and passes that collection to the model,
the RNG seeding (as explicit ranks), DDP, get_forward_backward_func,
the schedule, finalize_model_grads and the logits gather. The export
README snippet shows the same flow. On the standard grid these are
the groups and ranks the global accessors return, so the examples
behave as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
tools/run_text_generation_server.py picked the HTTP-serving rank from
the global PP, TP and EP accessors. run_inference_performance_test.py
set the static wrapper's pipeline flag from the global pipeline-stage
predicates, and the BERT embedding tool split its blocks over
parallel_state.get_data_parallel_group(). Each tool already holds a
model, or an inference wrapper, built on a process-group collection,
so these reads could disagree with the groups the model runs on.

The server now selects the serving rank from the model's collection.
The performance test uses the wrapper's own pp_group, as
prep_model_for_inference does. The embedder passes its model's
dp_gtp_remat group, the group get_data_parallel_group() returns by
default. get_blocks_by_rank takes that group as a required keyword
argument; the embedder is its only caller.

The new tests give the static wrapper a pipeline group unlike the
global grid and the embedder a data-parallel group of its own, with
the global accessors patched to raise.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The validation that follows replaces NVIDIA#8054's test of unset groups with
one against the inference config's own requirement, so it needs
NVIDIA#8054's tests and wrapper checks below it.

The text generation controller takes the pipeline-parallel group from
the wrapper, which rejects an unset one (NVIDIA#8054), and the data-parallel
group from the collection that the config resolves for the model
(NVIDIA#8058). The process-group usage allowlist is regenerated on the merged
tree; it equals NVIDIA#8058's, since NVIDIA#8054's fallback read in the controller
is gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
InferenceConfig.resolve_pg_collection is the one place where inference
picks its process groups, but it only checked the collections it took
from the model or built from parallel_state. A configured collection,
which is what to_inference_config and the inference shards pass, was
returned as given. ProcessGroupCollection reads an unset field as None,
and torch.distributed treats a None group as the world group. So for a
collection without mp, DynamicInferenceEngine's
start_listening_to_data_parallel_coordinator broadcast the request
address of global rank 0 over the world and called barrier() on the
world. That hangs, or synchronizes unrelated ranks, when inference runs
on part of the world, for example RL training and inference in one
job; when it does not hang, the MP workers of every replica subscribe
to the publisher of global rank 0. A None dp makes the coordinator
address broadcast a world broadcast and lets every replica spawn its
own coordinator, and a None tp or pp makes every rank an MP
coordinator.

The disaggregated coordinator setup also reads cp and expt_tp, and the
Dynamo engine service reads expt_dp, from the resolved collection. The
warned fallback did not build them, so those reads saw None.

resolve_pg_collection now checks whichever collection it returns. It
must set every group that inference reads (tp, pp, dp, mp, ep, cp,
expt_tp and expt_dp), and tp, pp, dp and mp must be process groups
rather than None; otherwise it raises ValueError before any component
uses the collection, and stores nothing. ep, cp, expt_tp and expt_dp
may be None for an axis that is off. The model's collection is used
only when it sets all eight groups, and the fallback builds all eight.

Collections from use_mpu_process_groups(), build_inference_pg_collection
and Megatron Bridge set all of these groups, so they are used as
before, and on the standard grid every group is unchanged.

NVIDIA#8054's tests of unset groups built collections that set only tp and
pp and expected the wrapper's and the controller's own messages, which
the config now reports first. test_rejects_unset_group now removes, or
sets to None, one of tp, pp, dp and mp from a full collection and
expects the config's message. The two controller tests for an unset dp
are dropped: the config rejects such a collection before a controller
is built, and the dp cases of test_rejects_unset_group cover that.

Depends on NVIDIA#8058 and NVIDIA#8054.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#8026 creates the global memory buffer on first use instead of in
initialize_model_parallel(). The process-group registry goes into
parallel_state on top of the standard-grid builder (NVIDIA#8039) and needs
the same end of initialize_model_parallel(), so merge NVIDIA#8026 here.

initialize_model_parallel() conflicts at its end: keep the builder
call and the registration, and drop the memory-buffer comment and the
_set_global_memory_buffer() call, which NVIDIA#8026 removes. The allowlist
merges cleanly but keeps a stale total; set it to the merged tree's
194 reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Seeding the RNG from the grid that initialize_megatron builds needs the
collection that initialize_model_parallel returns (NVIDIA#8039, this
branch's base) and the RNG seeding that takes a process-group
collection (NVIDIA#8031).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The guide described the checks as they were before the recent checker
and guard changes. Since then the checker also counts
get_registered_process_groups() as a shim and the other grid-reading
helpers of process_groups_config.py as guarded, and records which reads
a fallback warning precedes. The guard outside Megatron Core scans
scripts/, docs/ and the top-level scripts and follows assigned aliases
and getattr(). The workflow fails on the checker's tests and on the new
removal-version gate, and runs the checker and the guard as report-only
steps. pyproject.toml no longer ignores ParallelStateDeprecationWarning
when the caller is one of this repository's modules outside the tests.

Describe that coverage, the :warned entries, the CI job, the test
filter and the removal-version gate, as
api-backwards-compatibility-check.md already does, and list the gate
among the checks that run locally.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
initialize_megatron's seeding change edits the same initialization
path that NVIDIA#8037 moves onto one process-group collection, so this
branch builds on both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The guide listed create_all_gather_groups() among the helpers that are
not deprecated, without condition. Since the bootstrap changes, a call
without a pg_collection falls back to the global grid with a
ProcessGroupFallbackWarning, and update_pg_timeout() is documented as
lifecycle API next to create_group(). Say so, in the words of
api-backwards-compatibility-check.md.

Those changes also make pretrain() and initialize_megatron() take
their groups from the registered collection, and read an unregistered
grid only behind a fallback warning. Add that to the registry section,
which tells frameworks to register the collection of their own grid.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
initialize_megatron seeds the random number generators from the model's
collection when pretrain passes one. Stock training passes none, so
_set_random_seed and model_parallel_cuda_manual_seed read the pipeline,
data-parallel, tensor, expert and GTP-remat ranks from the global
parallel state, although initialize_model_parallel has just returned
the collection of the grid it built.

_initialize_distributed now returns that collection, or None when it
builds no grid (model-parallel setup skipped, a grid already
initialized, or no CUDA device). initialize_megatron seeds from it
unless the caller passes seed_pg_collection, and falls back to the
global state only when no grid was built. The log lines that report the
grid sizes read the same collection, so initialization reads no global
group, rank or size once the grid exists.

The collection holds the groups that initialize_model_parallel
registers, so the seeds equal those of the global state, and the
collection that pretrain builds afterwards from the registered groups
holds the same group objects. With lazy_mpu_init, the provisional
tensor-parallel rank that initialize_megatron sets to the global rank
before the grid exists no longer replaces the group rank in the
tensor-parallel seed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
A framework that builds its own grid, for example with HyperCommGrid,
has no supported way to make code that still reads parallel_state use
that grid. initialize_model_parallel() registers its groups through a
private helper that needs every StandardGridExtras field, so
Megatron-Bridge MIMO and tools/checkpoint write parallel_state's
private module variables instead. destroy_model_parallel() also leaves
ten registered values behind: the hierarchical CP groups, the partial
data parallel group with CP and its Gloo group, and seven global rank
lists. After a teardown, use_mpu_process_groups() still returns the old
hcp and intra_dp_cp groups, and the source, first and last rank
accessors still return ranks of the old grid.

Add register_process_groups(pg_collection, *, extras=None) and
get_registered_process_groups():

- register_process_groups() sets the module variables that the
  accessors and use_mpu_process_groups() read, and records the
  collection. When extras is omitted, the global rank lists are the
  ranks of the collection's groups, the GTP-inclusive partial data
  parallel groups are the partial groups when GTP_remat is off, and
  the groups a collection cannot describe (the data parallel, data
  parallel with CP and expert data parallel Gloo groups, TP x DP) stay
  None. The intra partial data parallel Gloo groups come from the
  collection's intra_dp_cp_gloo and intra_expt_dp_gloo fields, with or
  without extras, like every other field it has.
  A field that is unset registers no group, so a collection that
  leaves dynamic_cp unset no longer fails in dict.update().
- It raises TypeError for another type and RuntimeError when groups
  are already registered, instead of an assert.
- initialize_model_parallel() is build_process_group_collection()
  plus register_process_groups(); get_registered_process_groups()
  returns the collection it returned.
- destroy_model_parallel() clears every value that registration sets,
  destroys the partial data parallel Gloo group like the other Gloo
  groups, and clears the registered collection.

The accessors keep reading the module variables, so existing writes
to them (Megatron-Bridge MIMO, tools/checkpoint, tests) still change
what the accessors and use_mpu_process_groups() return. On the
standard grid every group creation, barrier, module variable,
accessor and use_mpu_process_groups() field after initialization is
the same as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Passing the training collection to the optimizer with Gloo groups on
relies on the Gloo fields that NVIDIA#6259 adds to ProcessGroupCollection.
This branch is based on NVIDIA#8055, whose schedule fallback warning the
training collection test expects.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The optimizer change follows the one collection that NVIDIA#8037 passes
through pretrain into setup_model_and_optimizer, and its test extends
NVIDIA#8037's training collection test. 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>
setup_model_and_optimizer() handed get_megatron_optimizer() its
process-group collection only when Gloo process groups were disabled.
With Gloo groups enabled, which is the default, it passed None, so the
optimizer rebuilt every group from the global parallel state through
ProcessGroupCollection.use_mpu_process_groups(). A collection given to
pretrain() thus reached the model and the training loop but not the
optimizer, which sharded its state and reduced its gradient statistics
over the global grid instead.

The collection now carries the Gloo mirrors of the groups the
distributed optimizer shards over (intra_dp_cp_gloo and
intra_expt_dp_gloo), and get_megatron_optimizer() takes them from it
after checking that their ranks match. Pass the collection
unconditionally. The one pretrain() builds from the global grid holds
the same group objects that use_mpu_process_groups() returns, so the
standard grid trains on the same groups as before, and with
--disable-gloo-process-groups nothing changes.

A ProcessGroupCollection passed to pretrain() with Gloo groups enabled
must now carry them; get_megatron_optimizer() raises a ValueError
naming the missing field otherwise. A MiMo
MultiModuleProcessGroupCollection is unaffected, since MiMo models
build their optimizer per module and ignore the argument.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
With NVIDIA#8055, get_forward_backward_func() warns when it falls back to the
global grid. The training collection test calls that fallback on purpose to
check that the global grid decides without a collection, and under an error
filter for ProcessGroupFallbackWarning the warning failed the test. Assert
the warning instead, with the once-per-site registry emptied so that it
shows even when an earlier test triggered it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY and others added 23 commits October 29, 2026 17:28
Without CUDA, the owner_pg_collection fixture builds a collection with
only tp, cp and pp. Since resolve_gtp_remat_group warns before it falls
back for a missing gtp_remat or expt_gtp_remat field, the local linears
that the owners build on that collection each warned, so the CPU run
emitted two ProcessGroupFallbackWarnings and five of the owner tests
failed under -W error::ProcessGroupFallbackWarning.

Set both fields to None, as build_test_pg_collection does in the CUDA
branch: GTP is off on this one-rank grid, so the tests keep checking
only which collection each linear receives.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
resolve_gtp_remat_group now warns whenever it falls back to the global
grid, including for a collection that lacks the gtp_remat field.
Attention and MultiTokenPredictionBlock, built without a collection,
warn and build one from the global grid with only the groups they
list, and gtp_remat was not among them. Every kind of linear built on
that collection then fell back for gtp_remat and warned as well, so a
single omitted collection gave three warnings for SelfAttention,
CrossAttention and the MTP block: the owner's, ColumnParallelLinear's
and RowParallelLinear's.

Add gtp_remat to both lists. The collection reads it without requiring
an initialized grid, as the linears' own fallback did, so they get the
same group, None on an uninitialized grid, and the owner's warning is
the only one. These modules build dense linears only; an MoE layer
cannot be built on the MTP block's collection, which has no ep group,
so neither needs expt_gtp_remat.

DSAIndexer, DSAttention and SimplifiedDSGQAIndexer keep their tp and
cp lists: their linears are TELinear or the ModelOpt Linear, which take
an explicit GTP group or none, so they already warn once. MLA builds
its fallback from every group.

The new CPU tests build each of these owners without a collection on a
one-rank grid, expect exactly one warning, for the owner, and check
that the fallback collection holds the global GTP group. If their
fixture creates the default process group for that grid, it destroys it
again, so later tests in the session do not find a one-rank group.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
A MoELayer built without a collection takes get_default_pg_collection,
which warns for the omission and builds the layer, router and
dispatcher groups and the experts' data-parallel groups from the
global grid. It set neither GTP weight-remat group, so every kind of
linear built on it fell back for its own group and warned as well: the
local experts' ColumnParallelLinear and RowParallelLinear for
expt_gtp_remat, and the shared experts' for gtp_remat. One omitted
collection gave three warnings, five with shared experts.

Build gtp_remat and expt_gtp_remat too. They are read without
requiring an initialized grid, as the linears' own fallback read them,
and the check that parallel_state created every group skips them,
since None means that the axis is off; a grid without them builds as
before. SequentialMLP and the latent projections get the groups they
read before without a warning of their own; the shared experts do too
with GTP off, but with dense GTP on (gtp_remat has more than one rank)
their Transformer Engine linears still warn for pg_collection.dp_cp.

One path changes. TEGroupedLinear reads expt_gtp_remat from its
collection without a fallback, so grouped experts built on the default
collection saw no expert GTP group and stayed unsharded even when the
global grid sharded experts. They now shard over the global expert GTP
group, as SequentialMLP's experts and a TEGroupedLinear built without
a collection already did. With expert GTP off the group has one rank,
and nothing changes.

The MoE process-group test expects both fields in the default
collection and no warnings from the experts' linears. The new CPU
cases build a MoELayer without a collection, with and without shared
experts, and expect one warning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
On a grid that is not initialized, resolve_process_groups warns with
the list of groups that its fallback collection holds as None and says
that torch.distributed reads a None group as WORLD. Since the MoE
default collection also resolves gtp_remat and expt_gtp_remat, that
list named them as well. For those two fields None means that the GTP
axis is off, which is how resolve_gtp_remat_group and the Transformer
Engine linears read it, so calling them WORLD misdescribed them.

Leave the two fields out of the list. A fallback whose only None
groups are GTP groups gets the plain fallback warning, as a collection
without None groups already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
MoELayer's default collection now carries the global expt_gtp_remat
group, so two comments in test_moe_egtp.py that say it has none are
wrong, and the assignments of that group after them do nothing.

Phase 2 of the loss-trajectory test now builds its MoELayer without a
collection, as phase 1 does. Its GTPShardedParam assertion then checks
directly that a MoELayer built without a collection shards its grouped
experts over the global expert GTP group under EGTP_remat=2, which it
did not do before. The precision-override test keeps its explicit
default collection and only asserts the group size.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The one-warning tests built every owner from local linears, so nothing
checked that the Transformer Engine linears, which resolve their GTP
group through the same helper, also find it in the owner's fallback
collection. Add a GPU case for SelfAttention with the Transformer
Engine spec and for CrossAttention with Transformer Engine linears.
Before the fallbacks carried gtp_remat, each warned three times.

The module docstring claimed that every fallback collection carries
the GTP groups. The DSA owners' collections do not, since their
linears read none, and with GTP on a Transformer Engine linear still
warns for the replica group of its shards. Say so.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The MoE default collection and the Attention fallback collection leave
dp_cp unset. With dense GTP weight remat on, the linears built on them
therefore get no dp_cp replica group for their shards: the Transformer
Engine column, row and layer-norm column linears (the shared experts',
Attention's) warn when they are built, the latent projections do not,
and checkpoint saving elects the shard writers over parallel_state's
dp_cp group, with a warning. The docstring named only the TE
shared-expert linears and did not limit the case to dense GTP, which is
the only GTP whose replica group they read; the Attention comment still
said its linears do not warn again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The parallel_state check changes that follow fail the checker tests
on any read in Megatron Core that is neither warned nor guarded, so
they need the change that moves the MoE metrics and MTP placement
reads behind their warnings below them. Merge it onto the bootstrap's
registry reads and the config logger's process groups. The checks
need those as well: the guard's stricter rule for silenced bootstrap
blocks accepts the bootstrap code of megatron/training only as the
registry change rewrites it, and the documentation merged next
describes both changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The parallel_state check changes that follow also update the
deprecation guide and its review advice, which this documentation
branch adds and revises, so they need its text below them. The guide
also describes the bootstrap's registry reads and the config logger's
process groups, which are already below. Merge it onto them and the
core residual-read changes.

The merge conflicts only in the docstrings of parallel_state.py. They
keep this branch's code and its registry, accessor-warning and Gloo
group text, and take the documentation branch's docstring changes
where both sides edit the same lines.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The checker resolved only calls made through imports. An assigned
module alias (grid = parallel_state), getattr() with a constant name,
a reference that is not called (a callback, a default, a stored
function), a module looked up through importlib.import_module() or
sys.modules, and any read or write of the private globals of
parallel_state all passed it, although the outside-core guard catches
most of these forms.

Resolve every name, attribute and constant getattr() as a reference
instead of only calls. Follow assigned aliases within the checker's
scoped bindings, so that the existing shadowing rules still hold, and
follow self, cls and class-attribute aliases the way the guard does.
Count a star import, and add a private kind: the private state of
parallel_state is not an interface, so its entries are never warned.
An alias of a counted function is not followed; the reference that
makes it counts instead.

The tree has none of the new forms: the per-line results over
megatron/core are identical to the previous checker, so the allowlist
does not change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The outside-core guard accepted any warnings.catch_warnings() block
that ignores ParallelStateDeprecationWarning as bootstrap code, so a
read of the global grid wrapped that way passed silently, and strict
mode does not cover that warning class. It also never looked at the
private attributes of parallel_state, which bypass the deprecation
warning altogether.

Accept such a block only after the code has reported the read itself:
an earlier statement of the same function, in the block of the with
or in a block that contains it, calls warnings.warn() with that
category or warn_global_process_group_fallback(). Only a spelling of
warnings.warn() that the file's imports bind counts (warnings.warn, an
alias of the warnings module, or warn imported from it), and the
category's final name must match exactly, so logger.warn() or a
category whose name merely contains the warning's does not. This is
what the lazy initialization in megatron/training/initialize.py does,
the only such block in the tree. Flag every read or write of a private
parallel_state attribute, in any block. The private-named
_set_global_memory_buffer() is the exception: it is a deprecated
function, which the checker in tools/ already treats as one, so the
guard does too and a fallback block allows it in both tools. The tree
passes unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Review found read forms that neither the checker in tools/ nor the
guard of the code outside Megatron Core followed, although the docs
said both flag writes of private state and follow star imports:

- setattr() and delattr() with a constant name, the write and delete
  forms of getattr();
- an alias through either side of a conditional expression or an
  operand of and and or;
- importlib.import_module() with a keyword name, or with a relative
  name and its package;
- a star import of megatron.core, which exports parallel_state;
- in the checker only, which binds names per scope: a name declared
  global or nonlocal and bound in another function, and the target of
  an assignment expression in a comprehension, which binds in the
  enclosing scope. The guard binds aliases file-wide and saw these.

Follow all of them. The checker looks for global and nonlocal
statements only in the statements of a file that contains one of the
words, which keeps the extra cost of a scan of megatron/core near
0.3 s. Neither tool finds anything new in the tree. Document the
forms that remain unfollowed (parameter defaults, for and with
targets, class-body imports reached through self), and that an
attribute alias such as self.grid counts in every class of its file,
subclasses included.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Nothing that blocks a merge caught a new unwarned read of the global
grid in Megatron Core: the ratchet step is report-only until NVIDIA#6307
decides whether new fallbacks may be added, TestAllowlist checks only
the allowlist file, and calls from Megatron Core never emit
ParallelStateDeprecationWarning, so even strict mode stays silent.

Add a test that runs the checker's classification over the tree and
requires every counted read, allowlisted or not, to be guarded or
warned, apart from the reads in UNWARNED_READS. It runs in the blocking
"Test the process-group usage checker" step and in the unit-test
bucket. A new warned fallback still passes: how many of them Megatron
Core may add stays with the report-only ratchet.

A file that UNWARNED_READS lists but the scan no longer finds, because
it was moved or deleted, is reported as a stale entry instead of
making the test error. The workflow comment, the deprecation guide and
the compatibility doc describe the new failure; the compatibility doc
also says that the test lists no such reads now, since the MoE metrics
and MTP placement reads already warn first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The checker's tests, the outside-core guard and the removal-version
gate read the source as text, so Testmon never maps a source change to
them and a selective unit-test run skips them, even for a pull request
that adds a read of the global grid.

Map the folders they scan, and each top-level script, to the three
files in tests/unit_tests/testmon_mandatory_tests.yaml. The top-level
scripts are listed one by one: a source entry must be an existing path
(test_repository_config_is_valid), and the fnmatch glob *.py would also
match every file in every folder. Only the H100 buckets get the
mapping, because the files carry no launch_on_gb200 marker and the
GB200 buckets would drop them. A new test checks that the mapping
covers every folder the three files scan and every top-level script,
and that the H100 catch-all bucket runs them.

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

The compatibility doc listed private parallel_state uses among the calls
the checker counts as guarded, but no fallback warning makes a private
use acceptable, and the checker tests fail on it. The one exception is
the private-named deprecated _set_global_memory_buffer(), which the
checker and the guard treat as a deprecated function, so a fallback
warning does cover it; even so, test_global_memory_buffer_imports.py
rejects the memory-buffer aliases from parallel_state under megatron/,
examples/, tools/ and the top-level scripts, fallback or not. The guide
also said that no code reads or writes a private parallel_state
attribute, without deletions or that carve-out. Say so in each text.

The doc's bootstrap rule required a warning "with that category", which
rejects a block after warn_global_process_group_fallback() that the
guard accepts, and its wording ("an earlier statement of the same
function, in the block of the with or in a block that contains it,
calls warnings.warn()") reads as accepting a warning inside an earlier
if branch, loop or with, an assignment such as `_ = warnings.warn(...)`,
or a warning inside the catch_warnings() block, all of which the guard
flags, and as excluding module-level bootstrap code, which it accepts.
Make the guide's guard paragraph, which the other texts point to, the
exact statement of what the guard checks: a bare call that comes before
the with, in the block that holds it or in a block that contains that
block; a reporting warnings.warn() that passes the category as its
second argument or as category=; a catch_warnings() body that calls
warnings.simplefilter("ignore", ParallelStateDeprecationWarning) or the
matching filterwarnings(), while catch_warnings(action=...,
category=...), a blanket ignore and an ignore of FutureWarning are
flagged although they ignore the warning at run time; and no cover for
a reference in a function or lambda defined inside either block. Spell
the warning the same way in the guard docstring and the compatibility
doc, link the doc to the guide, and stop saying that the silenced block
"ignores ParallelStateDeprecationWarning".

The workflow comments said a selective Testmon run of the pull request
that bumps the version can skip test_removal_versions.py; the mandatory
mapping adds it to the runs that change a file the mapping lists. They
also described the core reads that fail the step as the unwarned ones
only, and, like the compatibility doc, still called the ratchet steps
the place where a call that lost its warning shows up. TestTree's
message described its failures as reads that no warning precedes, which
a private read inside a fallback block is not.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The checkpoint-converter test saves its source checkpoint with
num_floating_point_operations_so_far=None. Since NVIDIA#7516 save_checkpoint
copies that value into the TrainState that set_global_variables()
creates, and TrainState.state_dict() turns it into a float64 tensor,
which fails for None ("must be real number, not NoneType") before
anything is written. main fails the same way; the recipe runs the test
only in its broken scopes, so nothing noticed.

Pass 0, the count a run starts from, so that the test reaches the
conversion and the reload again. The count is not compared anywhere.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The functional checkpoint-converter test read the global parallel grid
through eleven deprecated parallel_state calls, and it called get_model,
get_forward_backward_func, the pipeline schedule, the RNG seeding and
the tensor-parallel gather without process groups, so each of them fell
back to the grid as well. Every one of those paths warns since 0.21 and
stops working when the accessors and fallbacks are removed in 0.23, and
no check scanned the file.

Each pipeline now keeps the collection that initialize_model_parallel()
registers for the grid it sets up, read with
parallel_state.get_registered_process_groups(), and passes it, or its
groups, to every call that needs them. The groups are the ones the
accessors returned, so the ranks, the stage checks, the broadcast
source (the first rank of the tensor-parallel group), the embedding
all-reduce, the schedule and its pipeline communicator, the RNG seeds
and the order of the collectives stay the same. The methods that read
the groups become instance methods so that they can reach the
pipeline's collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The guard that keeps deprecated parallel_state calls out of the code
outside Megatron Core skipped all of tests/, so the functional
checkpoint-converter test could call eleven deprecated accessors that
will be removed in 0.23 without anything reporting them. The functional
tests run Megatron end to end, like the entry points the guard already
covers; only the unit tests use the global grid on purpose.

Add tests/functional_tests to the scanned folders. The test above now
uses its process groups, and nothing else under that folder calls a
deprecated function, so the real tree still passes; without that
change the guard lists the converter test's eleven calls.

The comment above SCANNED_FOLDERS now names every test folder the
guard still skips and why: the unit tests use the global grid on
purpose, and tests/test_utils and tests/performance_tests hold CI
scripts that do not import Megatron. Add tests/functional_tests to the
Testmon mandatory mapping as well, so that a change under it selects
the parallel_state source checks in a selective unit run, as the
mapping's consistency test requires for every scanned folder.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The deprecation guide lists the folders that
tests/unit_tests/test_parallel_state_accessor_uses.py guards, and the
guard now scans tests/functional_tests as well. Add it to the list so
the guide matches the folders the guard checks.

The guard docstring and the compatibility doc still said that the
guard skips the tests as a whole. Only the unit tests use the global
grid on purpose and are skipped, so say that in both places.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The process-group section of AGENTS.md, which CLAUDE.md links to,
predates the deprecation of the parallel_state accessors. It allows
"migration fallbacks with an explicit comment" and tells agents not to
apply the rules to megatron/training. Neither holds any more:
outside Megatron Core, test_parallel_state_accessor_uses.py rejects a
deprecated call in megatron/training, examples/, tools/, tasks/,
scripts/, docs/ and the top-level scripts unless it sits in a
global_process_group_fallback() block or in bootstrap code that
ignores the accessor warning, and under pytest pyproject.toml turns
the warning into an error for the megatron, examples, tools, tasks and
scripts packages and the top-level scripts it names. In Megatron Core,
a fallback warns through the helpers of process_groups_config.py, and
the checker fails on any read that its allowlist does not hold, warned
or not. An agent that followed the old text would add a commented,
silent read or skip the training code that the guard now covers.

State the current rules instead: groups come from the owning model's
collection; what is deprecated, in which releases, and what is not;
compatibility paths go only through the warned helpers, dated 0.21
and 0.23 by default, which warn when they fall back
(resolve_gtp_remat_group(owner=None) does not warn, since only a
caller that has already warned passes it); what the checker counts and
allows in megatron/core; what the guard covers and allows outside it,
including a block opened through a function of the same module that
returns one, as megatron/training/checkpointing.py does, and the
skipped tasks/eval_utils.py, which imports a module that no longer
exists; how to find fallbacks with strict mode; and what reviewers
flag. Point at the deprecation guide for the details.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The light and strict references of the pr-review skill carry their
own copy of the process-group guidance that AGENTS.md had: they let
"explicitly documented migration fallbacks" through, and the strict
one tells the reviewer not to apply the rules to megatron/training.
Since the parallel_state accessors are deprecated, a reviewer that
follows them would pass a commented, silent read of the global grid,
and would skip the training code that
test_parallel_state_accessor_uses.py now guards.

State the rules that AGENTS.md now states: in megatron/core, any new
call of a deprecated function, of the shim or registry getter, or of
a guarded helper is a finding unless it replaces an existing read in
the same function; outside megatron/core and the tests, a deprecated
call is a finding unless it is in a global_process_group_fallback()
block or in bootstrap code that ignores the accessor warning. The
strict rubric gives the helpers' dates, 0.21 and 0.23, as defaults: a
fallback that first warns in a later release passes its own
deprecated_in and removed_in, and is not a finding for its dates.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The "Reviewing?" paragraph of the deprecation guide still lists
"bootstrap code that materializes a collection from the globals" and
"warned compatibility fallbacks" as allowed in megatron/core. The
checker allows neither: outside parallel_state.py and
process_groups_config.py, it fails on any new read in megatron/core,
warned or guarded, unless it replaces an allowlisted read in the same
function, and it also counts get_registered_process_groups() and the
guarded helpers. The paragraph also says nothing about the code outside
Megatron Core, which the guard now covers, megatron/training included.

Say what the checker and the guard flag, as the "The CI check" section
of the same guide and AGENTS.md describe them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
AGENTS.md, the light and strict pr-review rubrics, the guide's
reviewing paragraph and the contribution policy restate the
process-group rules, and the checks they describe are now stricter.
The checker counts every use it can follow, including private
parallel_state state, and its tests fail the process-group usage job
on a read in megatron/core that is neither warned nor guarded and on a
private use; the guard accepts a silenced bootstrap block only after
the code warned on its own, never accepts a private attribute, and
also scans tests/functional_tests. Without this, an agent or reviewer
reading the rules would accept a silent catch_warnings() block or a
private write that CI rejects, would think that nothing blocks an
unwarned core read, and would exempt the functional tests.

- Only the unit tests may use the global grid. A functional test that
  runs Megatron end to end follows the same rules as the entry points,
  so list it with the guarded code.
- The contribution policy called the job report-only and let bootstrap
  code call use_mpu_process_groups() without qualification. Of its
  megatron/core steps, only the allowlist ratchet is report-only now.
  Say which reads in megatron/core the job fails on, what the checker
  counts, and that a function defined inside a fallback block is not
  covered by it.
- State the bootstrap rule once, as the guide does, and point to the
  guide for the details: a bare call of warnings.warn(...,
  ParallelStateDeprecationWarning) or warn_global_process_group_fallback()
  before the with, in the block that holds it or in a block that
  contains that block. The texts said "in the same function", "of the
  same function or at module level" or "an earlier statement of the
  same function ... calls warnings.warn()", or left out the category,
  which read as accepting a warning inside an earlier branch, loop or
  with, inside the catch_warnings() block, of another category, or at
  module level before the function that holds the block, and as
  excluding module-level code.
- Name the filter the guard accepts: a body call of
  warnings.simplefilter("ignore", ParallelStateDeprecationWarning) or
  the matching filterwarnings(). The guard flags
  catch_warnings(action=..., category=...), a blanket ignore and an
  ignore of FutureWarning, which "ignores
  ParallelStateDeprecationWarning" let through.
- Private parallel_state uses: both checks treat the deprecated
  _set_global_memory_buffer() as a deprecated function that a fallback
  warning covers, but test_global_memory_buffer_imports.py rejects
  every memory-buffer alias taken from parallel_state under megatron/,
  examples/, tools/ and the top-level scripts, fallback or not, so the
  buffer comes from megatron.core.utils. A private use fails the job
  only in the code the checker scans. AGENTS.md also said the guard
  allows such calls only in bootstrap code, which the guard has no
  notion of, and the light rubric had dropped the bootstrap qualifier
  that the other rubrics keep for reviewers.

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

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@Connor-XY

Copy link
Copy Markdown
Contributor Author

Closing: the agent and review rule updates moved into #8773, next to the checks they describe. Please review them there.

@Connor-XY Connor-XY closed this Nov 1, 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