[doc, worker] feat: Enable Megatron-Bridge for MTP#5323
[doc, worker] feat: Enable Megatron-Bridge for MTP#5323HollowMan6 wants to merge 2 commits intoverl-project:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request enables Multi-Token Prediction (MTP) support for Megatron-Bridge by removing an assertion that previously restricted MTP to vanilla_mbridge. The documentation is also updated to reflect this new capability and provide guidance on dependencies.
My main feedback is to consider adding a more specific check to ensure that MTP with Megatron-Bridge is only used with supported models, as the current implementation might be too permissive and could lead to runtime errors for users. This would improve the robustness of the new feature.
There was a problem hiding this comment.
Pull request overview
This PR enables Megatron-Bridge support for Multi-Token Prediction (MTP) training by removing an overly restrictive assertion that previously required vanilla_mbridge=True. According to the linked PR description, Megatron-Bridge now supports MTP models, so the restriction is no longer necessary.
Changes:
- Removed the
vanilla_mbridgeassertion for MTP inverl/workers/megatron_workers.py - Updated documentation to reflect support for both mbridge and Megatron-Bridge with MTP
- Added Megatron-Bridge PR reference to the documentation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| verl/workers/megatron_workers.py | Removed assertion requiring vanilla_mbridge=True for MTP, enabling Megatron-Bridge support |
| docs/advance/mtp.md | Updated training engine documentation to include Megatron-Bridge, added PR reference, and updated last modified date |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
98e3def to
370e0f4
Compare
Signed-off-by: Hollow Man <hollowman@opensuse.org>
370e0f4 to
a2a67ff
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Hollow Man <hollowman@opensuse.org>
a2a67ff to
84652ea
Compare
What does this PR do?
There's nothing specific in Megatron-Bridge that stops MTP support.
NVIDIA-NeMo/Megatron-Bridge#2387 adds MiMo dense MTP models bridge support so that
examples/mtp_trainer/test_dapo_mimo_7b_with_mtp_math_megatron.shcan also be used together with Megatron-Bridge (settingvanilla_mbridgeto beFalse).Checklist Before Starting
[{modules}] {type}: {description}(This will be checked by the CI){modules}includefsdp,megatron,veomni,sglang,vllm,rollout,trainer,ci,training_utils,recipe,hardware,deployment,ray,worker,single_controller,misc,perf,model,algo,env,tool,ckpt,doc,data,cfg,reward,like[megatron, fsdp, doc]{type}is infeat,fix,refactor,chore,test[BREAKING]to the beginning of the title.[BREAKING][fsdp, megatron] feat: dynamic batchingTest
API and Usage Example
# Add code snippet or script demonstrating how to use thisDesign & Code Changes
Checklist Before Submitting
Important
Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review.
pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=alwaysci-requestchannel in theverlSlack workspace. (If not accessible, please try the Feishu group (飞书群).)recipesubmodule, please also update the reference to the submodule commit viagit submodule update --remoteorcd recipe && git pull origin main.