Repurpose Scheduler Spec Dec metric for testing correctness

[ekagra-ranjan]

I was looking into SD metrics in V1 and find that spec_decoding_stats is reinit every time we do an engine step and we use an observe function which from the name is supposed to aggregate over miltiple observe calls. However, since it's reinit everytime, we will always have 1 observe call and there is no aggregation.

To enable AL computation for checking correctness, this PR aggregates the metrics across steps in the EngineCoreOutputs.scheduler_stats

[ekagra-ranjan]

num_spec_proposal is the num of times the SD call was made

mean_accepted_tokens = (sum of generated tokens over num_spec_proposal) / num_spec_proposal
= (num_spec_proposal + sum of accepted tokens over num_spec_proposal) / num_spec_proposal
= 1 + num_accepted_tokens / num_spec_proposal

[ekagra-ranjan]

cache the spec_decoding_stats so that it keeps a running metric instead of reinit it every engine step

[ekagra-ranjan]

using sglang model so that the prev SGL bench is comparable: https://docs.google.com/document/d/18ETJLsnxR88Qq3VDk5Mq-Hb7vuE9o3VNZ-hhz-OqAXk/edit?usp=sharing

[markmc]

hi @ekagra-ranjan

I was looking into SD metrics in V1 and find that spec_decoding_stats is reinit every time we do an engine step and we use an observe function which from the name is supposed to aggregate over miltiple observe calls. However, since it's reinit everytime, we will always have 1 observe call and there is no aggregation.

Hmm, we discussed this on Slack shortly after you submitted this PR

spec_decoding_stats = self.make_spec_decoding_stats(
                    spec_decoding_stats,

a new SpecDecodingStats should only be created once per update_from_output() call - we should aggregate across all requests in a single step

   def make_spec_decoding_stats(
        self,
        spec_decoding_stats: Optional[SpecDecodingStats],
        ...
    ) -> Optional[SpecDecodingStats]:
        ...
        if spec_decoding_stats is None:
            spec_decoding_stats = SpecDecodingStats()
        ...
        return spec_decoding_stats

Your response, for reference:

Oh, the purpose of the SpecDecodingStats is just to aggregate across the req per step and is reinit per step. Then it is working fine.

[ekagra-ranjan]

Hi @markmc - yup, we are good. I am still using this hacky PR whenever I want to quickly find the AL for my evals since vllm-project#16367 is still not merged

[github-actions]

This pull request has been automatically marked as stale because it has not had any activity within 90 days. It will be automatically closed if no further activity occurs within 30 days. Leave a comment if you feel this pull request should remain open. Thank you!

[github-actions]

This pull request has been automatically closed due to inactivity. Please feel free to reopen if you intend to continue working on it. Thank you!