Repository navigation
[SpecDecode] Reduce TP communication for large-vocab draft models speculative decoding - #39419
Conversation
Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
There was a problem hiding this comment.
Code Review
This pull request introduces the get_top_tokens method to the Qwen3 and Qwen3-DFlash models, enabling vocab-parallel argmax operations that avoid all-gathering full logits. Feedback for qwen3_dflash.py suggests optimizing the fallback logic when draft_id_to_target_id is present; instead of falling back to full logits computation, the remapping can be applied directly to the result of the optimized get_top_tokens call to maintain performance benefits.
|
Since this is an opt-in feature, can we also add it to |
Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
|
I have resubmitted the adaptation for Previously, the EAGLE3 code always created self.draft_id_to_target_id = nn.Parameter(
torch.zeros(self.config.draft_vocab_size, dtype=torch.long),
requires_grad=False,
)Following the approach in I also ran P-EAGLE experiments on Dataset:
The results show a clear performance improvement after enabling this optimization. |
| if "d2t" in name: | ||
| # When draft_vocab_size == target vocab_size, the parameter | ||
| # is set to None (no remapping needed), so skip loading d2t. | ||
| if self.draft_id_to_target_id is None: |
There was a problem hiding this comment.
I'm not sure this is the right way to handle it. If there's a D2T in the checkpoint, it probably means it should be used and we messed up by setting draft_id_to_target_id to None
| logits_new[:, targets] = logits | ||
| return logits_new | ||
|
|
||
| def get_top_tokens( |
There was a problem hiding this comment.
I don't love that we have to duplicate this code on all the EAGLE models. Maybe we can have a mixin similar to EagleModelMixin or some similar pattern?
| ) | ||
| target_vocab_size = vllm_config.model_config.get_vocab_size() | ||
| if self.config.draft_vocab_size != target_vocab_size: | ||
| self.draft_id_to_target_id = nn.Parameter( |
There was a problem hiding this comment.
It sems like compute_logits is already expecting this to be set to None. Are you sure there isn't some other place in the code or weight loading pathway that's handling this already?
| speculative decoding expects target vocab ids. | ||
| """ | ||
| if self.draft_id_to_target_id is not None: | ||
| return self.compute_logits(hidden_states).argmax(dim=-1) |
There was a problem hiding this comment.
Is this fallback necessary? I feel like we could still just do get_top_tokens and then map those tokens into D2T, right? What's the problem with that?
There was a problem hiding this comment.
Fixed by changing the logic to perform local argmax first, then apply D2T mapping
Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
|
Hi @EanWang211123, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
|
Hi @EanWang211123, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
Refactor local argmax codeAdd Core logic: perform local argmax first, then apply D2T mapping as needed: target_id = k + draft_id_to_target_id[k]Add qwen3.5-mtp supportTested with 32 concurrent requests on 4090 × 4, TP4. Dataset: GSM8K enabledisable |
For the local-argmax integration for I also think this optimization mainly benefits devices like the 4090 that do not have NVLink. Perhaps most devices capable of deploying |
|
@EanWang211123 I think we could also add it for deepseek in a follow-up if needed. I'll move forward for now |
…culative decoding (vllm-project#39419) Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
…culative decoding (vllm-project#39419) Signed-off-by: EanWang211123 <wangyiheng@sangfor.com.cn>
Purpose
Use local argmax + gather reduction instead of all-gathering full-vocabulary logits to reduce TP communication overhead in speculative decoding modes such as DFlash and PARD.
Enabling
use_local_argmax_reductionimproves performance in all tested scenarios.In terms of Total token throughput (tok/s), the gains are:
Current Issue
Speculative decoding methods such as DFlash and PARD often require the draft model and target model to have the same vocabulary size. Under the current implementation, this requires all-gathering the full logits tensor across TP ranks, which incurs O(batch_size × vocab_size) communication cost per step. For example, PARD-Qwen3-0.6B uses a vocabulary size of 40960.
We can reuse the approach introduced in #34049 to reduce this communication overhead.
This PR adds a
use_local_argmax_reductionoption tospeculative_configthat:(max_value, global_index)pairs — O(batch_size × 2 × tp_size) instead of O(batch_size × vocab_size)Changes
Add
get_top_tokens()to the corresponding model files.Test Plan
Environment: 4090D
Target model: Qwen3-8B
Draft models: Qwen3-8B-DFlash-b16 and PARD-Qwen3-0.6B
Dataset: gsm8k
Test Commands
Test Results
Accuracy
Enabling
use_local_argmax_reductiondoes not change the acceptance rate.Qwen3-8B + Qwen3-8B-DFlash-b16
Concurrency: 1
use_local_argmax_reduction=trueuse_local_argmax_reduction=falsePerformance
Open Question
I noticed that almost all speculative decoding setups using a draft model, whether parallelized or not, seem to benefit from this optimization. Is there a more general way to integrate this feature?
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.