[MTP][BugFix] Preserve MoE weight loaders during online updates - #13337
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug where MoE weight metadata was inadvertently dropped during online weight updates on Ascend NPUs. By switching to a parameter replacement utility that respects existing metadata, the system now correctly maintains expert-aware loaders. Additionally, the wake-up logic has been extended to ensure that MTP drafter models receive the same necessary layout preparations as the primary target model, ensuring consistency during online RL weight updates. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. Tip 💡 Consider Linking a Related Issue or RFCYour PR title contains the [BugFix] tag, indicating a bug fix or new feature. Linking a related issue or RFC in the PR description is strongly encouraged — it gives reviewers helpful context and speeds up the review. You can use any of these keywords:
🙏 Thanks for helping us keep the project well-organized! |
There was a problem hiding this comment.
Code Review
This pull request ensures that weight_loader metadata is preserved during online weight updates for unquantized Ascend MoE weights by utilizing vLLM's replace_parameter helper. It also updates NPUWorker.wake_up() to prepare both the target model and the MTP drafter model when speculative decoding is enabled with the 'mtp' method. The review feedback correctly identifies a style guide violation regarding the PR title format and provides a compliant title and summary suggestion.
| speculative_config = self.vllm_config.speculative_config | ||
| if speculative_config is not None and speculative_config.method == "mtp": |
There was a problem hiding this comment.
The Pull Request Title and Summary do not fully adhere to the Repository Style Guide. Specifically, the title format should be [Branch][Module][Action] Pull Request Title. The current title uses [BugFix][MTP] which swaps the Action and Module.\n\nHere are the suggested PR Title and Summary according to the style guide:\n\nSuggested PR Title:\n\nmarkdown\n[MTP][BugFix] Preserve MoE weight loaders during online updates\n\n\nSuggested PR Summary:\n\nmarkdown\n### What this PR does / why we need it?\n\nThis PR prevents existing `weight_loader` metadata from being dropped when unquantized Ascend MoE weights change layout during online RL weight updates with MTP.\n\nThe target model and the MTP drafter receive actor weights through `model.load_weights()`. After loading, Ascend converts `w13_weight` and `w2_weight` from the loadable layout to the execution layout. The previous implementation registered raw `torch.nn.Parameter` objects during this conversion, which dropped the existing expert-aware `weight_loader`.\n\nIn addition, `NPUWorker.wake_up()` restored the loadable layout only for the target model, while the MTP drafter is a separate model that receives the same online updates.\n\nThis change:\n- Uses vLLM's public `replace_parameter()` helper when converting unquantized MoE weights in both layout directions, preserving an existing `weight_loader`.\n- Prepares both the target model and the MTP drafter during `wake_up()` when `speculative_config.method == "mtp"`.\n- Keeps non-MTP drafter behavior unchanged.\n- Adds unit coverage for loader preservation, target/drafter wake-up handling, and non-MTP isolation.\n\n### Does this PR introduce _any_ user-facing change?\n\nYes. In MTP mode, `NPUWorker.wake_up()` now applies the existing unquantized MoE layout preparation to both the target and the MTP drafter.\n\nThere is no API or configuration change, and non-MTP speculative methods keep their existing behavior.\n\n### How was this patch tested?\n\nUnit coverage is included for:\n- Preserving `w13_weight` and `w2_weight` layout, contiguity, and `weight_loader` after `process_weights_after_loading()`.\n- Preparing both target and MTP drafter weights in `NPUWorker.wake_up()`.\n- Confirming that non-MTP drafters are not included in the new behavior.\n\nManual integration validation was performed with:\n- Ascend NPU;\n- Qwen3.5-35B-A3B;\n- MTP speculative decoding enabled;\n- repeated online actor-to-rollout weight updates;\n- graph rollout modes.\n
References
- The Pull Request Title and Summary must follow the specified format in the Repository Style Guide. The current title '[BugFix][MTP]' has the Action and Module swapped (should be '[MTP][BugFix]'). (link)
Preserve expert-aware weight_loader metadata while converting unquantized Ascend MoE weights between load and execution layouts. Prepare both the target model and the MTP drafter before online weight updates, while leaving non-MTP speculative methods unchanged. Signed-off-by: ZhangMinjie <781650427@qq.com>
dd59be2 to
8ad68e0
Compare
…into vllm-new # By shenhui-cli (8) and others # Via GitHub (1) and shenhui-cli (1) * 'vllm-new' of https://github.com/shenhui-cli/vllm-ascend: (34 commits) When the PR modifies any files in the CSRC folder, skip the test case filtering logic and execute all test cases by default. [MRV2][Bugfix] Fix two triton ops errors in model_runner_v2 num_nans_kernel and apply_panalties (vllm-project#13159) [Cherry-pick][main][Doc][BugFix] Update proxy script name in DeepSeek-V3.2 tutorial (from vllm-project#13537) (vllm-project#13575) [Refactor][quantization]remove mxfp_compat compatibility shim and inline dtype references (vllm-project#13447) [MTP][BugFix] Preserve MoE weight loaders during online updates (vllm-project#13337) [Feature][xlite] Support MLA and DSA (`DeepseekV3ForCausalLM`, `DeepseekV32ForCausalLM` and `GlmMoeDsaForCausalLM`) in the xlite adapter (vllm-project#13378) [BugFix][xlite] Fix DP metadata handling in XliteWrapper and pass down the max number of tokens across all DP ranks for collective communications. (vllm-project#11213) [Refactor][Ops] Move expert routing into router classes (vllm-project#13417) [Platform][Refactor] Refactor NPUPlatform for better organization and clarity (vllm-project#13484) [BugFix] fix fiaV2 contiguous err in GQA (vllm-project#13456) [feature][KV Offload] Support Sparse KV Cache Offload (vllm-project#13026) [Bugfix][MRV2]Skip D2H copy and synchronize when spec decode is not active (vllm-project#13382) [Bugfix] Change AscendSFAIndexerCacheSpec to inherit MLAAttentionSpec (vllm-project#12849) [BugFix][FusedMoE] Restore BF16 quant method initialization (vllm-project#13412) [Doc] Fix link errors and add section anchors (vllm-project#13485) [TEST]Revise the A3 case (vllm-project#13495) [CI] modify default cann_version and add build_type support in nightly (vllm-project#13480) [CI]Improve logging, optimize function-level recommendation algorithm, and add early exit for no product code changes (vllm-project#13472) This PR restricts the test case discovery scope to the following directories: tests/e2e/pull_request/ tests/ut/ This PR restricts the test case discovery scope to the following directories: tests/e2e/pull_request/ tests/ut/ ... # Conflicts: # .github/workflows/scripts/test_selector.py
…-project#13337) ### What this PR does / why we need it? This PR prevents existing `weight_loader` metadata from being dropped when unquantized Ascend MoE weights change layout during online RL weight updates with MTP. The target model and the MTP drafter receive actor weights through `model.load_weights()`. After loading, Ascend converts `w13_weight` and `w2_weight` from the loadable layout to the execution layout. The previous implementation registered raw `torch.nn.Parameter` objects during this conversion, which dropped the existing expert-aware `weight_loader`. In addition, `NPUWorker.wake_up()` restored the loadable layout only for the target model, while the MTP drafter is a separate model that receives the same online updates. This change: - uses vLLM's public `replace_parameter()` helper when converting unquantized MoE weights in both layout directions, preserving an existing `weight_loader`; - prepares both the target model and the MTP drafter during `wake_up()` when `speculative_config.method == "mtp"`; - keeps non-MTP drafter behavior unchanged; - adds unit coverage for loader preservation, target/drafter wake-up handling, and non-MTP isolation. The change does not add model code, environment variables, global state, or a framework-side patch. ### Does this PR introduce any user-facing change? Yes. In MTP mode, `NPUWorker.wake_up()` now applies the existing unquantized MoE layout preparation to both the target and the MTP drafter. There is no API or configuration change, and non-MTP speculative methods keep their existing behavior. ### How was this patch tested? Unit coverage is included for: - preserving `w13_weight` and `w2_weight` layout, contiguity, and `weight_loader` after `process_weights_after_loading()`; - preparing both target and MTP drafter weights in `NPUWorker.wake_up()`; - confirming that non-MTP drafters are not included in the new behavior. Manual integration validation was performed with: - Ascend NPU; - Qwen3.5-35B-A3B; - MTP speculative decoding enabled; - repeated online actor-to-rollout weight updates; - graph rollout modes. Both modes completed multiple training iterations without `weight_loader` errors. Draft acceptance and reward remained normal. <img width="871" height="497" alt="image" src="https://github.com/user-attachments/assets/581cb666-8b3c-47fd-a028-b75f897517fb" /> Additional checks: ```bash git diff --check python -m py_compile \ tests/ut/ops/test_fused_moe.py \ tests/ut/worker/a2/test_worker_v1.py \ vllm_ascend/ops/fused_moe/routed_experts.py \ vllm_ascend/worker/worker.py - vLLM version: v0.26.0 - vLLM main: vllm-project/vllm@d02df74 Signed-off-by: ZhangMinjie <781650427@qq.com>
…-project#13337) ### What this PR does / why we need it? This PR prevents existing `weight_loader` metadata from being dropped when unquantized Ascend MoE weights change layout during online RL weight updates with MTP. The target model and the MTP drafter receive actor weights through `model.load_weights()`. After loading, Ascend converts `w13_weight` and `w2_weight` from the loadable layout to the execution layout. The previous implementation registered raw `torch.nn.Parameter` objects during this conversion, which dropped the existing expert-aware `weight_loader`. In addition, `NPUWorker.wake_up()` restored the loadable layout only for the target model, while the MTP drafter is a separate model that receives the same online updates. This change: - uses vLLM's public `replace_parameter()` helper when converting unquantized MoE weights in both layout directions, preserving an existing `weight_loader`; - prepares both the target model and the MTP drafter during `wake_up()` when `speculative_config.method == "mtp"`; - keeps non-MTP drafter behavior unchanged; - adds unit coverage for loader preservation, target/drafter wake-up handling, and non-MTP isolation. The change does not add model code, environment variables, global state, or a framework-side patch. ### Does this PR introduce any user-facing change? Yes. In MTP mode, `NPUWorker.wake_up()` now applies the existing unquantized MoE layout preparation to both the target and the MTP drafter. There is no API or configuration change, and non-MTP speculative methods keep their existing behavior. ### How was this patch tested? Unit coverage is included for: - preserving `w13_weight` and `w2_weight` layout, contiguity, and `weight_loader` after `process_weights_after_loading()`; - preparing both target and MTP drafter weights in `NPUWorker.wake_up()`; - confirming that non-MTP drafters are not included in the new behavior. Manual integration validation was performed with: - Ascend NPU; - Qwen3.5-35B-A3B; - MTP speculative decoding enabled; - repeated online actor-to-rollout weight updates; - graph rollout modes. Both modes completed multiple training iterations without `weight_loader` errors. Draft acceptance and reward remained normal. <img width="871" height="497" alt="image" src="https://github.com/user-attachments/assets/581cb666-8b3c-47fd-a028-b75f897517fb" /> Additional checks: ```bash git diff --check python -m py_compile \ tests/ut/ops/test_fused_moe.py \ tests/ut/worker/a2/test_worker_v1.py \ vllm_ascend/ops/fused_moe/routed_experts.py \ vllm_ascend/worker/worker.py - vLLM version: v0.26.0 - vLLM main: vllm-project/vllm@d02df74 Signed-off-by: ZhangMinjie <781650427@qq.com>
…-project#13337) ### What this PR does / why we need it? This PR prevents existing `weight_loader` metadata from being dropped when unquantized Ascend MoE weights change layout during online RL weight updates with MTP. The target model and the MTP drafter receive actor weights through `model.load_weights()`. After loading, Ascend converts `w13_weight` and `w2_weight` from the loadable layout to the execution layout. The previous implementation registered raw `torch.nn.Parameter` objects during this conversion, which dropped the existing expert-aware `weight_loader`. In addition, `NPUWorker.wake_up()` restored the loadable layout only for the target model, while the MTP drafter is a separate model that receives the same online updates. This change: - uses vLLM's public `replace_parameter()` helper when converting unquantized MoE weights in both layout directions, preserving an existing `weight_loader`; - prepares both the target model and the MTP drafter during `wake_up()` when `speculative_config.method == "mtp"`; - keeps non-MTP drafter behavior unchanged; - adds unit coverage for loader preservation, target/drafter wake-up handling, and non-MTP isolation. The change does not add model code, environment variables, global state, or a framework-side patch. ### Does this PR introduce any user-facing change? Yes. In MTP mode, `NPUWorker.wake_up()` now applies the existing unquantized MoE layout preparation to both the target and the MTP drafter. There is no API or configuration change, and non-MTP speculative methods keep their existing behavior. ### How was this patch tested? Unit coverage is included for: - preserving `w13_weight` and `w2_weight` layout, contiguity, and `weight_loader` after `process_weights_after_loading()`; - preparing both target and MTP drafter weights in `NPUWorker.wake_up()`; - confirming that non-MTP drafters are not included in the new behavior. Manual integration validation was performed with: - Ascend NPU; - Qwen3.5-35B-A3B; - MTP speculative decoding enabled; - repeated online actor-to-rollout weight updates; - graph rollout modes. Both modes completed multiple training iterations without `weight_loader` errors. Draft acceptance and reward remained normal. <img width="871" height="497" alt="image" src="https://github.com/user-attachments/assets/581cb666-8b3c-47fd-a028-b75f897517fb" /> Additional checks: ```bash git diff --check python -m py_compile \ tests/ut/ops/test_fused_moe.py \ tests/ut/worker/a2/test_worker_v1.py \ vllm_ascend/ops/fused_moe/routed_experts.py \ vllm_ascend/worker/worker.py - vLLM version: v0.26.0 - vLLM main: vllm-project/vllm@d02df74 Signed-off-by: ZhangMinjie <781650427@qq.com>
What this PR does / why we need it?
This PR prevents existing
weight_loadermetadata from being dropped when unquantized Ascend MoE weights change layout during online RL weight updates with MTP.The target model and the MTP drafter receive actor weights through
model.load_weights(). After loading, Ascend convertsw13_weightandw2_weightfrom the loadable layout to the execution layout. The previous implementation registered rawtorch.nn.Parameterobjects during this conversion, which dropped the existing expert-awareweight_loader.In addition,
NPUWorker.wake_up()restored the loadable layout only for the target model, while the MTP drafter is a separate model that receives the same online updates.This change:
replace_parameter()helper when converting unquantized MoE weights in both layout directions, preserving an existingweight_loader;wake_up()whenspeculative_config.method == "mtp";The change does not add model code, environment variables, global state, or a framework-side patch.
Does this PR introduce any user-facing change?
Yes. In MTP mode,
NPUWorker.wake_up()now applies the existing unquantized MoE layout preparation to both the target and the MTP drafter.There is no API or configuration change, and non-MTP speculative methods keep their existing behavior.
How was this patch tested?
Unit coverage is included for:
w13_weightandw2_weightlayout, contiguity, andweight_loaderafterprocess_weights_after_loading();NPUWorker.wake_up();Manual integration validation was performed with:
Both modes completed multiple training iterations without

weight_loadererrors. Draft acceptance and reward remained normal.Additional checks: