-
Notifications
You must be signed in to change notification settings - Fork 34.2k
feat[vLLM]: Support text replacement offsets in the remaining old-format processors #47614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
e3847e8
fc70dec
faeea18
d27ea0d
d69ba74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,16 +12,12 @@ | |
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| import re | ||
|
|
||
| from ...feature_extraction_utils import BatchFeature | ||
| from ...image_utils import ImageInput, make_nested_list_of_images | ||
| from ...processing_utils import MultiModalData, ProcessingKwargs, ProcessorMixin, Unpack | ||
| from ...tokenization_utils_base import PreTokenizedInput, TextInput | ||
| from ...utils import auto_docstring, logging, to_py_obj | ||
|
|
||
|
|
||
| logger = logging.get_logger(__name__) | ||
| from ...utils import auto_docstring, to_py_obj | ||
|
|
||
|
|
||
| class Gemma3ProcessorKwargs(ProcessingKwargs, total=False): | ||
|
|
@@ -55,6 +51,7 @@ def __init__( | |
| self.image_seq_length = image_seq_length | ||
| self.image_token_id = tokenizer.image_token_id | ||
| self.boi_token = tokenizer.boi_token | ||
| self.image_token = tokenizer.boi_token | ||
|
Comment on lines
+52
to
+54
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we swapping This is a breaking change for downstream libraries and it doesn't appear to be used anywhere else in this diff.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. discussed internally: it was deprecated and already time to update, the token in used by parent class methods with assumption that all placeholder tokens are called |
||
| image_tokens_expanded = "".join([tokenizer.image_token] * image_seq_length) | ||
| self.full_image_sequence = f"\n\n{tokenizer.boi_token}{image_tokens_expanded}{tokenizer.eoi_token}\n\n" | ||
|
|
||
|
|
@@ -72,67 +69,53 @@ def __call__( | |
| text: TextInput | PreTokenizedInput | list[TextInput] | list[PreTokenizedInput] = None, | ||
| **kwargs: Unpack[Gemma3ProcessorKwargs], | ||
| ) -> BatchFeature: | ||
| if text is None and images is None: | ||
| raise ValueError("Provide at least one of `text` or `images`.") | ||
| # Create empty text to be replaced with placeholders | ||
| if images is not None and not text: | ||
| images = self.image_processor.fetch_images(images) | ||
| text = [" ".join([self.boi_token] * len(images)) for images in make_nested_list_of_images(images)] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better places in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved! |
||
|
|
||
| output_kwargs = self._merge_kwargs( | ||
| Gemma3ProcessorKwargs, | ||
| tokenizer_init_kwargs=self.tokenizer.init_kwargs, | ||
| **kwargs, | ||
| ) | ||
| model_inputs = super().__call__(images=images, text=text, **kwargs) | ||
| if "mm_token_type_ids" in model_inputs: | ||
| model_inputs["token_type_ids"] = model_inputs.pop("mm_token_type_ids") | ||
| return model_inputs | ||
|
|
||
| if isinstance(text, str): | ||
| text = [text] | ||
| elif not isinstance(text, list) and not isinstance(text[0], str): | ||
| raise TypeError("Invalid input text. Please provide a string, or a list of strings") | ||
| def validate_inputs( | ||
| self, | ||
| images: ImageInput | None = None, | ||
| text: TextInput | PreTokenizedInput | list[TextInput] | list[PreTokenizedInput] | None = None, | ||
| **kwargs: Unpack[ProcessingKwargs], | ||
| ): | ||
| super().validate_inputs(images=images, text=text, **kwargs) | ||
|
|
||
| image_inputs = {} | ||
| if images is not None: | ||
| images = self.image_processor.fetch_images(images) | ||
| if images is not None and text is not None: | ||
| batched_images = make_nested_list_of_images(images) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same, make nested list is beter placed in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| image_inputs = self.image_processor(images, **output_kwargs["images_kwargs"]) | ||
|
|
||
| # Create empty text to be replaced with placeholders | ||
| if not text: | ||
| text = [" ".join([self.boi_token] * len(images)) for images in batched_images] | ||
|
|
||
| if len(batched_images) != len(text): | ||
| raise ValueError( | ||
| f"Received inconsistently sized batches of images ({len(batched_images)}) and text ({len(text)})." | ||
| ) | ||
|
|
||
| # Replace image tokens by the full expanded sequence | ||
| num_crops = to_py_obj(image_inputs.pop("num_crops")) | ||
| batch_num_crops = [[num_crops.pop(0) for _ in range(len(images))] for images in batched_images] | ||
| for batch_idx, (prompt, images, num_crops) in enumerate(zip(text, batched_images, batch_num_crops)): | ||
| image_indexes = [m.start() for m in re.finditer(self.boi_token, prompt)] | ||
|
|
||
| if len(images) != len(image_indexes): | ||
| for prompt, images in zip(text, batched_images): | ||
| if len(images) != prompt.count(self.boi_token): | ||
| raise ValueError( | ||
| f"Prompt contained {len(image_indexes)} image tokens but received {len(images)} images." | ||
| f"Prompt contained {prompt.count(self.boi_token)} image tokens but received {len(images)} images." | ||
| ) | ||
|
|
||
| # Insert additional image tokens for Pan-and-Scan crops | ||
| for num, idx in reversed(list(zip(num_crops, image_indexes))): | ||
| if num: | ||
| formatted_image_text = ( | ||
| f"Here is the original image {self.boi_token} and here are some crops to help you see better " | ||
| + " ".join([self.boi_token] * num) | ||
| ) | ||
| prompt = prompt[:idx] + formatted_image_text + prompt[idx + len(self.boi_token) :] | ||
| text[batch_idx] = prompt | ||
|
|
||
| # Expand placeholder image tokens to the full image token sequence | ||
| text = [prompt.replace(self.boi_token, self.full_image_sequence) for prompt in text] | ||
|
|
||
| return_tensors = output_kwargs["text_kwargs"].pop("return_tensors", None) | ||
| return_mm_token_type_ids = output_kwargs["text_kwargs"].pop("return_mm_token_type_ids", False) | ||
| text_inputs = self.tokenizer(text=text, **output_kwargs["text_kwargs"]) | ||
| self._check_special_mm_tokens(text, text_inputs, modalities=["image"]) | ||
|
|
||
| if return_mm_token_type_ids: | ||
| text_inputs["token_type_ids"] = self.create_mm_token_type_ids(text_inputs["input_ids"]) | ||
| return BatchFeature(data={**text_inputs, **image_inputs}, tensor_type=return_tensors) | ||
| def _check_special_mm_tokens(self, text: list[str], text_inputs: "BatchFeature", modalities: list[str]): | ||
| """ | ||
| Checks that number of special tokens in text and processed text is same. The count can be different | ||
| if tokenized text was truncated, leading to issues in model code. | ||
| """ | ||
| token_str = self.tokenizer.image_token | ||
| token_id = self.tokenizer.image_token_id | ||
| if token_str is not None and token_id is not None: | ||
| ids_count = [list(ids).count(token_id) for ids in text_inputs["input_ids"]] | ||
| text_count = [sample.count(token_str) for sample in text] | ||
|
|
||
| if ids_count != text_count: | ||
| raise ValueError( | ||
| f"Mismatch in `image` token count between text and `input_ids`. Got ids={ids_count} and text={text_count}. " | ||
| "Likely due to `truncation='max_length'`. Please disable truncation or increase `max_length`." | ||
|
Comment on lines
+110
to
+126
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you add a small comment on why overriden - different tokens used for placeholder in input text and expanded text
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| ) | ||
|
|
||
| def _get_num_multimodal_tokens(self, image_sizes=None, **kwargs): | ||
| """ | ||
|
|
@@ -165,13 +148,16 @@ def model_input_names(self) -> list[str]: | |
| def unused_input_names(self) -> list[str]: | ||
| return ["num_crops"] | ||
|
|
||
| @property | ||
| def image_token(self) -> list[str]: | ||
| logger.warning_once( | ||
| "Deprecated: `processor.image_token` will switch from returning " | ||
| "`tokenizer.image_token` to `tokenizer.boi_token` in v5.11." | ||
| def replace_image_token(self, image_inputs: dict, image_idx: int, **kwargs) -> str: | ||
| num_crops = to_py_obj(image_inputs["num_crops"])[image_idx] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed, |
||
| if not num_crops: | ||
| return self.full_image_sequence | ||
|
|
||
| # Insert additional image tokens for Pan-and-Scan crops | ||
| return ( | ||
| f"Here is the original image {self.full_image_sequence} and here are some crops to help you see better " | ||
| + " ".join([self.full_image_sequence] * num_crops) | ||
| ) | ||
| return self.tokenizer.image_token | ||
|
|
||
|
|
||
| __all__ = ["Gemma3Processor"] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,23 +13,28 @@ | |
| # limitations under the License. | ||
| """Processor class for Granite Speech.""" | ||
|
|
||
| from typing import Union | ||
|
|
||
| from ...audio_utils import AudioInput | ||
| from ...feature_extraction_utils import BatchFeature | ||
| from ...processing_utils import ProcessorMixin | ||
| from ...processing_utils import ProcessingKwargs, ProcessorMixin, Unpack | ||
| from ...tokenization_python import PreTokenizedInput, TextInput | ||
| from ...utils import auto_docstring, is_torch_available, logging | ||
| from ...utils.import_utils import requires_backends | ||
|
|
||
| from ...utils import auto_docstring | ||
|
|
||
| if is_torch_available(): | ||
| import torch | ||
|
|
||
| logger = logging.get_logger(__name__) | ||
| class GraniteSpeechProcessorKwargs(ProcessingKwargs, total=False): | ||
| _defaults = { | ||
| "text_kwargs": { | ||
| "padding": True, | ||
| }, | ||
| "audio_kwargs": { | ||
| "device": "cpu", | ||
| }, | ||
| } | ||
|
|
||
|
|
||
| @auto_docstring | ||
| class GraniteSpeechProcessor(ProcessorMixin): | ||
| valid_processor_kwargs = GraniteSpeechProcessorKwargs | ||
|
|
||
| def __init__( | ||
| self, | ||
| audio_processor, | ||
|
|
@@ -50,49 +55,13 @@ def __init__( | |
| def __call__( | ||
| self, | ||
| text: TextInput | PreTokenizedInput | list[TextInput] | list[PreTokenizedInput], | ||
| audio: Union["torch.Tensor", list["torch.Tensor"]] = None, | ||
| audio: AudioInput | None = None, | ||
| device: str = "cpu", | ||
| **kwargs, | ||
| **kwargs: Unpack[GraniteSpeechProcessorKwargs], | ||
| ) -> BatchFeature: | ||
| requires_backends(self, ["torch"]) | ||
|
|
||
| text = self._get_validated_text(text) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'd rename this to common
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| prompt_strings = text | ||
|
|
||
| if audio is not None: | ||
| # NOTE - we intentionally avoid throwing for potentially misaligned | ||
| # text / audio inputs here because some inference engines will | ||
| # trigger the conditions due to the way they call multimodal | ||
| # processors, e.g., vLLM. | ||
| audio_inputs = self.audio_processor(audio, device=device) | ||
|
|
||
| # TODO (@alex-jw-brooks); we should add a util to get_num_audio_tokens | ||
| # from feature lengths and call it here, rather than returning it | ||
| # from the feature extractor. | ||
| audio_embed_sizes = audio_inputs.pop("audio_embed_sizes") | ||
|
|
||
| # Expand the audio placeholders to match the feature dims; this | ||
| # is similar to how many VLMs handle image tokens, e.g., llava next | ||
| prompt_strings = [] | ||
| num_replaced = 0 | ||
| for sample in text: | ||
| while self.audio_token in sample: | ||
| sample = sample.replace( | ||
| self.audio_token, | ||
| "<placeholder>" * audio_embed_sizes[num_replaced], | ||
| 1, | ||
| ) | ||
| num_replaced += 1 | ||
| prompt_strings.append(sample) | ||
|
|
||
| prompt_strings = [sample.replace("<placeholder>", self.audio_token) for sample in prompt_strings] | ||
| else: | ||
| audio_inputs = {} | ||
|
|
||
| if "padding" not in kwargs: | ||
| kwargs["padding"] = True | ||
| text_inputs = self.tokenizer(prompt_strings, **kwargs) | ||
| return BatchFeature(data={**text_inputs, **audio_inputs}) | ||
| kwargs.setdefault("audio_kwargs", {}).setdefault("device", device) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not needed, defaults are set anyway when
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kept it for now, you're right the default case is already covered. This is what passes an explicit
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah interesting, then it makes sense, I think Eustache's audio processor PR will bring |
||
| return super().__call__(text=text, audio=audio, **kwargs) | ||
|
|
||
| def _get_validated_text(self, text: str | list) -> list[str]: | ||
| if isinstance(text, str): | ||
|
|
@@ -101,5 +70,23 @@ def _get_validated_text(self, text: str | list) -> list[str]: | |
| return text | ||
| raise TypeError("Invalid text provided! Text should be a string or list of strings.") | ||
|
|
||
| def _process_audio(self, audio: AudioInput, **kwargs): | ||
| # Audio samples are already collated | ||
| if len(audio) == 1 and audio[0].ndim == 2: | ||
| audio = audio[0] | ||
|
Comment on lines
+74
to
+76
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, we didn't have to do that prev, any reason it's required now?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it's because
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahh oke, so iiuc the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Effectively yes you can say that, it never inspects |
||
| audio_inputs = self.audio_processor(audio, device=kwargs["device"]) | ||
|
|
||
| audio_replacements = [self.replace_audio_token(audio_inputs, audio_idx=idx) for idx in range(len(audio))] | ||
| return audio_inputs, audio_replacements | ||
|
|
||
| def replace_audio_token(self, audio_inputs: dict, audio_idx: int, **kwargs) -> str: | ||
| num_audio_tokens = audio_inputs["audio_embed_sizes"][audio_idx] | ||
| return self.audio_token * num_audio_tokens | ||
|
|
||
| @property | ||
| def unused_input_names(self) -> list[str]: | ||
| "Input names returned always by subprocessors but not used in model's `forward`" | ||
| return ["audio_embed_sizes"] | ||
|
|
||
|
|
||
| __all__ = ["GraniteSpeechProcessor"] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we also have to update
image_token_idtoboi_token_idhereUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks for catching this :)
Needed an
image_token_idsoverride with it otherwisetoken_type_idsmarks the boi instead of the 256 soft tokens. Also updated vLLM to read the soft id off the tokenizer like nativegemma3_mm.py