Skip to content

Pass the process-group collection to MTP's input projections - #7403

Merged
asolergi-nv merged 1 commit into
NVIDIA:mainfrom
deepakn94:dnarayanan/mtp_eh_proj_pg_collection
Sep 17, 2026
Merged

asolergi-nv merged 1 commit into
NVIDIA:mainfrom
deepakn94:dnarayanan/mtp_eh_proj_pg_collection

Conversation

@deepakn94

Copy link
Copy Markdown
Contributor
  • I, the PR author, have personally reviewed every line of this PR.

What does this PR do?

MTP's input projections are the only weight-bearing modules in that path built without a pg_collection:

# multi_token_prediction.py, MHC path
"tp_group": pg_collection.tp if pg_collection is not None else None,
# and the eh_proj path
tp_group=pg_collection.tp if pg_collection is not None else None,

Everything else in the path passes the collection through: mtp_model_layer, and the block's layers. enorm, hnorm and final_layernorm take no group arguments at all, but they are 1-D norm weights and are not GTP-sharded anywhere.

ColumnParallelLinear and the TE equivalents read their GTP axis with resolve_gtp_remat_group(pg_collection, is_expert). With pg_collection=None that skips the attr in vars(...) branch and falls back to ProcessGroupCollection.use_mpu_process_groups(...):

  • non-MIMO: the MPU globals are initialized, the fallback returns the real gtp_remat group, and the projection is sharded. The missing argument is invisible.
  • MIMO: examples/mimo/pretrain_mimo.py passes skip_model_parallel_init=True, so the accessors run with check_initialized=False and return None. The projection is then silently built unsharded.

Same silent-None fallback as #7356, reached by a different route: there the top-level wrapper failed the vars() check, here the argument is simply never passed.

Impact

eh_proj is [2h, h]. At hidden_size=8192 that is ~134M parameters held as a full replica on every GTP peer rather than one shard, so at gtp_weight_remat_size=64 the layer costs 64 copies instead of one. e_proj and h_proj are [h, h] each and have the same problem on the MHC path.

Gradient accounting stays correct: the parameters are not tagged is_gtp_weight_remat, so param_is_not_gtp_duplicate counts them on GTP rank 0 only, which is the right answer for genuinely replicated tensors. The other visible consequence is that these projections' checkpoint shapes differ between a MIMO and a non-MIMO run at the same GTP size.

Fix

Pass pg_collection on both paths. All three implementations already accept it: ColumnParallelLinear, TEColumnParallelLinear and InferenceColumnParallelLinear.

The eh_proj line dates to 9a60a18ef7 ("ultra refit", 2026-03-19), before GTP under MIMO existed, and the MHC path inherited the same shape, so this looks like an oversight rather than a deliberate exclusion.

Tests

test_mtp_eh_proj_uses_custom_gtp_group in tests/unit_tests/generalized_tensor_parallel/test_gtp_custom_pgs.py, modelled on the neighbouring test_gdp_projections_use_custom_gtp_group. The MPU keeps one GTP pairing while the layer is handed the other, so reading the globals yields a valid-but-wrong group and the assertion on weight.group fails. The existing test_gtp_mtp.py cases cannot catch this, since they run with MPU globals initialized to the same topology.

The test covers the default eh_proj path only; mhc_enabled is off by default and its projections take the identical fix.

Pre-checks

  • I have added relevant unit tests
  • I have added relevant functional tests
  • I have added proper typing to my code
  • I have added relevant documentation
  • I have run the autoformatter on my PR

black and isort clean. The test was not run locally; it needs 4 GPUs.

🤖 Generated with Claude Code

eh_proj, and e_proj/h_proj on the MHC path, were built with tp_group alone, so
resolve_gtp_remat_group fell through to the MPU globals for their GTP axis.
Non-MIMO runs got the right group by accident; a MIMO run never creates those
globals, so the projections were silently built unsharded and replicated across
every GTP peer. Pass pg_collection, which all three implementations already
accept.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 16, 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.

@deepakn94
deepakn94 marked this pull request as ready for review September 16, 2026 15:58
@deepakn94
deepakn94 requested review from a team as code owners September 16, 2026 15:58
@svcnvidia-nemo-ci svcnvidia-nemo-ci added the Approved All necessary approvals have been made label Sep 16, 2026
@asolergi-nv
asolergi-nv added this pull request to the merge queue Sep 16, 2026
@nemo-automation-bot

Copy link
Copy Markdown

🔄 Merge queue validation started!

You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/35176313472

@fanshiqing fanshiqing linked an issue Sep 17, 2026 that may be closed by this pull request
Merged via the queue into NVIDIA:main with commit fb425d6 Sep 17, 2026
117 of 119 checks passed
@deepakn94
deepakn94 deleted the dnarayanan/mtp_eh_proj_pg_collection branch September 19, 2026 17:44
xuwchen added a commit to xuwchen/Megatron-LM that referenced this pull request Sep 22, 2026
…7403)

Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit fb425d6)

Ported by hand: this tree builds e_proj / h_proj / eh_proj directly rather than
through a projection_kwargs dict, so pg_collection= is added at each of the three
build sites; the test file is taken verbatim.
xuwchen pushed a commit to xuwchen/Megatron-LM that referenced this pull request Sep 22, 2026
…7403)

Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit fb425d6)

Ported by hand: this tree builds e_proj / h_proj / eh_proj directly rather than
through a projection_kwargs dict, so pg_collection= is added at each of the three
build sites; the test file is taken verbatim.
xuwchen pushed a commit to xuwchen/Megatron-LM that referenced this pull request Sep 22, 2026
…7403)

Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit fb425d6)

Ported by hand: this tree builds e_proj / h_proj / eh_proj directly rather than
through a projection_kwargs dict, so pg_collection= is added at each of the three
build sites; the test file is taken verbatim.
xuwchen pushed a commit to xuwchen/Megatron-LM that referenced this pull request Oct 10, 2026
…7403)

Signed-off-by: Deepak Narayanan <dnarayanan@nvidia.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit fb425d6)

Ported by hand: this tree builds e_proj / h_proj / eh_proj directly rather than
through a projection_kwargs dict, so pg_collection= is added at each of the three
build sites; the test file is taken verbatim.

Signed-off-by: xuwenc <xuwenc@nvidia.com>

This branch was previously deployed

2 inactive deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved All necessary approvals have been made complexity: low nemotron

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] GTP+MTP

6 participants