Skip to content

llama3.1-8b: use TokensPrompt in the Offline vLLM SUT and fail fast on worker death - #2670

Open
100-JM wants to merge 1 commit into
mlcommons:masterfrom
100-JM:fix/llama3.1-8b-vllm-tokensprompt
Open

100-JM wants to merge 1 commit into
mlcommons:masterfrom
100-JM:fix/llama3.1-8b-vllm-tokensprompt

Conversation

@100-JM

@100-JM 100-JM commented Sep 11, 2026

Copy link
Copy Markdown

Problem

SUT.process_queries (Offline) in language/llama3.1-8b/SUT_VLLM.py calls

self.model.generate(prompt_token_ids=input_ids_tensor, sampling_params=...)

prompt_token_ids= was a deprecated keyword that newer vLLM releases no longer accept. On vLLM 0.10.2 (NGC vllm:25.10-py3):

TypeError: LLM.generate() got an unexpected keyword argument 'prompt_token_ids'

SUTServer already passes pre-tokenized input as TokensPrompt objects, which works on both the pinned vllm==0.6.3 and current releases; only the Offline path was left on the old form.

The exception also exposed a second problem: it only killed the worker thread. LoadGen kept waiting for responses that would never arrive and the run sat idle until the job's wall-clock limit — four hours in our case — instead of failing.

Changes

  • Offline path builds [TokensPrompt(prompt_token_ids=...) for q in qitem] and passes it positionally, exactly as SUTServer does. Same tokens reach the model; no behavioural change on 0.6.3.
  • process_queries in both SUTs now wraps the worker loop: on an unhandled exception it logs the traceback and os._exit(1)s, so a broken SUT fails the run immediately instead of hanging LoadGen. If maintainers would rather keep the harness hands-off here, the first change stands alone.

Verification

  • Ran the Offline scenario on vLLM 0.10.2 (B300, TP1, nvidia/Llama-3.1-8B-Instruct-NVFP4) with the equivalent patch — generate([{"prompt_token_ids": ids}, …]), i.e. the same dict TokensPrompt constructs: performance VALID, accuracy run completes and passes all five evaluation metrics (ROUGE1/2/L/Lsum and gen_len above the 99% thresholds). The fail-fast wrapper was exercised only by the original TypeError run, where the thread died and LoadGen waited four hours.
  • python -m py_compile and autopep8 -a --max-line-length 79 clean. I could not exercise vllm==0.6.3 itself, but TokensPrompt has existed since 0.4.x and is what SUTServer already uses on that version.

Related: #2341 (Server/SingleStream EngineDeadError) is a different failure in the same file and is not addressed here.

🤖 Generated with Claude Code

… worker death

SUT.process_queries (Offline) called LLM.generate(prompt_token_ids=...),
a keyword that newer vLLM releases no longer accept:

    TypeError: LLM.generate() got an unexpected keyword argument
    'prompt_token_ids'

SUTServer already passes pre-tokenized input as TokensPrompt objects,
which works on both the pinned vllm==0.6.3 and current releases, so the
Offline path now does the same. No change in what is sent to the model.

The TypeError also exposed a second problem: the exception only killed
the worker thread. LoadGen kept waiting for responses that would never
arrive and the run sat idle until the job's wall-clock limit (four hours
in our case) instead of failing. process_queries in both SUTs now wraps
the worker loop, logs the traceback and exits the process, so a broken
SUT fails the run immediately.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@100-JM
100-JM requested review from a team as code owners September 11, 2026 10:45
@github-actions

Copy link
Copy Markdown
Contributor

MLCommons CLA bot:
Thank you very much for your submission; we really appreciate it. Before we can accept your contribution,
we ask that you sign the MLCommons CLA (Apache 2). Please submit your GitHub ID to our onboarding form to initiate
authorization. If you are from a MLCommons member organization, we will request that you be added to the CLA.
If you are not from a member organization, we will email you a CLA to sign. For any questions, please contact
support@mlcommons.org.
0 out of 1 committers have signed the MLCommons CLA.
@100-JM
You can retrigger this bot by commenting recheck in this Pull Request

@100-JM

100-JM commented Sep 12, 2026

Copy link
Copy Markdown
Author

recheck2

@100milliongold

Copy link
Copy Markdown

Adjacent finding from running this SUT with a recent vLLM, in case it belongs in this PR.

SUT_VLLM.py constructs the engine without touching prefix caching:

self.model = LLM(
    self.model_path,
    dtype=self.dtype,
    tensor_parallel_size=self.tensor_parallel_size,
)

so it inherits the vLLM default, which is now on:

CacheConfig().enable_prefix_caching  ->  True      # vLLM 0.28.1rc1.dev580+g385dce36b

This is not only nominal. A full Offline run on this SUT logged real cross-query reuse:

prefix_caching=True
Prefix cache hit rate: 1.7%
Prefix cache hit rate: 1.8%

inference_rules.adoc seems to draw the line at the batch boundary:

Q: Is it allowed to store continuous keys and values in non-contiguous memory space for the KV-cache, i.e. PagedAttention?
A: Yes, it is allowed as long as the KV-cache block is reused only within the batch of queries.

Prefix caching reuses blocks across queries by design, so a nonzero hit rate means blocks are being reused outside the batch that produced them. Line 459 ("caching of any other queries, query parameters, or intermediate results is prohibited") and line 542 point the same way.

Older vLLM defaulted this off, so the reference implementation was compliant when it was written. The default flipped with the V1 engine, and the SUT inherits whatever the installed version does. That makes compliance depend on the vLLM version rather than on the reference code.

If it fits the scope here, one line makes it explicit:

self.model = LLM(
    self.model_path,
    dtype=self.dtype,
    tensor_parallel_size=self.tensor_parallel_size,
    enable_prefix_caching=False,
)

SUTServer builds AsyncEngineArgs separately and would need the same treatment.

I have not confirmed how the working group reads this for prefix caching specifically; the rule text above is the closest thing I found. Happy to open a separate issue instead if that is the better place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants