Skip to content

feat: add MTP (multi-token prediction) training support for nemotron - #2801

Merged
yfw merged 8 commits into
mainfrom
yifu/super_mtp_merge_main
Jun 16, 2026
Merged

feat: add MTP (multi-token prediction) training support for nemotron#2801
yfw merged 8 commits into
mainfrom
yifu/super_mtp_merge_main

Conversation

@yfw

@yfw yfw commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

What does this PR do ?

Adds training-side Megatron Multi-Token Prediction support, rebased onto the Megatron-Bridge 79c8957 bump which includes needed Megatron-LM changes. Part of Nemotron 3 Super upstreaming.

  • megatron_policy_worker: compute mtp_loss_mask from token_mask × sample_mask (with optional mtp_positive_only gating on advantages), set mtp_grad_scale_func for MTP loss scaling, and collect MTP metrics.
  • data.py / train.py: plumb mtp_loss_mask through microbatch processing/packing into model_forward's loss_mask.
  • common.py: get_mtp_metrics (per-layer MTP loss + acceptance rate).
  • grpo.py / lm_policy.py: surface mtp/* metrics in training logs.

Scope: training only. The MTP inference path (vLLM drafter weight load / speculative-config handling) will be handled separately as part of the ultra upstreaming.

Issues

List issues that this PR closes (syntax):

Usage

  • You can potentially add a usage example below
# Add a code snippet demonstrating how to use this

Before your PR is "Ready for review"

Pre checks:

  • Make sure you read and followed Contributor guidelines
  • Did you write any new necessary tests?
  • Did you run the unit tests and functional tests locally? Visit our Testing Guide for how to run tests
  • Did you add or update any necessary documentation? Visit our Document Development Guide for how to write, build and test the docs.

Additional Information

  • ...

@yfw
yfw requested review from a team as code owners June 12, 2026 19:45
@copy-pr-bot

copy-pr-bot Bot commented Jun 12, 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.

@github-actions

Copy link
Copy Markdown

✅ Submodule Fast-Forward Check Results

Check based on commit: 05c85eb (PR #2801 from yifu/super_mtp_merge_main)

✅ Submodules that are properly updated:

Megatron-Bridge: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

@yfw yfw added the CI:L1 Run doctests, unit tests, and functional tests label Jun 12, 2026
@yfw

yfw commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 05c85eb

@yfw
yfw marked this pull request as draft June 12, 2026 19:58
@yfw
yfw marked this pull request as ready for review June 13, 2026 06:06
@yfw

yfw commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 2fc8700

@github-actions

Copy link
Copy Markdown

✅ Submodule Fast-Forward Check Results

Check based on commit: 2fc8700 (PR #2801 from yifu/super_mtp_merge_main)

✅ Submodules that are properly updated:

Megatron-Bridge: ✅ PR branch is ahead of main branch (fast-forward)

All submodule changes look good! ✨

yfw and others added 5 commits June 15, 2026 11:09
Cherry-pick of 8106050 (MTP changes) onto ci/bump-mbridge-79c8957.
Resolved conflicts against the newer base; build-custom-vllm.sh change dropped
per request. Where the commit's context lines were stale (master_config access
style, model_forward cfg= arg, vllm hf_overrides.update, the inline IPC weight
loader), kept the new base's versions and grafted only the genuine MTP additions
(mtp_loss_mask plumbing, mtp_metrics logging, num_speculative_tokens==0 handling,
megatron parallel-state imports).

Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
- test_mtp_metrics.py: get_mtp_metrics per-layer loss/acceptance (1-indexed),
  empty-tracker and defaults-when-only-loss cases. Stubs the cross-rank reduce
  so it runs single-process on CPU.
- test_megatron_data.py: process_microbatch propagates mtp_loss_mask in the
  no-packing path and packs it alongside input_ids in the packing path; absent
  -> None. Tightened the existing with_packing fixture's __contains__ mock
  (was return_value=True for all keys, which now spuriously triggers the
  mtp_loss_mask branch).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
setup._apply_mtp_config currently only wires mtp_num_layers; extend it to also
apply mtp_loss_scaling_factor, mtp_use_repeated_layer and mtp_detach_heads onto
the mcore model config (gated on presence), so these MTP knobs are
user-controllable instead of silently using megatron defaults. Ported from
499108c.

Config conventions:
- Declare the MTP keys (incl. mtp_positive_only) on the MegatronConfig TypedDict
  and add documented defaults to the exemplar examples/configs/grpo_math_1B.yaml
  (+ matching tests/unit/reference_configs/grpo_math_1B.yaml so the v1->v2 check
  stays green).
- Drop the forbidden call-site default in megatron_policy_worker:
  cfg["megatron_cfg"].get("mtp_positive_only", False) -> .get("mtp_positive_only").

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
mtp_positive_only (restrict the MTP loss mask to positive-advantage tokens) was
experimental and is unused. Remove it from the MTP loss-mask computation, the
MegatronConfig schema, and the grpo_math_1B exemplar/reference configs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
Collecting MTP metrics inline in MegatronPolicyWorker.train() made cloudpickle
pull an unpicklable torch ConfigModuleInstance into the actor serialization
(`cannot pickle 'ConfigModuleInstance' object`), so Ray could not create the
megatron policy worker. Move the collection into a small _collect_mtp_metrics
method that lazily imports get_mtp_metrics, and drop the module-level
get_mtp_metrics import, so train()'s (large) globals footprint no longer
references it. Verified the worker class cloudpickles again.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
@yfw
yfw force-pushed the yifu/super_mtp_merge_main branch from 2fc8700 to f22fece Compare June 15, 2026 18:23
@yfw yfw removed the CI:L1 Run doctests, unit tests, and functional tests label Jun 15, 2026

@ashors1 ashors1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the MTP training support — clean implementation with good test coverage!

A few findings below (5 inline comments). The VLM guard and test assertion are the most actionable.

Copyright note (spans multiple files): common.py, data.py, megatron_policy_worker.py, __init__.py, lm_policy.py, and grpo.py still carry # Copyright (c) 2025. Current year is 2026 — train.py and setup.py were already correctly updated in this PR.

Generated by Claude Code

Comment thread nemo_rl/models/megatron/common.py Outdated
Comment thread nemo_rl/models/megatron/data.py
Comment thread nemo_rl/models/megatron/data.py
Comment thread tests/unit/models/megatron/test_megatron_data.py Outdated
Comment thread nemo_rl/models/megatron/common.py
Comment thread nemo_rl/models/policy/workers/megatron_policy_worker.py
yfw and others added 2 commits June 15, 2026 15:59
Co-authored-by: Anna Shors <ashors@nvidia.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
@yfw

yfw commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 0981ab0

Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
@yfw

yfw commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test b86c1a1

@yfw yfw changed the title feat: add MTP (multi-token prediction) training support feat: add MTP (multi-token prediction) training support for nemotron Jun 16, 2026
@yfw
yfw merged commit d584a89 into main Jun 16, 2026
76 of 77 checks passed
@yfw
yfw deleted the yifu/super_mtp_merge_main branch June 16, 2026 21:11
ashors1 added a commit that referenced this pull request Jun 27, 2026
Signed-off-by: Yi-Fu Wu <yifu.wu@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Anna Shors <ashors@nvidia.com>
Signed-off-by: Anna Shors <ashors@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI:Lfast Runs a fast test suite and re-use nightly `main` container (but sync dependencies to PRs version)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants