Skip to content

Commit fe50bdc

Browse files
tmduc3claude
andcommitted
[Bugfix][Model] Pad SigLIP text prompts to the trained sequence length
SigLIP is trained with padding="max_length" and without an attention mask, so padding tokens are part of the input and the pooled embedding is read from the last position. Text embeddings computed without padding are not aligned with the image embeddings, which makes image-text similarity unusable. Callers that cannot pass tokenization kwargs — notably /v1/embeddings, which builds TokenizeParams from scratch in entrypoints/pooling/base/protocol.py — silently get wrong values: the request still returns 200 with a correctly shaped vector. Gate the default on the tokenizer's declared inputs, so a finetune trained with masking (attention_mask in model_input_names) opts out on its own. Test: each case in _run_test now carries separate HF and vLLM tokenization kwargs. With explicit_padding=False vLLM is told nothing about padding and still has to reach the same embedding as HF, which is the regression being guarded. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: tmduc3 <tmduc3@rd.misa.com.vn>
1 parent 5fd7a88 commit fe50bdc

5 files changed

Lines changed: 163 additions & 9 deletions

File tree

tests/models/multimodal/pooling/test_siglip.py

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@
3333
def _run_test(
3434
hf_runner: type[HfRunner],
3535
vllm_runner: type[VllmRunner],
36-
input_cases: list[tuple[list[str], PromptImageInput, dict[str, Any]]],
36+
input_cases: list[
37+
tuple[list[str], PromptImageInput, dict[str, Any], dict[str, Any]]
38+
],
3739
model: str,
3840
*,
3941
dtype: str,
@@ -50,9 +52,9 @@ def _run_test(
5052
vllm_model.embed(
5153
input_texts,
5254
images=input_images,
53-
tokenization_kwargs=tokenization_kwargs,
55+
tokenization_kwargs=vllm_tokenization_kwargs,
5456
)
55-
for input_texts, input_images, tokenization_kwargs in input_cases
57+
for input_texts, input_images, _, vllm_tokenization_kwargs in input_cases
5658
]
5759

5860
texts = [HF_TEXT_PROMPTS[0]]
@@ -65,7 +67,7 @@ def _run_test(
6567

6668
with hf_runner(model, dtype=dtype, auto_cls=SiglipModel) as hf_model:
6769
hf_outputs_per_case = []
68-
for input_texts, input_images, tokenization_kwargs in input_cases:
70+
for input_texts, input_images, tokenization_kwargs, _ in input_cases:
6971
all_inputs = hf_model.get_inputs(
7072
input_texts,
7173
images=input_images,
@@ -103,25 +105,34 @@ def _run_test(
103105

104106
@pytest.mark.parametrize("model", MODELS)
105107
@pytest.mark.parametrize("dtype", ["float"])
108+
@pytest.mark.parametrize("explicit_padding", [True, False])
106109
def test_models(
107110
hf_runner,
108111
vllm_runner,
109112
image_assets,
110113
model: str,
111114
dtype: str,
115+
explicit_padding: bool,
112116
) -> None:
117+
"""Text embeddings must match HF whether or not the caller asks for padding.
118+
119+
SigLIP is trained with ``padding="max_length"`` and vLLM now applies it by
120+
default, so ``explicit_padding=False`` sends vLLM no padding kwargs at all
121+
and still has to reach the same embedding. Callers that cannot pass
122+
tokenization kwargs (the OpenAI-compatible server) would otherwise get
123+
embeddings that are not aligned with the image embeddings.
124+
"""
113125
text_images = [None] * len(HF_TEXT_PROMPTS)
114126
images = [asset.pil_image for asset in image_assets]
127+
padding_kwargs = {"padding": "max_length", "max_length": 64}
115128
input_cases = [
116129
(
117130
HF_TEXT_PROMPTS,
118131
text_images,
119-
{
120-
"padding": "max_length",
121-
"max_length": 64,
122-
},
132+
padding_kwargs,
133+
padding_kwargs if explicit_padding else {},
123134
),
124-
(HF_IMAGE_PROMPTS, images, {}),
135+
(HF_IMAGE_PROMPTS, images, {}, {}),
125136
]
126137

127138
_run_test(

tests/renderers/test_completions.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class MockModelConfig:
3434
tokenizer_revision = None
3535
tokenizer_mode = "auto"
3636
hf_config = MockHFConfig()
37+
max_model_len: int = 1024
3738
encoder_config: dict[str, Any] | None = None
3839
enable_prompt_embeds: bool = True
3940
skip_tokenizer_init: bool = False
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
# SPDX-License-Identifier: Apache-2.0
2+
# SPDX-FileCopyrightText: Copyright contributors to the vLLM project
3+
"""Model-level padding defaults must reach frontends that build their own
4+
`TokenizeParams`, such as the OpenAI-compatible pooling endpoints."""
5+
6+
import pytest
7+
8+
from vllm.engine.arg_utils import EngineArgs
9+
from vllm.inputs import TextPrompt
10+
from vllm.renderers import TokenizeParams
11+
from vllm.renderers.hf import HfRenderer
12+
from vllm.tokenizers import get_tokenizer
13+
14+
MODEL = "google/siglip-base-patch16-224"
15+
TRAINED_LENGTH = 64
16+
17+
18+
@pytest.fixture(scope="module")
19+
def renderer() -> HfRenderer:
20+
vllm_config = EngineArgs(
21+
model=MODEL,
22+
runner="pooling",
23+
dtype="float32",
24+
max_model_len=TRAINED_LENGTH,
25+
enforce_eager=True,
26+
).create_engine_config()
27+
28+
return HfRenderer(vllm_config, get_tokenizer(MODEL))
29+
30+
31+
def _request_built_params() -> TokenizeParams:
32+
"""What the pooling endpoints build from a request: no padding field."""
33+
return TokenizeParams(
34+
max_total_tokens=TRAINED_LENGTH,
35+
max_output_tokens=0,
36+
add_special_tokens=True,
37+
)
38+
39+
40+
def test_model_default_is_applied(renderer: HfRenderer):
41+
assert renderer.default_cmpl_tok_params.pad_prompt_tokens == -1
42+
43+
prompt = TextPrompt(prompt="a photo of a stop sign")
44+
tokenized = renderer._tokenize_singleton_prompt(prompt, _request_built_params())
45+
46+
assert len(tokenized["prompt_token_ids"]) == TRAINED_LENGTH
47+
48+
49+
def test_explicit_padding_is_not_overridden(renderer: HfRenderer):
50+
params = TokenizeParams(
51+
max_total_tokens=TRAINED_LENGTH,
52+
max_output_tokens=0,
53+
pad_prompt_tokens=16,
54+
)
55+
prompt = TextPrompt(prompt="a photo of a stop sign")
56+
57+
tokenized = renderer._tokenize_singleton_prompt(prompt, params)
58+
59+
assert len(tokenized["prompt_token_ids"]) == 16
60+
61+
62+
def test_multimodal_prompts_are_not_padded(renderer: HfRenderer):
63+
"""Image prompts carry placeholder text that processing replaces."""
64+
prompt = TextPrompt(prompt="", multi_modal_data={"image": []})
65+
66+
tokenized = renderer._tokenize_singleton_prompt(prompt, _request_built_params())
67+
68+
assert len(tokenized["prompt_token_ids"]) < TRAINED_LENGTH
69+
70+
71+
def _declare_model_inputs(
72+
renderer: HfRenderer, monkeypatch: pytest.MonkeyPatch, names: list[str] | None
73+
):
74+
info = renderer.get_mm_processor().info
75+
init_kwargs = dict(info.get_tokenizer().init_kwargs)
76+
77+
if names is None:
78+
init_kwargs.pop("model_input_names", None)
79+
else:
80+
init_kwargs["model_input_names"] = names
81+
82+
monkeypatch.setattr(info.get_tokenizer(), "init_kwargs", init_kwargs)
83+
return info
84+
85+
86+
def test_checkpoint_consuming_attention_mask_opts_out(
87+
renderer: HfRenderer, monkeypatch: pytest.MonkeyPatch
88+
):
89+
"""A checkpoint that declares an attention mask is left unpadded."""
90+
info = _declare_model_inputs(renderer, monkeypatch, ["input_ids", "attention_mask"])
91+
92+
assert info.get_default_tok_params().pad_prompt_tokens is None
93+
94+
95+
def test_undeclared_model_inputs_still_pad(
96+
renderer: HfRenderer, monkeypatch: pytest.MonkeyPatch
97+
):
98+
"""`SiglipTokenizer.model_input_names` defaults to containing
99+
`attention_mask`, so a checkpoint that declares nothing must still pad."""
100+
info = _declare_model_inputs(renderer, monkeypatch, None)
101+
102+
assert info.get_default_tok_params().pad_prompt_tokens == -1

vllm/model_executor/models/siglip.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
PromptUpdate,
5454
TimingContext,
5555
)
56+
from vllm.renderers import TokenizeParams
5657
from vllm.sequence import IntermediateTensors
5758
from vllm.utils.gpu_sync_debug import gpu_sync_allowed
5859
from vllm.utils.tensor_schema import TensorSchema, TensorShape
@@ -146,6 +147,22 @@ def get_max_image_tokens(self) -> int:
146147
image_width=target_width, image_height=target_height
147148
)
148149

150+
def get_default_tok_params(self) -> TokenizeParams:
151+
# SigLIP is trained with padding="max_length" and no attention mask, so
152+
# the pooled embedding is read from the last position and unpadded text
153+
# does not line up with the image embeddings. A checkpoint that declares
154+
# attention_mask in tokenizer_config.json consumes a mask; leave it be.
155+
# init_kwargs is read rather than tokenizer.model_input_names, which
156+
# falls back to a class default that already contains attention_mask.
157+
declared = getattr(self.get_tokenizer(), "init_kwargs", {}).get(
158+
"model_input_names"
159+
)
160+
tok_params = super().get_default_tok_params()
161+
if declared is not None and "attention_mask" in declared:
162+
return tok_params
163+
164+
return tok_params.with_kwargs(pad_prompt_tokens=-1)
165+
149166

150167
class SiglipDummyInputsBuilder(BaseDummyInputsBuilder[SiglipProcessingInfo]):
151168
def get_dummy_text(self, mm_counts: Mapping[str, int]) -> str:

vllm/renderers/base.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from abc import ABC, abstractmethod
66
from collections.abc import Mapping, Sequence
77
from concurrent.futures import Executor, ThreadPoolExecutor
8+
from dataclasses import replace
89
from functools import cached_property
910
from typing import TYPE_CHECKING, Any, Generic, overload
1011

@@ -513,11 +514,31 @@ def _tokenize_singleton_prompt( # type: ignore[misc]
513514
params: TokenizeParams,
514515
) -> EmbedsPrompt: ...
515516

517+
def _apply_default_padding(
518+
self,
519+
prompt: SingletonDictPrompt,
520+
params: TokenizeParams,
521+
) -> TokenizeParams:
522+
# Frontends build TokenizeParams from the request and do not inherit
523+
# model-level defaults, so a model that is only correct with padded
524+
# inputs (SigLIP) would be served unpadded. Multi-modal prompts are left
525+
# alone: their text is replaced by placeholder tokens during processing.
526+
if params.pad_prompt_tokens is not None or prompt.get("multi_modal_data"):
527+
return params
528+
529+
default = self.default_cmpl_tok_params.pad_prompt_tokens
530+
if default is None:
531+
return params
532+
533+
return replace(params, pad_prompt_tokens=default)
534+
516535
def _tokenize_singleton_prompt(
517536
self,
518537
prompt: SingletonDictPrompt,
519538
params: TokenizeParams,
520539
) -> SingletonTokPrompt:
540+
params = self._apply_default_padding(prompt, params)
541+
521542
if "prompt_token_ids" not in prompt and "prompt_embeds" not in prompt:
522543
if not isinstance(prompt.get("prompt"), str):
523544
raise TypeError(
@@ -554,6 +575,8 @@ async def _tokenize_singleton_prompt_async(
554575
prompt: SingletonDictPrompt,
555576
params: TokenizeParams,
556577
) -> SingletonTokPrompt:
578+
params = self._apply_default_padding(prompt, params)
579+
557580
if "prompt_token_ids" not in prompt and "prompt_embeds" not in prompt:
558581
if not isinstance(prompt.get("prompt"), str):
559582
raise TypeError(

0 commit comments

Comments
 (0)