[V1][Spec Decoding] Add num_drafts and num_accepted_tokens_per_position metrics

[markmc]

As discussed in the design doc.

Record num_drafts so that it can be used to calculate mean acceptance length.

Record num_accepted_tokens per draft position and log these as per-position acceptance probabilities. In Prometheus, use a Counter with a position label - somewhat similar to how a Histogram works, but acknowledging token positions are discrete values and we're not bucketing continuous values.

[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]

FYI @luyuzhe111 @WoosukKwon @LiuXiaoxuanPKU @ekagra-ranjan

[wwl2755]

I think the PR is comprehensive and well-structured!

Just wondering should we have some comprehensive testing covers edge cases, because a lot of upcoming debuging work would depend on the metrics? Maybe we could make them in another PR since we could include all unit tests related to the V1 spec decoding metrics as they are missing right now.

[markmc]

I think the PR is comprehensive and well-structured!

Thanks!

Just wondering should we have some comprehensive testing covers edge cases, because a lot of upcoming debuging work would depend on the metrics? Maybe we could make them in another PR since we could include all unit tests related to the V1 spec decoding metrics as they are missing right now.

Yes, unit tests would be helpful 👍

[markmc]

For the record, I just tried:

VLLM_LOGGING_LEVEL=DEBUG VLLM_USE_V1=1 VLLM_ENABLE_V1_MULTIPROCESSING=1 vllm serve meta-llama/Meta-Llama-3-8B-Instruct --speculative-config '{"method": "eagle", "model" : "abhigoyal/EAGLE-LLaMA3-Instruct-8B-vllm", "num_speculative_tokens": 4}'

(some of those env vars are probably not necessary)

with

python ./benchmarks/benchmark_serving.py --model meta-llama/Meta-Llama-3-8B-Instruct --tokenizer meta-llama/Meta-Llama-3-8B-Instruct --endpoint /v1/completions --dataset-name sharegpt --dataset-path ./ShareGPT_V3_unfiltered_cleaned_split.json --request-rate 3.0 --num-prompts 200

and got

INFO 04-16 09:49:04 [metrics.py:82] SpecDecoding metrics: Draft acceptance rate: 28.8%, Mean acceptance length: 1.15, Accepted: 686 tokens, Drafted: 2384 tokens, Per-position acceptance probabilities: 0.576, 0.327, 0.159, 0.089
INFO 04-16 09:49:14 [metrics.py:82] SpecDecoding metrics: Draft acceptance rate: 40.6%, Mean acceptance length: 1.62, Accepted: 2951 tokens, Drafted: 7264 tokens, Per-position acceptance probabilities: 0.695, 0.442, 0.286, 0.202
INFO 04-16 09:49:24 [metrics.py:82] SpecDecoding metrics: Draft acceptance rate: 30.5%, Mean acceptance length: 1.22, Accepted: 3345 tokens, Drafted: 10984 tokens, Per-position acceptance probabilities: 0.588, 0.336, 0.185, 0.109
INFO 04-16 09:49:34 [metrics.py:82] SpecDecoding metrics: Draft acceptance rate: 30.9%, Mean acceptance length: 1.24, Accepted: 4029 tokens, Drafted: 13024 tokens, Per-position acceptance probabilities: 0.600, 0.333, 0.189, 0.115
INFO 04-16 09:49:44 [metrics.py:82] SpecDecoding metrics: Draft acceptance rate: 30.5%, Mean acceptance length: 1.22, Accepted: 3682 tokens, Drafted: 12084 tokens, Per-position acceptance probabilities: 0.575, 0.335, 0.189, 0.119
INFO 04-16 09:49:54 [metrics.py:82] SpecDecoding metrics: Draft acceptance rate: 32.1%, Mean acceptance length: 1.28, Accepted: 2969 tokens, Drafted: 9260 tokens, Per-position acceptance probabilities: 0.581, 0.346, 0.216, 0.139
INFO 04-16 09:50:04 [metrics.py:82] SpecDecoding metrics: Draft acceptance rate: 29.4%, Mean acceptance length: 1.18, Accepted: 3545 tokens, Drafted: 12048 tokens, Per-position acceptance probabilities: 0.579, 0.319, 0.176, 0.103

It would be nice to do this online inferencing run with an equivalent setup to @ekagra-ranjan and @luyuzhe111 offline inferencing setup - e.g. mt_bench and yuhuili/EAGLE-LLaMA3-Instruct-8B - because the mean acceptance length looks quite different?

[markmc]

Just wondering should we have some comprehensive testing covers edge cases, because a lot of upcoming debuging work would depend on the metrics? Maybe we could make them in another PR since we could include all unit tests related to the V1 spec decoding metrics as they are missing right now.

Yes, unit tests would be helpful 👍

Ok, I extended the existing scheduler unit test for SpecDecodingStats - at least that's some coverage 👍

[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

[WoosukKwon]

@markmc Thanks for the PR!

One thing I'm confused about is the per-position acceptance probability. I've observed in the log that
mean acceptance length == sum(p1, p2, p3) when num_spec_tokens is 3.
Is this expected? I was expecting that the mean acceptance length would be 1 * p1 + 2 * p2 + 3 * p3.

INFO 04-23 22:03:06 [metrics.py:82] SpecDecoding metrics: Draft acceptance rate: 29.3%, Mean acceptance length: 0.88, Accepted: 754 tokens, Drafted: 2574 tokens, Per-position acceptance probabilities: 0.535, 0.233, 0.111

[markmc]

One thing I'm confused about is the per-position acceptance probability. I've observed in the log that mean acceptance length == sum(p1, p2, p3) when num_spec_tokens is 3. Is this expected? I was expecting that the mean acceptance length would be 1 * p1 + 2 * p2 + 3 * p3.

Wonderful question! I hadn't considered the relationship at all

1 * p1 + 2 * p2 + 3 * p3 would be correct if e.g. p2 was "the probability that a draft has exactly 2 tokens accepted", but it is actually "the probability that a draft has 2 or 3 tokens accepted".

Simple worked example

num_spec_tokens = 3

drafts:
- #1: 3 accepted
- #2: 1 accepted
- #3: 2 accepted
- #4: 2 accepted
- #5: 1 accepted

observe:
- num_drafts = 5
- num_draft_tokens = 15
- num_accepted_tokens = 9
- accepted_tokens_per_pos = [5, 3, 1]

compute:
- acceptance_rate = 9/15 = 0.6
- mean_acceptance_length = 1.8
- acceptance_probs_per_pos = [1.0, 0.6, 0.2]

HTH!

[WoosukKwon]

LGTM. Thanks for addressing my comments!

[davidxia]

Hi, just curious if this changed the behavior of http_* metrics? I used these CPU dev setup instructions and built from source. I ran vllm serve TinyLlama/TinyLlama-1.1B-Chat-v1.0 on the commit that was merged 340d7b1 into main branch and don't see anything with curl --silent localhost:8000/metrics | grep http. But on the parent commit 1bcbcbf, the metrics appear.

If I revert that commit on tip of main, these metrics also exist. But on git tag v0.8.5, the metrics don't exist.

If I compile from source for GPU, the metrics don't show up before 340d7b1 all the way back to at least v0.8.3.

Can someone confirm if this http_* metric change occurred and if it is intentional?

I don't see how this change removed the http_* metrics. The only metrics removed from vllm/v1/metrics/loggers.py are ones related to speculative decoding.

[markmc]

Hi, just curious if this changed the behavior of http_* metrics? I used these CPU dev setup instructions and built from source. I ran vllm serve TinyLlama/TinyLlama-1.1B-Chat-v1.0 on the commit that was merged 340d7b1 into main branch and don't see anything with curl --silent localhost:8000/metrics | grep http.

Thanks for the report! But I can't reproduce this, using the same commit and same command, I see the HTTP metrics

But on the parent commit 1bcbcbf, the metrics appear.

If I revert that commit on tip of main, these metrics also exist. But on git tag v0.8.5, the metrics don't exist.

Can someone confirm if this http_* metric change occurred and if it is intentional?

I don't see how this change removed the http_* metrics. The only metrics removed from vllm/v1/metrics/loggers.py are ones related to speculative decoding.

The HTTP metrics were fixed in in V1 by #15894

[davidxia]

@markmc thanks for checking. How are you installing and running vllm? Are you installing from source for GPU with conda as documented here?

Feel free to message me on Slack. I'm dxia there. Relevant thread.