[Bugfix] [SpecDecode] Default speculative_draft_tensor_parallel_size to 1 when using MLPSpeculator #7105
Conversation
…culator Signed-off-by: Thomas Parnell <tpa@zurich.ibm.com>
|
👋 Hi! Thank you for contributing to the vLLM project. Once the PR is approved and ready to go, please make sure to run full CI as it is required to merge (or just use auto-merge). To run full CI, you can do one of these:
🚀 |
| speculative_draft_tensor_parallel_size = \ | ||
| target_parallel_config.tensor_parallel_size |
There was a problem hiding this comment.
Looking at this logic now, I don't think it makes sense since below (L1157) we explicitly say that speculative_draft_tensor_parallel_size cannot be set to any value other than 1, but we default it to be the target_parallel_config.tensor_parallel_size which can be greater than 1?
There was a problem hiding this comment.
Should we just default it to 1 in all cases instead?
There was a problem hiding this comment.
I think for regular draft models it might currently work either with tp=1 or tp same as target model. That could be the reason for this logic (but it strangely still doesn't allow setting tp to the same as the target model explicitly).
…to 1 when using MLPSpeculator (vllm-project#7105) Signed-off-by: Thomas Parnell <tpa@zurich.ibm.com>
…to 1 when using MLPSpeculator (vllm-project#7105) Signed-off-by: Thomas Parnell <tpa@zurich.ibm.com>
…to 1 when using MLPSpeculator (vllm-project#7105) Signed-off-by: Thomas Parnell <tpa@zurich.ibm.com> Signed-off-by: Alvant <alvasian@yandex.ru>
…to 1 when using MLPSpeculator (vllm-project#7105) Signed-off-by: Thomas Parnell <tpa@zurich.ibm.com>
Let's default the value of
speculative_draft_tensor_parallel_sizeto 1 when we detect MLPSpeculator, since this is the only case that works right now.cc @njhill