Skip to content

Conversation

@freeliuzc
Copy link
Collaborator

Motivation

💡 If this PR is a Cherry Pick, the PR title needs to follow the format by adding the [Cherry-Pick] label at the very beginning and appending the original PR ID at the end. For example, [Cherry-Pick][CI] Add check trigger and logic(#5191)

💡 如若此PR是Cherry Pick,PR标题需遵循格式,在最开始加上[Cherry-Pick]标签,以及最后面加上原PR ID,例如[Cherry-Pick][CI] Add check trigger and logic(#5191)

Modifications

Usage or Command

Accuracy Tests

Checklist

  • Add at least a tag in the PR title.
    • Tag list: [[FDConfig],[APIServer],[Engine], [Scheduler], [PD Disaggregation], [Executor], [Graph Optimization], [Speculative Decoding], [RL], [Models], [Quantization], [Loader], [OP], [KVCache], [DataProcessor], [BugFix], [Docs], [CI], [Optimization], [Feature], [Benchmark], [Others], [XPU], [HPU], [GCU], [DCU], [Iluvatar], [Metax]]
    • You can add new tags based on the PR content, but the semantics must be clear.
  • Format your code, run pre-commit before commit.
  • Add unit tests. Please write the reason in this PR if no unit tests.
  • Provide accuracy results.
  • If the current PR is submitting to the release branch, make sure the PR has been submitted to the develop branch, then cherry-pick it to the release branch with the [Cherry-Pick] PR tag.

Copilot AI review requested due to automatic review settings December 23, 2025 12:48
@paddle-bot
Copy link

paddle-bot bot commented Dec 23, 2025

Thanks for your contribution!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

这是一个Cherry-Pick PR,旨在修复MTP(多步投机解码)在splitwise-prefill模式下的问题。该修复在配置后处理阶段添加了逻辑,当使用MTP方法且处于prefill角色时,将投机解码的token数量和模型步数限制为1。

主要变更:

  • 在配置后处理方法中添加了MTP在prefill模式下的特殊处理逻辑
  • 当检测到speculative_config使用"mtp"方法且splitwise_role为"prefill"时,自动调整配置参数

Comment on lines 1707 to 1711


# adjust speculative config
if self.speculative_config is not None and self.speculative_config.method == "mtp":
if self.scheduler_config.splitwise_role == "prefill:
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

缺少闭合引号。字符串 "prefill 后面应该有一个双引号来闭合字符串。这会导致语法错误。应该改为 "prefill"。

Suggested change
# adjust speculative config
if self.speculative_config is not None and self.speculative_config.method == "mtp":
if self.scheduler_config.splitwise_role == "prefill:
# adjust speculative config
if self.speculative_config is not None and self.speculative_config.method == "mtp":
if self.scheduler_config.splitwise_role == "prefill":

Copilot uses AI. Check for mistakes.
Comment on lines 1707 to 1708


Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

此处有两个连续的空行(1707和1708),而代码库的其他部分通常只使用一个空行来分隔代码块。为保持一致性,建议只保留一个空行。

Suggested change

Copilot uses AI. Check for mistakes.
Comment on lines 1710 to 1712
if self.speculative_config is not None and self.speculative_config.method == "mtp":
if self.scheduler_config.splitwise_role == "prefill:
self.speculative_config.num_speculative_tokens = 1
self.speculative_config.num_model_steps = 1
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

新增的MTP在splitwise prefill模式下的配置调整逻辑缺少相应的单元测试。建议在tests/utils/test_config.py中添加测试用例,验证当speculative_config.method为"mtp"且scheduler_config.splitwise_role为"prefill"时,num_speculative_tokens和num_model_steps是否正确设置为1。

Copilot uses AI. Check for mistakes.
Comment on lines 1710 to 1712
if self.speculative_config is not None and self.speculative_config.method == "mtp":
if self.scheduler_config.splitwise_role == "prefill:
self.speculative_config.num_speculative_tokens = 1
self.speculative_config.num_model_steps = 1
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

PR描述中缺少关键信息。根据要求,PR描述至少应说明为什么进行这些修改以及正在解决什么问题。当前PR描述只有模板内容,没有填写Motivation(动机)、Modifications(修改内容)等必要信息。建议补充:1) 此修复解决的具体问题是什么;2) 为什么在splitwise prefill模式下需要将num_speculative_tokens和num_model_steps设置为1;3) 此修复如何解决问题。

Copilot generated this review using guidance from repository custom instructions.
@freeliuzc freeliuzc force-pushed the merge_fix_mtp_splite_prefill_1131 branch from 268b468 to 3980252 Compare December 23, 2025 12:58
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (release/online/20251131@1b74540). Learn more about missing BASE report.

Files with missing lines Patch % Lines
fastdeploy/config.py 25.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##             release/online/20251131    #5724   +/-   ##
==========================================================
  Coverage                           ?   59.08%           
==========================================================
  Files                              ?      319           
  Lines                              ?    39106           
  Branches                           ?     5893           
==========================================================
  Hits                               ?    23105           
  Misses                             ?    14148           
  Partials                           ?     1853           
Flag Coverage Δ
GPU 59.08% <25.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yuanlehome yuanlehome merged commit b018c49 into PaddlePaddle:release/online/20251131 Dec 24, 2025
12 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants