Skip to content

Enforce process-group checks in CI, functional tests and review rules - #8773

Closed
Connor-XY wants to merge 606 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-h1-checks
Closed

Connor-XY wants to merge 606 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-h1-checks

Conversation

@Connor-XY

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

Copy link
Copy Markdown
Contributor

Depends on #7991, #8730, #8768 and #8772. Please review the diff from their merge: scoped diff.

This PR takes over #8774, which ran the checkpoint-converter functional test on its registered groups, and #8776, which aligned the agent and review rules with these checks.

Why.

  • Nothing that blocks a merge caught a new unwarned read of the global grid in Megatron Core. The ratchet step is report-only until [TRACKING] Deprecate parallel_state: explicit process groups in megatron/core #6307 decides on new fallbacks, TestAllowlist checks only the allowlist file, and calls from Megatron Core never emit ParallelStateDeprecationWarning, so strict mode stays silent too.
  • 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 module looked up through importlib or sys.modules, and any use of parallel_state's private globals all passed it.
  • The guard for code outside Megatron Core accepted any catch_warnings() block that ignores ParallelStateDeprecationWarning as bootstrap code, so a read wrapped that way passed silently. It never looked at private attributes, which bypass the deprecation warning altogether. It skipped all of tests/, functional tests included.
  • The checker tests, the guard and the removal-version test read the source as text. Testmon never maps a source change to them, so a selective unit-test run skips them.
  • The functional checkpoint-converter test (tests/functional_tests/test_cases/common/ckpt_converter) read the global grid through 11 deprecated parallel_state calls. It also 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 global grid. Since save train state for checkpoints #7516 its save fails with a TypeError, on main too, because it passes num_floating_point_operations_so_far=None. The recipe runs the test only in its broken scopes, so nothing noticed.
  • 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. The first six commits tighten the checker and the guard.

  • The checker (tools/check_process_group_usage.py) counts every reference to a counted function, called or not. It follows:
    • assigned aliases within its scoped bindings, including names declared global or nonlocal, walrus targets in comprehensions, either side of a conditional expression and the operands of and and or;
    • self, cls and class-attribute aliases, as the guard does;
    • getattr(), setattr() and delattr() with a constant name;
    • importlib.import_module() with a positional or keyword name, and relative names with their package;
    • sys.modules lookups and star imports, megatron.core's included.
  • A new private kind counts reads, writes and deletions of parallel_state's private attributes. They are never warned. The deprecated _set_global_memory_buffer() stays a deprecated function. Each alias binding is resolved once per pass, so long reassignment chains cannot stall CI.
  • The tree has none of the new forms. The per-line results over megatron/core are identical to the previous checker's, and the allowlist does not change.
  • The guard (tests/unit_tests/test_parallel_state_accessor_uses.py) flags every use of a private parallel_state attribute, including through setattr() and delattr(), and follows the same new alias and lookup forms.
  • The guard accepts a silenced block only after the code has warned on its own: a statement that is only a 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. Only a spelling of warnings.warn that the file's imports bind counts, and the category name must match exactly, so logger.warn() does not. The lazy initialization in megatron/training/initialize.py is the only such block in the tree.
  • New TestTree in test_check_process_group_usage.py runs the checker's classification over megatron/core. It fails on a counted read that is neither warned nor guarded, apart from the reads in UNWARNED_READS (none now), and on any private use. It also reports stale UNWARNED_READS entries, including those of a moved or deleted file. It runs in the blocking "Test the process-group usage checker" step and in the unit-test bucket.
  • tests/unit_tests/testmon_mandatory_tests.yaml maps the folders these three tests scan, and each top-level script, to the three test files, for the H100 buckets. A new test in tests/test_utils/test_unit_testmon_mandatory.py checks that the mapping covers every scanned folder and top-level script.
  • The guide, docs/api-backwards-compatibility-check.md and the workflow comments describe the new failures, the private reads, and the exact bootstrap rule. The guide's guard paragraph is the reference statement of that rule: which filter forms the guard accepts (simplefilter or filterwarnings with "ignore" and the category, in the block's body), which it flags (catch_warnings(action=..., category=...), a blanket ignore, an ignore of FutureWarning), and that a function or lambda defined in the block gets no cover. The docs also say that test_global_memory_buffer_imports.py rejects memory-buffer aliases from parallel_state, fallback or not.

The next four commits move the checkpoint-converter functional test onto its registered groups.

  • One commit saves with num_floating_point_operations_so_far=0, the count a run starts from. It is independent of the rest and applies to main on its own. Would maintainers rather take it as a separate PR?

  • Each pipeline keeps the collection that initialize_model_parallel() registers for its grid, read with parallel_state.get_registered_process_groups(), and passes it or its groups to every call that needs them:

    • ranks and stages from pg.tp, pg.pp, is_pp_first_stage() and is_pp_last_stage(), and the first rank of pg.tp as the broadcast source;
    • get_model(pg_collection=...) and model_parallel_cuda_manual_seed(123, pg_collection=...);
    • get_forward_backward_func(pp_size=pg.pp.size(), vp_size=None), and the schedule with pg_collection and a P2PCommunicator on pg.pp;
    • the embedding all-reduce over pg.embd, and gather_from_tensor_model_parallel_region(group=pg.tp).

    The ranks, stage checks, broadcast source, RNG seeds and the order of the collectives stay the same. The helpers that read the groups become instance methods.

  • The guard also scans tests/functional_tests. Functional tests run Megatron end to end like the entry points the guard already covers, and only the unit tests use the global grid on purpose. The comment above SCANNED_FOLDERS names every folder the guard skips and why. tests/functional_tests joins the Testmon mandatory mapping, so a change there selects the parallel_state source checks in a selective unit run.

  • The guide lists tests/functional_tests/ among the guarded folders. The guard docstring and docs/api-backwards-compatibility-check.md say that the unit tests, not all tests, are skipped.

The last four commits align the agent and review rules. They change docs and agent guidance only.

  • AGENTS.md states 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.

  • New self-tests for each read form, the guard's bootstrap and private rules, the stale-entry report and the Testmon mapping. test_many_reassignments_of_a_name_resolve_quickly fails if resolving 60 reassignments takes more than 10 s.
  • Before the sync, on 4 CPU ranks with Gloo, test_check_process_group_usage.py, test_parallel_state_accessor_uses.py, test_removal_versions.py and test_unit_testmon_mandatory.py: 93 passed on the base and 116 on this branch, with no failures. Without the converter change, the guard lists the converter test's 11 deprecated calls.
  • The checker on this branch and on the base, before the sync: "OK: no new uses of global parallel_state (101 grandfathered: 57 guarded, 44 warned, 0 unwarned)".
  • The bootstrap rule, the accepted filter forms and the memory-buffer rule in the docs were checked by running the guard, the checker and test_global_memory_buffer_imports.py on small examples of each form.
  • A CPU run of the converter harness before the sync, with Gloo and the local transformer implementation, without the conversion step, which needs CUDA, gives bitwise-identical outputs before and after the change on five TP and PP grids. It raises no deprecation or fallback warning after the change, against 9 to 13 per rank before.

On 4 GPUs against its base, no test regresses:

  • test_check_process_group_usage.py: 70 passed here vs 52 on the base (202 vs 130 subtests). The 18 new tests pass.
  • test_parallel_state_accessor_uses.py: 11 passed here vs 7 on the base (93 vs 49 subtests). The 4 new tests pass.
  • test_unit_testmon_mandatory.py: 28 passed here vs 27 on the base. The new test passes.
  • Four files this PR does not touch, test_parallel_state_deprecation.py, test_example_process_groups.py, transformer/test_owner_linear_pg_collection.py and inference/high_level_api/test_apis.py, pass 185 tests on each side.
  • tests/functional_tests/test_cases/common/ckpt_converter/__main__.py errors at pytest collection on both sides, because it is a functional-test entry point that the recipe runs with torchrun -m.

In the integration run (main 77391e7 with all 93 #6307 PRs and 6 related fixes merged, against main), this PR's test files in tests/unit_tests show no regressions against main. On 4 GPUs there, test_check_process_group_usage.py passes 70 tests against 29 on main. test_parallel_state_accessor_uses.py, which main does not have, passes 11.

The converter test ran on GPUs only before the 2026-11-01 sync, on the stacked version of these commits, where they were three PRs:

  • With all the other process-group changes from [TRACKING] Deprecate parallel_state: explicit process groups in megatron/core #6307 applied, the converter's TP2 PP2 round trip passes on 4 GPUs, with no ParallelStateDeprecationWarning or ProcessGroupFallbackWarning.
  • On that tree the full converter test module on 8 GPUs (two nodes) passes all 7 GPT pipelines: t8p1 to t1p8, t4p2 to t2p4, t2p4 to t4p2, t1p8 to t8p1, TE to local, local to local and local to TE. The output MSE against the source model is 0 to 1.7e-07 after loading, against 6.9e-03 to 2.3e-02 after the test re-initializes the weights at random. No log has a deprecation or fallback warning.
  • On main with only the save fix, the round trip still fails in the conversion with model_provider() missing 1 required positional argument: 'model_builder'. Run tools/checkpoint on registered process groups without rank overrides #8762 fixes that loader bug.

Compatibility.

  • No change to Megatron Core or the training code: the checker, the guard, tests, the CI workflow, docs and agent guidance. The allowlist is unchanged, and the guard passes on the tree.
  • Newly blocking: the "Test the process-group usage checker" step fails on a counted read in megatron/core that is neither warned nor guarded, and on any private parallel_state use there. The ratchet step stays report-only, and a new warned or guarded read still passes this test. How many fallbacks Megatron Core may add stays with [TRACKING] Deprecate parallel_state: explicit process groups in megatron/core #6307.
  • TestTree adds about 2.3 s to that step and to the unit-test bucket.
  • The guard now reports deprecated parallel_state calls in functional tests. Nothing else under tests/functional_tests calls one.
  • A core call of resolve_gtp_remat_group(..., owner=None) still counts as guarded, although it does not warn. Its three callers warn before the call. Should a follow-up classify these calls by the warning before them, like the other reads?
  • Neither tool follows a module passed as an argument or a parameter default, returned, stored in a container, or bound by a for or with target; an import in a class body reached through self; getattr() with a name built at run time; vars(parallel_state)[...], parallel_state.__dict__[...], __import__() or sys.modules.get(). An attribute alias such as self.grid counts in every class of its file.
  • A new top-level script needs a line in testmon_mandatory_tests.yaml. test_unit_testmon_mandatory.py checks that, but no GitHub workflow runs tests/test_utils. The mapping does not list test_inference_entry_point_groups.py, whose static checks a selective run therefore skips. Should a follow-up add it?
  • The recipe tests/test_utils/recipes/h100/ckpt_converter.yaml stays in its broken scopes (mr-broken, mr-github-broken). Should it be re-enabled, given the GPU runs above? The recipe also checks out a reference tree for its backward-compatibility step, which these runs did not cover.
  • Not exercised in the converter test: the LLaVA pipeline (commented out in test_all_pipelines) and the MoE pipelines (marked to-do). vp_size=None is deliberate: the old fallback read the grid's virtual-pipeline size, which Utils.initialize_model_parallel leaves None. The harness passes the registered collection object itself, with all its fields set.
  • 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 base contains registered kernel files. This PR's own diff passes the coverage check.

The branch was synced with main 77391e7 on 2026-11-01 by a merge of its updated bases, with no rebase or force-push, so existing commits and review links stay valid. That merge had no conflicts.

Later on 2026-11-01 the branch also merged main bfab98f. Its #8471 moves FullyShardedDataParallelV1 and FullyShardedDataParallelV2 from mcore_fsdp_adapter.py into mcore_adapter_v1.py and mcore_adapter_v2.py, and keeps mcore_fsdp_adapter.py as the factory that re-exports them. The merge carries its bases' edits to those classes where the code moved; this PR's own diff does not change. On 4 GPUs against its new base, the Megatron-FSDP adapter tests (7 files) show no regressions: 80 passed here, 80 on the base. The two CUDA-graph end-to-end cases of test_mcore_fully_sharded_data_parallel.py fail on both sides, as they do on main.

Part of #6307.

🤖 Generated with Claude Code

@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 and others added 23 commits October 30, 2026 14:46
The guard tests that follow extend NVIDIA#8724's
test_sequential_mlp_process_groups.py, and the checker classification
requires the SequentialMLP experts to read their collection's expert
groups instead of the global grid.

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

The guard tests that follow edit NVIDIA#8725's test_mfsdp_gtp_weight_remat.py.

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

The checker classification that follows requires every allowlisted
read in megatron/core to be warned or guarded. NVIDIA#8735 warns the global
fallbacks of finalize_model_grads and SimplifiedDSGQAIndexer, and
brings NVIDIA#8027 and NVIDIA#8049's fallback warning class.

The merge conflicts only in the process-group usage allowlist, which is
regenerated on the merged tree.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The checker classification that follows requires every allowlisted
read in megatron/core to be warned or guarded. NVIDIA#8749 warns the global
fallbacks of Megatron-FSDP v1, Torch FSDP and the FSDP-DTensor expert
offsets, and brings NVIDIA#8017, NVIDIA#8045 and NVIDIA#8051. The guard tests that follow
edit the mfsdp_v1 test_mcore_fsdp_dist_index.py that NVIDIA#8749 extends.

The merge conflicts only in the process-group usage allowlist, which is
regenerated on the merged tree.

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

The fallbacks that follow give the Attention and MTP owners the GTP
group of their linears, in the attention.py code that NVIDIA#8729 moves onto
the module's groups, and NVIDIA#8729 brings NVIDIA#6293.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The one-warning tests that follow check that a MoELayer built without a
collection gives its experts the GTP groups of the global grid, and
read them from experts.pg_collection. NVIDIA#8732's 645cd0d makes
SequentialMLP and TEGroupedMLP keep their collection; without it that
test case fails with "'SequentialMLP' object has no attribute
'pg_collection'". NVIDIA#8732 brings NVIDIA#8044.

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

The fallbacks that follow give the Attention, MTP and MoE owners the
GTP groups of their linears, and the one-warning tests extend NVIDIA#8748's
test_owner_linear_pg_collection.py: a Transformer Engine owner without a
collection must warn once, which needs NVIDIA#8748 to pass the owner's
collection to its linears. NVIDIA#8748 brings NVIDIA#8017 and NVIDIA#8035.

NVIDIA#8035, which NVIDIA#8748 brings, meets NVIDIA#6293, which NVIDIA#8729 brings, in three
files. They are resolved as follows:
- transformer_engine.py: the process_groups_config import takes both
  sides (amax_reduction_group and warn_global_process_group_fallback).
- t5_model.py: keep NVIDIA#6293's side. T5Model already resolves an omitted
  collection with every global group at the top of its constructor,
  which includes the amax groups NVIDIA#8035 adds.
- multi_token_prediction.py: keep NVIDIA#6293's fallback warning and hcp
  group, and add NVIDIA#8035's tp_cp and tp_dp_cp to required_pgs.
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>
The parallel_state check changes that follow also update the
deprecation guide and its review advice, which NVIDIA#7991 adds and revises,
so they need its text below them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The deprecation-guide lines that follow cite
tests/unit_tests/test_global_memory_buffer_imports.py, which NVIDIA#8730 adds
when it takes the global memory buffer from megatron.core.utils. NVIDIA#8730
brings NVIDIA#8026.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#6303 makes LanguageModule require an explicit process-group
collection, on top of NVIDIA#6293's deprecated global fallbacks, and imports
ProcessGroupCollection at module level in
tests/unit_tests/dist_checkpointing/utils.py and
tests/unit_tests/training/test_param_norm.py. The optimizer tests that
pass their process groups to get_megatron_optimizer build
ProcessGroupCollection objects in those files, and without this import
they raise NameError, so merge NVIDIA#6303 here. The merge is clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#8017 adds test helpers that forbid global grid reads and build
process-group collections; its head already merges NVIDIA#6259 and sets the
Gloo fields of build_test_pg_collection. The clip_qk and optimizer
fallback tests use the helpers, so merge NVIDIA#8017 here. The merge is
clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#8043 lets clip_qk reduce over the caller's data-parallel group and
passes it the model's DP x CP group. The clip_qk fallback warning that
follows changes the same code, so merge NVIDIA#8043 here. The merge is clean.

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


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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#6303 makes LanguageModule require an explicit process-group
collection, on top of NVIDIA#6293's deprecated global fallbacks. It passes
pg_collection=ProcessGroupCollection.use_mpu_process_groups() to the
GPTModel calls of the hybrid-interop, layer-wise optimizer, optimizer
and DSA/Mamba equivalence tests, and imports ProcessGroupCollection at
module level in test_gpt_hybrid_interop.py and test_optimizer.py. The
layer-placement change replaces those five keyword lines with one
collection per test, and its new lines use that import; without NVIDIA#6303
they raise NameError. So merge NVIDIA#6303 here. The merge is clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#8017 adds test helpers that forbid global grid reads and build
process-group collections; its head also carries NVIDIA#6259's Gloo fields of
the collection. The layer-placement tests call forbid_global_process_groups,
so merge NVIDIA#8017 here.
The merge is clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#8028 passes the combined 1F1B schedule its CP size and last-stage
flag from the model chunk's process groups and adds
test_combined_1f1b_process_groups.py. The layer-placement fallback
change edits that test, so merge NVIDIA#8028 here. The merge is clean.

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,
adds test_pipeline_parallel_layer_layout.py, and sets the checkpoint
converter's pipeline rank on its fake group. The layer-placement
fallback warning changes the default-rank path of that layout code and
its tests, so merge NVIDIA#8033 here.

Only the process-group usage allowlist conflicts, on its total (172 on
this branch, 195 on NVIDIA#8033). It is regenerated on the merged tree:
170 reads (151 accessor, 19 shim). The entries equal this branch's
side; NVIDIA#8033 changes no read, and this branch's file kept a stale total.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY and others added 15 commits November 1, 2026 09:00
Conflicts:
- megatron/core/transformer/cuda_graphs.py: The PR added
  CudaGraphManager._resolve_reuse_cudagraphs and main added the static
  _initialize_mempool at the same spot. The result keeps both methods in
  the order the base PR uses. Both reuse_cudagraphs call sites still go
  through _resolve_reuse_cudagraphs.
- megatron/core/transformer/moe/batch_invariant.py: Main (NVIDIA#8559) split
  unpermute into _unpermute_loop, which takes a required ep_size, and a
  public dispatcher that read the global EP size itself. The result keeps
  main's split and Triton kernels. The PR's optional ep_size keyword,
  its docstring sentence and its resolve_process_groups fallback move
  into the new public unpermute and replace main's global read.
- tests/unit_tests/ssm/test_gated_delta_product.py: Both sides added
  imports. The result keeps main's Float16Module and
  is_causal_conv1d_min_version imports and the PR's global_amax_groups
  import.

Adapted to main outside the conflicts:
- tests/unit_tests/transformer/moe/test_moe_batch_invariant_unpermute.py:
  Main's new test set the EP size through
  parallel_state.set_expert_model_parallel_world_size, which unpermute no
  longer reads. The ep_world_size fixture is removed and each unpermute
  call passes ep_size explicitly, as the PR does for the RL tests.
- tests/unit_tests/determinism/kernels/test_moe_kernels.py: Main's new
  test_batch_invariant_unpermute_replays patched
  get_expert_model_parallel_world_size. The patch is removed and the
  unpermute call passes ep_size=2.

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- examples/inference/advanced/gpt_dynamic_inference.py,
  examples/inference/advanced/gpt_static_inference.py,
  examples/inference/launch_inference_server.py,
  examples/inference/offline_inference.py,
  tools/run_dynamic_text_generation_server.py: main replaces
  build_tokenizer with get_tokenizer(). The result keeps the PR's
  get_registered_process_groups() model and engine builds and main's
  tokenizer changes.
- tools/run_text_generation_server.py: main builds the model through
  get_model_builder(args, cfg.tokenizer, ...) with a pg_collection from
  use_mpu_process_groups(). The result keeps main's builder and takes the
  PR's registered groups and bound ModelOpt builder.
- megatron/inference/utils.py: main adds a tokenizer_config argument to
  get_model_builder and switches to get_tokenizer(). The result keeps
  main's calls inside the PR's global_process_group_fallback block and
  the PR's pg_collection pass-through.
- examples/t5/pretrain_t5.py: main drops build_tokenizer. The result
  keeps the PR's get_pg_rank import for the tp_group passed to get_batch.
- megatron/training/training.py: main moves and rewrites
  _get_model_pg_collection. The PR's change is re-applied to main's
  version, so the fallback read runs inside
  global_process_group_fallback.
- pretrain_gpt.py, pretrain_hybrid.py: main moves is_dataset_built_on_rank
  and core_gpt_dataset_config_from_args into shared helpers. The result
  takes the move and keeps the PR's get_batch fallback block.

Adapted to main outside the conflicts:
- megatron/training/datasets/providers.py: the PR's edit to
  is_dataset_built_on_rank is re-applied where main moved it, so the
  fallback read runs inside global_process_group_fallback. The unused
  warn_global_process_group_fallback import is removed.
- tests/unit_tests/inference/test_inference_entry_point_groups.py: the
  PR's test stubs get_run_config() and get_tokenizer() and accepts the
  tokenizer_config argument, matching main's tokenizer refactor.
- tests/unit_tests/test_example_process_groups.py: Main's BERT model
  provider reads the run config before the arguments (NVIDIA#8391). The
  provider test stubs get_run_config as well as get_args.

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

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- megatron/core/extensions/transformer_engine.py: In
  split_te_layernorm_column_parallel_linear, this PR passes the fused
  layer's pg_collection to the new TEColumnParallelLinear, and main
  (NVIDIA#8426) moves the new linear onto the fused layer's weight device. The
  result keeps both. The split linear gets the owner's collection and is
  then moved with .to(device=fused_layer.weight.device).

Also in this merge:
- tests/unit_tests/transformer/test_owner_linear_pg_collection.py: The
  fused-MLP test no longer checks MLP.pg_collection, which NVIDIA#8044 adds
  and this PR's base does not have.

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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
…ir update to main

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- megatron/core/distributed/fsdp/mcore_fsdp_adapter.py: Main adds the
  ShortcutMoEBlock import (NVIDIA#8312) and the PR adds the
  resolve_process_groups import. The result keeps both in isort order,
  along with the PR's _init_dist_index rewrite, which no longer reads
  parallel_state.
- megatron/core/models/common/embeddings/rotary_pos_embedding.py: Main
  adds the register_capture_constant import for its embedding cache
  (NVIDIA#8358) and the PR adds the warn_global_process_group_fallback
  import. The result keeps both, plus the PR's warned cp_group fallback.
- pretrain_gpt.py, pretrain_hybrid.py: Main moves
  is_dataset_built_on_rank and core_gpt_dataset_config_from_args into
  megatron/training/datasets/providers.py (NVIDIA#8341, NVIDIA#8592). The result
  takes main's side and keeps the PR's pp_group in get_batch.
- examples/t5/pretrain_t5.py: The result keeps main's tokenizer
  refactor (NVIDIA#8391) and the PR's pg_collection default passed to
  T5Model. The unused build_tokenizer import that main dropped stays
  dropped.

Adapted to main outside the conflicts:
- megatron/training/datasets/providers.py: The PR's pp_group argument
  to mtp_on_this_rank is re-applied in is_dataset_built_on_rank_explicit,
  where main moved the code.

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#8749, NVIDIA#8735, NVIDIA#8059, NVIDIA#8038, NVIDIA#8724 and NVIDIA#8725 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/core/transformer/moe/batch_invariant.py: Main (NVIDIA#8559)
  splits unpermute into a fused path, a reference _unpermute_loop that
  takes ep_size, and a new public unpermute() that reads the global EP
  world size. The result keeps main's split and re-applies the PR's
  optional ep_size argument and warned resolve_process_groups fallback
  to the new unpermute(), so no new parallel_state read remains. Main's
  unpermute tests in test_moe_batch_invariant_unpermute.py and
  test_moe_kernels.py pass ep_size explicitly instead of setting the
  global EP world-size override.
- megatron/core/transformer/cuda_graphs.py: Main adds
  CudaGraphManager._initialize_mempool and the PR adds
  _resolve_reuse_cudagraphs. The result keeps both, with reuse decided
  lazily from the pp group as the PR does.
- megatron/core/tensor_parallel/random.py: CheckpointFunction.forward
  keeps the PR's explicit tp_group and main's new autocast capture.
- tests/unit_tests/dist_checkpointing/utils.py: Both helpers keep main's
  config.finalize_optimizer_config() call (NVIDIA#8213) and then the PR's
  get_megatron_optimizer call with an explicit pg_collection.
- pretrain_gpt.py, pretrain_hybrid.py: The dataset provider keeps the
  PR's pg_collection parameter, which its body passes to
  is_dataset_built_on_rank.
- megatron/core/distributed/fsdp/mcore_fsdp_adapter.py,
  tests/unit_tests/ssm/test_gated_delta_product.py,
  tests/unit_tests/transformer/test_attention_variant_dsa_gqa.py: Import
  conflicts. The result keeps the imports of both sides.

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- megatron/core/extensions/transformer_engine.py: In
  split_te_layernorm_column_parallel_linear, the TEColumnParallelLinear
  call gets pg_collection=fused_layer._pg_collection from the base PR,
  and main (NVIDIA#8426) appends .to(device=fused_layer.weight.device). The
  result keeps both. The other changes from main merge without
  conflict: TENorm .to(device), the RMSNorm is_rmsnorm_weight markers
  (NVIDIA#8246) and the mark_grouped_tensor helpers (NVIDIA#6828).
- tests/unit_tests/extension/test_te_layernorm_column_parallel_linear.py:
  The imports combine the PR's process-group imports with main's
  parenthesized TENorm import. Main's new test_rmsnorm_gamma_marker
  (NVIDIA#8246) stays where main put it, and the PR's _all_ranks helper and
  TestShardedStateDictDpCpGroup class follow it unchanged.

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>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
Conflicts:
- tests/unit_tests/distributed/mfsdp_v2/test_mcore_moe.py: Main
  (NVIDIA#8312, via NVIDIA#8773) rewrites the file around parametrized standard and
  shortcut MoE training tests, while the base branch adds the
  fully_sharded_optimizer import and
  test_optimizer_names_experts_by_the_model_expert_layout. The result
  keeps main's file and adds those two pieces after the training tests.
- pretrain_gpt.py, pretrain_hybrid.py: The conflicts come only from the
  criss-cross merge bases; the updated base branches leave these files
  unchanged. The result keeps the branch side, with the pg_collection
  parameter on train_valid_test_datasets_provider and the
  is_dataset_built_on_rank predicate bound to that pg_collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY and others added 10 commits November 1, 2026 12:00
Main's NVIDIA#8471 moves FullyShardedDataParallelV1 and FullyShardedDataParallelV2 out of
mcore_fsdp_adapter.py into mcore_adapter_v1.py and mcore_adapter_v2.py, and keeps
mcore_fsdp_adapter.py as the factory that re-exports them. This branch's edits to the
two classes and their helpers are applied unchanged where the code moved, and the
process-group allowlist follows the move.

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>
Main's NVIDIA#8471 moves FullyShardedDataParallelV1 and FullyShardedDataParallelV2 out of
mcore_fsdp_adapter.py into mcore_adapter_v1.py and mcore_adapter_v2.py, and keeps
mcore_fsdp_adapter.py as the factory that re-exports them. This branch's edits to the
two classes and their helpers are applied unchanged where the code moved, and the
process-group allowlist follows the move.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Main's NVIDIA#8471 moves FullyShardedDataParallelV1 and FullyShardedDataParallelV2 out of
mcore_fsdp_adapter.py into mcore_adapter_v1.py and mcore_adapter_v2.py, and keeps
mcore_fsdp_adapter.py as the factory that re-exports them. This branch's edits to the
two classes and their helpers are applied unchanged where the code moved, and the
process-group allowlist follows the move.

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>
Main's NVIDIA#8471 moves FullyShardedDataParallelV1 and FullyShardedDataParallelV2 out of
mcore_fsdp_adapter.py into mcore_adapter_v1.py and mcore_adapter_v2.py, and keeps
mcore_fsdp_adapter.py as the factory that re-exports them. This branch's edits to the
two classes and their helpers are applied unchanged where the code moved, and the
process-group allowlist follows the move.

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>
Main's NVIDIA#8471 moves FullyShardedDataParallelV1 and FullyShardedDataParallelV2 out of
mcore_fsdp_adapter.py into mcore_adapter_v1.py and mcore_adapter_v2.py, and keeps
mcore_fsdp_adapter.py as the factory that re-exports them. This branch's edits to the
two classes and their helpers are applied unchanged where the code moved, and the
process-group allowlist follows the move.

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>
Main's NVIDIA#8471 moves FullyShardedDataParallelV1 and FullyShardedDataParallelV2 out of
mcore_fsdp_adapter.py into mcore_adapter_v1.py and mcore_adapter_v2.py, and keeps
mcore_fsdp_adapter.py as the factory that re-exports them. This branch's edits to the
two classes and their helpers are applied unchanged where the code moved, and the
process-group allowlist follows the move.

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 #8910, which carries this change as commit c3724a3 under the same title, with "Originally #8773." in its message. Closing so review happens in one place. The branch stays on the fork.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant