Skip to content

Validate the inference process groups before collectives use them - #8736

Closed
Connor-XY wants to merge 70 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-g11-dynamic-engine-barrier
Closed

Connor-XY wants to merge 70 commits into
NVIDIA:mainfrom
Connor-XY:yx/pstate-g11-dynamic-engine-barrier

Conversation

@Connor-XY

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

Copy link
Copy Markdown
Contributor

Depends on #8054 and #8058 (which contains #8049 and #6293). Please review the diff from the merge of their heads: scoped diff. The merge commit resolves the conflicts between #8058 and #8054 in text_generation_controller.py and the allowlist; this PR's own commit touches neither file.

Why. InferenceConfig.resolve_pg_collection is where inference picks its process groups. It checked the collections it took from the model or built from parallel_state, but returned a configured collection as given, and that is what to_inference_config and the inference shards pass. An unset field reads as None, which torch.distributed treats as WORLD. For a collection without mp, DynamicInferenceEngine.start_listening_to_data_parallel_coordinator broadcast global rank 0's request address 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 global rank 0's publisher.

A None dp turns the coordinator address broadcast into a world broadcast and lets every replica start its own coordinator. 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, which the warned fallback never built.

What.

  • resolve_pg_collection checks whichever collection it returns. The collection must set tp, pp, dp, mp, ep, cp, expt_tp and expt_dp, and tp, pp, dp and mp must be process groups, not None. Otherwise it raises ValueError before any component uses the collection, and stores nothing.
  • ep, cp, expt_tp and expt_dp may still be None for an axis that is off.
  • A model's collection is used only when it sets all eight groups and tp_ep, and the fallback builds all eight and tp_ep, as Resolve inference process groups at one warned boundary #8058 requires. A configured collection may omit tp_ep.
  • Use and validate the inference process groups instead of the global grid #8054's tests for unset groups built collections with 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 in a full collection and expects the config's message. The two controller tests for an unset dp are dropped, because the config rejects such a collection before a controller exists and the dp cases of test_rejects_unset_group cover it.

Tests. test_inference_process_group_fallbacks.py adds cases for:

  • each communication group absent or None, and absent axis groups;
  • non-member groups;
  • model collections: complete, partial, without tp_ep, and with mp=None;
  • the eight-group fallback;
  • a dynamic context that rejects a collection without mp while global reads raise.

On 4 GPUs against its base, with no regressions: models/test_t5_model.py: 26 passed here vs 26 on the base; test_process_group_fallback.py: 19 vs 19; training/test_dataset_providers.py: 103 vs 103; transformer/test_rope.py: 8 vs 8. TestT5Model::test_constructor fails on both sides because it needs 8 ranks; the integration run, which merges all #6307 PRs onto main 77391e7, passes it on 8 GPUs (two nodes). These runs did not include this PR's two inference test files, and the new tests were not run without this change. In the integration run, this PR's test files show no regressions against main: test_inference_process_group_fallbacks.py passes 27 of 27 and test_inference_process_groups.py passes 12 of 12 on 4 GPUs. Neither file exists on main. Before the sync, the branch ran on 4 CPU gloo ranks: test_inference_process_groups.py passed 12 of 12, including the 4 new test_rejects_unset_group cases. test_inference_process_group_fallbacks.py needs flash_attn and does not import on CPU.

Compatibility.

  • Collections from use_mpu_process_groups(), build_inference_pg_collection and Megatron-Bridge set all eight groups, so the standard grid is unchanged.
  • A hand-built configured collection that is partial, or that has tp, pp, dp or mp set to None, now raises instead of silently using world groups. This applies on every inference path, including static and direct mode, which never run the coordinator collectives. NeMo-RL builds InferenceConfig without a collection, so the new raise for configured collections does not reach it.
  • A model whose own collection sets only some of the eight groups now takes the warned global fallback instead of its own groups. No in-repo producer builds such a collection. Should that case raise instead?
  • Use and validate the inference process groups instead of the global grid #8054 allows dp=None when the sampling-seed offset is off. This PR requires dp on every path, so Use and validate the inference process groups instead of the global grid #8054's wrapper tp/pp check and its controller dp=None check can no longer be reached through the config. They are left in place.
  • Not done, by design: deriving mp from tp x pp (the config raises instead), and the engine and coordinator communicators that deliberately span the world.
  • Resolve the remaining inference process-group fallbacks with warnings #8751 updates a stub collection in its inference tests to pass this check.
  • Needs determinism-exempt while Remove global process-group reads from Megatron Core #6293 is unmerged, because the kernel coverage check diffs against main. This PR's own diff touches no kernel file.

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

Part of #6307.

🤖 Generated with Claude Code

Connor-XY and others added 30 commits October 8, 2026 10:27
Signed-off-by: Yan Xu <yxu1@nvidia.com>
A collection without an embd field, or tied pipeline embedding/output stages
without a usable embedding group, is a caller error. Report it with
ValueError instead of an assert, which is skipped under python -O.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

hybrid_cp_schedule.py no longer imports parallel_state at all.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Third attempt. The previous two failed because my inventory was incomplete: the
first scan skipped positional call sites, the second scanned MLASelfAttention
and AbsorbedMLASelfAttention by name and missed FusedMLASelfAttention, which
also reaches MultiLatentAttention.__init__ (110 assertion failures, all in
TestFusedMLA*).

This time the subclass list was derived programmatically from the class
hierarchy: MLASelfAttention (18 sites), FusedMLASelfAttention (6 sites),
AbsorbedMLASelfAttention (0). All now pass a collection.

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

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The only construction path is GroupedMLP.__init__ (experts.py), which
dereferences pg_collection.ep and .expt_tp unconditionally before passing it on
-- so it cannot be None there. No direct constructors exist anywhere in the
repo.

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

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
dynamic_engine and abstract_model_inference_wrapper each derived a collection
from global state when the inference config carried none. Both now call
InferenceConfig.resolve_pg_collection(), so the fallback exists once at the
config boundary and consumers can assume a collection.

Resolution is lazy rather than in __post_init__, since configs are commonly
built before initialize_model_parallel.

Also drops a stale comment in attention.py referring to a fallback that no
longer exists.

Global process-group reads in megatron/core: 110 -> 109 (shims 10 -> 9).

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

Two independent reductions of the same shape as the MoE one: many global reads
become one, with no signature or caller changes.

distributed_trtllm_model_weights_converter: five parallel_state reads (pp size,
tp size, tp rank, pp rank, tp group) become one use_mpu_process_groups call with
sizes and ranks derived from it. The file no longer imports parallel_state.

inference/communication_utils: broadcast_tensor fetched the model-parallel
source rank and the group separately; the source rank is rank 0 of that group,
so one lookup gives both. broadcast_list called get_model_parallel_src_rank
twice for the same value.

Global process-group reads in megatron/core: 102 -> 96 (accessors 92 -> 85).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Rebasing onto current main surfaced three FusedMLASelfAttention constructors
added since this work began. They reach MultiLatentAttention.__init__, which
this branch makes require an explicit collection.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Rebasing onto main (c9b53d0) brought in nine constructor calls written
against the old fallbacks, which this series has removed:

- YarnRotaryEmbedding x2 in test_mla_yarn_rope_apply.py: cp_group
- SelfAttention and RotaryEmbedding in test_attention_packed_seq.py:
  pg_collection / cp_group (the build-time CP group the test already reads)
- MLASelfAttention in test_multi_latent_attention.py: pg_collection
- MultiTokenPredictionBlock x4 in test_multi_token_prediction.py: pg_collection
  with cp/tp/pp, since the block now dereferences pg_collection.pp as well

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Two call sites the series left behind:

- pretrain_vlm.py built LLaVAModel without a collection although
  model_provider already accepts pg_collection. Resolve the global-grid
  fallback at the script boundary and forward it, as
  examples/multimodal/model.py does.
- The hybrid CP scheduler tests monkeypatched
  hybrid_cp_schedule.parallel_state, which the scheduler no longer imports.
  Hand them a fake pg_collection with tp and dp_cp groups instead and pass
  it through; the TP broadcast source now comes from
  torch.distributed.get_global_rank, so map that to the group rank too.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
TEGroupedLinear now requires an explicit process-group collection. The
mixed-split determinism test builds one directly; pass the global collection
there as the other constructions in this file do.

Signed-off-by: Yan Xu <yxu1@nvidia.com>
Connor-XY and others added 3 commits October 29, 2026 09:37
…anch

NVIDIA#6293 now dates the global process-group fallbacks from Megatron Core 0.21,
the first release that emits them, with removal in 0.23. Take its
defaults and keep this branch's note that a fallback which starts to warn
in a later release passes its own versions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#6293 now dates the fallback defaults from 0.21, the first release
that emits them, with removal in 0.23. The docstring of
warn_global_process_group_fallback still said that a fallback which
starts to warn after 0.20 passes its own versions; that cohort now
starts after 0.21.

Two tests checked that explicit versions reach the message by passing
0.21 and 0.23. Those now equal the defaults, so the tests would pass
even if the arguments were ignored. Pass 0.22 and 0.24 instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
NVIDIA#8049 now dates the global process-group fallbacks from Megatron Core
0.21, with removal in 0.23, and aligns its docstring and explicit-version
tests with those dates. Merge its current head so this branch builds on
it.

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

copy-pr-bot Bot commented Oct 30, 2026

Copy link
Copy Markdown

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

Contributors can view more details about this message here.

Connor-XY and others added 3 commits October 30, 2026 13:12
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.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The validation that follows edits the resolve_pg_collection body and
the _INFERENCE_PROCESS_GROUPS tuple that NVIDIA#8058 adds, and the
process-group helpers of NVIDIA#8049 that NVIDIA#8058 contains.

Two conflicts with NVIDIA#8054, resolved as follows:
- text_generation_controller.py: the 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).
- tools/process_group_usage_allowlist.json: NVIDIA#8058's side, since
  NVIDIA#8054's fallback read in the controller is gone. The checker accepts
  it as is.

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>
@Connor-XY
Connor-XY force-pushed the yx/pstate-g11-dynamic-engine-barrier branch from c23bae9 to ff0154b Compare November 1, 2026 01:37
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
The checker's function counting that follows updates a comment in
NVIDIA#8058's test_inference_process_group_fallbacks.py that names the
forbidden parallel_state functions.

Two conflicts with NVIDIA#8054, resolved as in NVIDIA#8736's base merge of the same
two PRs; both files equal that merge's:
- text_generation_controller.py: the 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).
- tools/process_group_usage_allowlist.json: NVIDIA#8058's side, since
  NVIDIA#8054's fallback read in the controller is gone. Regenerating it on
  the merged tree gives the same file.

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

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>
Connor-XY added a commit to Connor-XY/Megatron-LM that referenced this pull request Nov 1, 2026
…8739

This base holds every other PR of the parallel_state deprecation series
(NVIDIA#6307): the end-to-end test that follows asserts that no process-group
fallback or deprecation warning fires anywhere on a collection that is
not the global grid, and the strict-fallback functional tests turn
every remaining fallback into an error, so both need the whole series
below them. NVIDIA#8739 brings NVIDIA#8028 and NVIDIA#8035.

NVIDIA#8035, which NVIDIA#8739 brings, meets NVIDIA#6293, which NVIDIA#8736 brings through
NVIDIA#8058, 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>
Connor-XY and others added 6 commits November 1, 2026 09:00
Conflicts:
- examples/t5/pretrain_t5.py: Main adds `cfg = get_run_config()` to
  model_provider and drops the `build_tokenizer` import in favour of
  `get_tokenizer()`. The PR moves that import for isort and passes a
  ProcessGroupCollection into T5Model. The result keeps main's `cfg`
  line and the PR's pg_collection fallback and argument. It drops the
  `build_tokenizer` import, which would otherwise be unused.
- megatron/core/models/common/embeddings/rotary_pos_embedding.py: Main
  imports `register_capture_constant` for its shared rotary cache. The
  PR imports `warn_global_process_group_fallback`. Both imports are
  kept, along with main's cache refactor and the PR's warning
  cp_group fallback.
- pretrain_gpt.py, pretrain_hybrid.py: Main (NVIDIA#8341, NVIDIA#8592) moves
  `is_dataset_built_on_rank` into megatron/training/datasets/providers.py
  and the dataset config builder into argument_utils. The PR adds
  `pp_group` to the `mtp_on_this_rank` call in the local predicate. The
  result takes main's layout and keeps the PR's `pp_group` in get_batch.

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

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The process-group allowlist records the reads main added since this PR
was written.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
The process-group allowlist records the reads main added since this PR
was written.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Adapted to main outside the conflicts:
- megatron/core/inference/config.py: resolve_pg_collection now also
  requires tp_ep, both to use the model's collection and in the
  deprecated global fallback. Main's NVIDIA#4743 reads tp_ep in the dynamic
  context and engine. Without this, the fallback left tp_ep unset, so
  EP>1 or ETP>1 runs read it as None and sized it to 1. A collection
  set on InferenceConfig.pg_collection is still used as-is. A model
  collection without tp_ep now takes the warned global fallback.
- tests/unit_tests/inference/test_inference_process_group_fallbacks.py:
  the tests check that the fallback builds tp_ep from the global group
  and that a model collection lacking only tp_ep falls back.

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Yan Xu <yxu1@nvidia.com>
Conflicts:
- examples/t5/pretrain_t5.py: The PR falls back to
  ProcessGroupCollection.use_mpu_process_groups() when pg_collection is
  None and passes pg_collection to T5Model. Main (NVIDIA#8391) reads the
  vocabulary size from get_run_config() and uses get_tokenizer(). The
  result keeps both and drops the now unused build_tokenizer import.
- megatron/core/models/common/embeddings/rotary_pos_embedding.py: Both
  sides added an import. The result keeps the PR's
  warn_global_process_group_fallback and main's register_capture_constant.
- pretrain_gpt.py, pretrain_hybrid.py: Main (NVIDIA#8341, NVIDIA#8592) moves the
  dataset rank rule and config builder into
  megatron/training/datasets/providers.py. The result takes main's side
  and keeps the PR's pp_group edit in get_batch.

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 rank rule.
- megatron/core/inference/config.py: resolve_pg_collection requires tp_ep
  only for the model collection check and the parallel_state fallback,
  as NVIDIA#8058 does on main. A configured collection is still checked
  against the groups inference reads and may omit tp_ep. The tests in
  test_inference_process_group_fallbacks.py cover both paths.

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

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

Copy link
Copy Markdown
Contributor Author

Superseded by #8901, which carries this change as commit 059fac6 under the same title, with "Originally #8736." 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