Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Feature]: MLPSpeculator Tensor Parallel support #5809

Closed
njhill opened this issue Jun 25, 2024 · 3 comments · Fixed by #6050
Closed

[Feature]: MLPSpeculator Tensor Parallel support #5809

njhill opened this issue Jun 25, 2024 · 3 comments · Fixed by #6050
Labels
feature request New feature or request

Comments

@njhill
Copy link
Member

njhill commented Jun 25, 2024

🚀 The feature, motivation and pitch

MLPSpeculator-based speculative decoding was recently added in #4947, but the initial integration only covers single GPU usage.

There will soon be "speculator" models available for larger target models that require multiple GPUs so we would like to ensure that TP can be used.

The first part of this issue would be testing it out in conjunction with #5414 and making necessary adjustments so that it will work with TP=1 for the speculator and TP=N for the target model.

Following this we can look at having the speculator itself run with TP>1, but that may be more involved since it will require some distributed coordination of the sampling of each speculated token in the MLPSpeculator loop. It might be possible to avoid additional communication here by the having the sampler used by the speculator model use a dedicated torch.Generator for its sampling and doing this sampling in tandem across the ranks.

@JRosenkranz already used VocabParallelEmbedding in the implementation so the model layers themselves should work fine.

cc @cadedaniel @sirejdua @JRosenkranz @tdoublep

@njhill njhill added the feature request New feature or request label Jun 25, 2024
@cadedaniel
Copy link
Collaborator

initial thought:

@njhill
Copy link
Member Author

njhill commented Jun 25, 2024

we can start today with a small model (don't have to wait for new MLPSpeculator), the result should generalize to larger target models.

Yes sorry I should have made that clear, the large models are more the motivation but it can be developed/tested with existing ones.

@sirejdua
Copy link
Contributor

Thanks for writing this up @njhill , I can start working on it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants