[Bugfix] Incorrect MM data format in vllm bench throughput
#26395
Conversation
Signed-off-by: DarkLight1337 <[email protected]>
DarkLight1337
added
the
ready
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
| for request in requests: | ||
| prompts.append( | ||
| TokensPrompt( | ||
| prompt_token_ids=request.prompt["prompt_token_ids"], | ||
| multi_modal_data=request.multi_modal_data, | ||
| ) | ||
| prompt = ( | ||
| TokensPrompt(prompt_token_ids=request.prompt["prompt_token_ids"]) | ||
| if "prompt_token_ids" in request.prompt |
There was a problem hiding this comment.
Inside the request loop a prompt object is built but never appended to prompts. The later loop for i, (prompt, sp, lr) in enumerate(zip(prompts, sampling_params, lora_requests)): therefore iterates zero times, so no calls to llm.generate are issued and the benchmark completes without exercising the engine, returning a meaningless near‑zero latency. This regresses all vllm bench throughput runs that rely on run_vllm_async.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes a bug where multi_modal_data could be incorrectly passed as None in vllm bench throughput, causing an error. The change in run_vllm_async correctly avoids this by only setting multi_modal_data if it has a value. However, the added assertion to check the type of multi_modal_data is too restrictive and could cause crashes with certain datasets. A similar bug of passing None also appears to exist in the run_vllm function, which might be worth addressing for consistency.
…oject#26395) Signed-off-by: DarkLight1337 <[email protected]>
…oject#26395) Signed-off-by: DarkLight1337 <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>
…oject#26395) Signed-off-by: DarkLight1337 <[email protected]> Signed-off-by: Dhruvil Bhatt <[email protected]>
…oject#26395) Signed-off-by: DarkLight1337 <[email protected]>
…oject#26395) Signed-off-by: DarkLight1337 <[email protected]>
…oject#26395) Signed-off-by: DarkLight1337 <[email protected]>
…oject#26395) Signed-off-by: DarkLight1337 <[email protected]> Signed-off-by: xuebwang-amd <[email protected]>
…oject#26395) Signed-off-by: DarkLight1337 <[email protected]>
2 participants
Purpose
FIX #26320
According to the definition of
TextPrompt/TokensPrompt,multi_modal_datais marked asNotRequired[MultiModalDataDict], which means that if the item exists in the dictionary, it must beMultiModalDataDict, notNone. However invllm bench servethe data is passed asNonewhich results in this error.cc @huydhn
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.