[V1][Metrics] Initial speculative decoding metrics

[markmc]

Fixes #13990, part of #10582

cc @WoosukKwon @LiuXiaoxuanPKU @sroy745

[github-actions]

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

[markmc]

I'm a bit uncertain whether to include this in the count ... it matches what I was getting at the rejection sampler level, but ... I guess it depends what we want the system efficiency metric to mean.

Will dig into it, but thoughts welcome!

[markmc]

See commit 586f123

    [V1][Speculative Decoding] Properly account for ngram drafter
    
    When the ngram drafter proposes zero tokens, we should treat
    this as a proposal of num_spec_tokens which were all rejected.
    This will allow us to compare fairly across ngram vs draft
    models.
    
    Encode a zero-token proposal in ModelRunnerOutput as an
    empty list, whereas None means no speculating was done. This
    also requires Request.spec_token_ids empty list to be
    interpretted this way by the scheduler.

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @markmc.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @markmc.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[markmc]

I've rebased and pulled out a bunch of stuff from this PR to try and simplify the discussion:

• I've removed the system efficiency calculation, along with the num_proposals tracking
• I think we should remove num_emitted_tokens also since it's mainly there for the system efficiency calculation

On the topic of properly tracking num_accepted_tokens, using Lily's example:

When the draft model proposes tokens, it also assigns a probability to each token. For example, suppose the draft model proposes three tokens [t1, t2, t3] with associated probabilities [0.6, 0.7, 0.6]. The target model then evaluates these tokens and provides its own probabilities, say [0.7, 0.2, 0.8]. To give some intuition, the draft model is quite confident about t2 (0.7), but the target model assigns a much lower probability (0.2), suggesting it likely considers t2 incorrect.
After rejection sampling, V0 produces an accepted mask—for this example, [1, 0, 1]. This mask is computed purely based on the raw probabilities, without accounting for whether previous tokens were accepted. From a decoding perspective, only t1 should be considered accepted—we stop at the first rejected token (t2), and don’t proceed to t3. So the actual number of accepted tokens is 1.

This PR will calculate num_accepted_tokens = 1 correctly, and it's pretty clear why - because we're doing this in the scheduler, we actually have no insight into the probability assigned to t3 by the target model

[markmc]

I've removed num_emitted_tokens too now, and added a scheduler test case

[markmc]

Note also, at @WoosukKwon's request we retain the most simplistic behavior for ngram prompt lookup - for num_draft_tokens, we only count tokens that the drafter proposed instead of, for example, counting num_speculative_tokens for each step. This means an acceptance rate of 0.5 would be common with ngram, but this acceptance rate probably gives nowhere near the speedup achieved by a draft model with acceptance rate of 0.5

[markmc]

For reference, here is a mini design doc we've been using to discuss the above

[markmc]

Rebased to pull in CI fix from #15757

[WoosukKwon]

Looks generally good to me. However, as we discussed offline, I think the draft & accepted number of tokens per position (0, 1, ..., num_speculative_tokens-1) would be very much useful to understand how well the draft model is doing. WDYT?

[markmc]

Looks generally good to me. However, as we discussed offline, I think the draft & accepted number of tokens per position (0, 1, ..., num_speculative_tokens-1) would be very much useful to understand how well the draft model is doing. WDYT?

Absolutely. I think there's a lot more ideas to explore. I'd like to take this incrementally though and start by adding back the acceptance rate metric that we had in V0. This lays the groundwork for adding future metrics also.

[markmc]

Looks generally good to me. However, as we discussed offline, I think the draft & accepted number of tokens per position (0, 1, ..., num_speculative_tokens-1) would be very much useful to understand how well the draft model is doing. WDYT?

Absolutely. I think there's a lot more ideas to explore. I'd like to take this incrementally though and start by adding back the acceptance rate metric that we had in V0. This lays the groundwork for adding future metrics also.

Unless we think the acceptance rate metric (i.e. num_draft and num_accepted) itself is useless and we want to deprecate those too! I don't believe so, though. I think we will continue to want a single metric to compare performance over time and between models.

[WoosukKwon]

@markmc Got it. Looks good to me as the first step.