Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Spec Decode] Disable Log Prob serialization to CPU for spec decoding for both draft and target models. #6485

Merged
merged 45 commits into from
Jul 21, 2024

Conversation

sroy745
Copy link
Contributor

@sroy745 sroy745 commented Jul 16, 2024

In this PR we disable the serialization of the LogProbs to CPU for both draft and target models. To that end we make the following changes

  1. Add top level flags to allow users to configure if they want token log probabilities during speculative decoding
  2. Introduce a light weight TargetModelRunner using which we can set the SamplingMetadata.skip_sampler_cpu_output as needed.
  3. Made changes to SpecDecodeWorker to skip serialization of log probability tensors if not needed.
  4. Fixed clearing out the appropriate tensor in SamplerOutput. E.g. earlier we were clearing sampler_output.probs which is not a valid tensor.
  5. Fixed failing tests as needed.

Copy link

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only trigger fastcheck CI to run, which consists only a small and essential subset of tests to quickly catch errors with the flexibility to run extra individual tests on top (you can do this by unblocking test steps in the Buildkite run).

Full CI run is still required to merge this PR so once the PR is ready to go, please make sure to run it. If you need all test signals in between PR commits, you can trigger full CI as well.

To run full CI, you can do one of these:

  • Comment /ready on the PR
  • Add ready label to the PR
  • Enable auto-merge.

🚀

@sroy745 sroy745 marked this pull request as draft July 16, 2024 20:45
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Jul 19, 2024
@sroy745 sroy745 changed the title [WIP] [Spec Decode] Disable Log Probs computation for spec decoding for both draft and target models. [Spec Decode] Disable Log Probs for spec decoding for both draft and target models. Jul 19, 2024
@sroy745 sroy745 changed the title [Spec Decode] Disable Log Probs for spec decoding for both draft and target models. [Spec Decode] Disable Log Prob serialization to CPU for spec decoding for both draft and target models. Jul 19, 2024
@sroy745 sroy745 marked this pull request as ready for review July 19, 2024 19:49
@sroy745
Copy link
Contributor Author

sroy745 commented Jul 19, 2024

@cadedaniel this pr is now ready for review. PTAL.

Copy link
Collaborator

@cadedaniel cadedaniel left a comment

Choose a reason for hiding this comment

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

great stuff, thanks! all feedback is code cleanliness/comments

vllm/spec_decode/spec_decode_worker.py Outdated Show resolved Hide resolved
vllm/spec_decode/spec_decode_worker.py Outdated Show resolved Hide resolved
vllm/spec_decode/target_model_runner.py Outdated Show resolved Hide resolved


class TargetModelRunner(ModelRunner):
"""Specialized model runner for speculative decoding target model.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: add comment explaining why we do this

In speculative decoding, the logprobs selected may not be the same ones as
selected by the target model sampling. This means that the time spent in the
logprob calculation of the target model is time wasted, since we calculate
logprobs after deciding which tokens are accepted. For this reason we disable
logprobs in the target model so scoring is faster.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

vllm/spec_decode/spec_decode_worker.py Outdated Show resolved Hide resolved
vllm/config.py Outdated Show resolved Hide resolved
@sroy745
Copy link
Contributor Author

sroy745 commented Jul 21, 2024

Thanks for the review. Addressed all comments. PTAL.

@cadedaniel
Copy link
Collaborator

Enabling auto merge

@cadedaniel cadedaniel merged commit 14f91fe into vllm-project:main Jul 21, 2024
72 checks passed
xjpang pushed a commit to xjpang/vllm that referenced this pull request Jul 24, 2024
gnpinkert pushed a commit to gnpinkert/vllm that referenced this pull request Jul 26, 2024
cduk pushed a commit to cduk/vllm-pascal that referenced this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants