Support mm_processor_cache in the Transformers multimodal backend - #50107
Support mm_processor_cache in the Transformers multimodal backend#50107harshaljanjani wants to merge 15 commits into
Conversation
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
There was a problem hiding this comment.
hey @harshaljanjani !
Discussed internally with Harry so sharing with you
TBH this approach seems to have a few edge cases with multi-image inputs. I merged just yesterday a PR on transformer that updated all VLMs to enable returning placeholder_metadata.
Since I was already planning to update things on vllm after that PR, imo it'd be easier for us to try enable mm_processor_caching. The idea is to remove mm-token-type ids and instead ask for replacement offsets dict (return_text_replacement_offsets=True). It will be a dict of this format for each image/video/audio.
{
"type": "image",
"span": (start, end),
"new_span": (start_with_offset, start_with_offset + len(replacement_text)), # char span, not token id
"text": m.group(),
"replacement_text": replacement_text,
}
The major issue with caching was that the backend didn't have a working self.get_prompt_updates. I haven't tried anything yet myself, so it is a very raw idea. I am planning to start working on it next week but if you want to take this further, please feel free to check out to transformers main and draft a PoC for "mm_processor_cache support in Transformers backend". I will be able to hop in and help next week 😄
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
|
Thanks for taking the time to flesh out the discussion and what you had in mind @zucchini-nlp :) |
|
@harshaljanjani wow, that was fast! Correct, we will need to allow the arg as |
|
@zucchini-nlp I've raised huggingface/transformers#47614, thanks! |
zucchini-nlp
left a comment
There was a problem hiding this comment.
Left a few questions, though the main one is why we need to re-purpose offsets as a new mm-field and can we instead rely on raw hf_processor_mm_kwargs when updating prompt replacements? I was hoping to reduce "magically inferred stuff" by introducing this metadata 😅
I haven't been following vLLM for a while, so cmiiw anywhere and cc @hmellor for another pair of eyes to review
|
Oke, to summarize and align on next steps:
LMK if first two points make sense, that would clear up a lot of "magical" code imo. The last point is more about format/style, and not so much magically inferred |
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
|
@zucchini-nlp Yep makes sense, went ahead and did it on both sides, pushed to here 1fbbf5 and Transformers faeea1 so you can look directly 🤗 → Looking forward to your thoughts! |
|
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
|
Documentation preview: https://vllm--50107.org.readthedocs.build/en/50107/ |
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
zucchini-nlp
left a comment
There was a problem hiding this comment.
Amazing work, thanks for iterating on this! LGTM, maybe we can add more tests with different ckpt but it's up to vllm maintainers
One thing I wanted to ask, I am seeing that you deleted the tokenizer.decode when ids are passed. IIUC now we take base class'es apply_text_only/mm_only path which allows us to support tokenized inputs more seamlessly? Did you by chance test it, if not dw :)
| processed_data["num_image_patches"] = torch.tensor( | ||
| mm_tokens["num_image_patches"] |
There was a problem hiding this comment.
i don't like having this which forces us to hold an unused helper in transformers, but it's not on you. @hmellor , I will keep this in mind and try to delete in the future
There was a problem hiding this comment.
Which helper are you referring to?
I see lots of references to num_image_patches/get_number_of_image_patches in Transformers main
There was a problem hiding this comment.
I meant calling processor._get_num_multimodal_tokens which iiuc is not used outside vllm. The number of patches is used by CB atm as well since our pixels are shaped differently
I guess we either call directly get_number_of_image_patches in vllm or try to set easier way to infer/split total pixels per input image
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
Yep that's exactly it, and it's covered by an existing test which doesn't regress with this PR, and across all models (except the Voxtral XFAIL being worked on rn) the backend tests cover the placeholders are identical either way |
|
Actually qq, won't we need a |
|
ah forgot we didn't release it yet. Then we'll need to wait a bit until the next transformers release which should be around next week and until vllm bumps that version |
hmellor
left a comment
There was a problem hiding this comment.
First pass from me, could we update the PR description too to accurately represent the current state
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
@hmellor Done, rewrote and pushed the changes. |
|
pulling this out of offline discussions: I think chat-template prompts stay broken after this PR, #50899 patches them in the backend text path, which this PR removes. logiflow is, IIRC: the template renders bos itself. The renderer then encodes the rendered string with add_special_tokens=True. This happens for offline LLM.chat (mm default since #34510 ) and for pre-rendered strings passed to generate. The online endpoint passes False and is unaffected. in #50899 I put So the fix must be on the renderer, I think in |
|
Can confirm the double BOS comes from _tokenize_prompt, so fixing the renderer seems like the right approach. But an important thing I found is that of the models this backend covers, I could find only Gemma 3's template actually renders |
|
With this PR we would rely 100% on |
Signed-off-by: Harshal Janjani <harshaljanjani@gmail.com>
|
Done! |
|
A few more problems:
|
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
|
in the meantime I've just pushed a small commit with some naming improvements so it's easier to understand what's going on |
|
@harshaljanjani hey, thanks a lot for your help ❤️ |
|
Thank you @hmellor and @zucchini-nlp, and noted on the takeover :) |
|
This pull request has merge conflicts that must be resolved before it can be |
What does this PR do?
→ Follow-up to the Transformers audio backend, dropping the
return_mm_token_type_idsrequest to the processor and the cache-bypassingapply()override so mm_processor_cache works with the backend._get_prompt_updatesnow locates placeholders purely from the processor'sreturn_text_replacement_offsets, and both the prompt-update target and the embed token are derived from those offsets, so the per-model_get_{audio,image}_token_idlookups are gone.→ Requires the companion Transformers PR for Gemma3, Granite Speech and VibeVoice, whose processors only emit offsets once migrated to the base
__call__. AudioFlamingo3 and GLM-ASR already emit them.→ Not supported: Passing pre-computed embeddings through this backend. The offsets come from the HF processor, which is skipped entirely when embeddings are passed, so the extra fields the backend needs are never produced. Documented next to the
enable_mm_embedswarning inmultimodal_inputs.md.Verified no regressions:
→
tests/models/multimodal/processing/test_transformers_image.pyandtest_transformers_audio.py→ 9 passed, 1 xfailed (the Voxtral xfail, tracked in #49958)→
tests/models/multimodal/generation/test_transformers_audio.py→ 4 passed→
tests/models/multimodal/generation/test_common.py -k transformers→ 4 passed, 1 skipped→ Models: LLaVA-OneVision and Gemma3 placeholders are identical (Gemma3 stays 260 tokens with 256 embed positions), and Granite Speech, AudioFlamingo3, VibeVoice and GLM-ASR all generate matching HF
cc: @hmellor
Before
After
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.