-
Notifications
You must be signed in to change notification settings - Fork 34.3k
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 all 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 | ||
|
|
||
|
|
||
| class Gemma3ProcessorKwargs(ProcessingKwargs, total=False): | ||
|
|
@@ -53,8 +49,9 @@ def __init__( | |
| **kwargs, | ||
| ): | ||
| self.image_seq_length = image_seq_length | ||
| self.image_token_id = tokenizer.image_token_id | ||
| self.image_token_id = tokenizer.boi_token_id | ||
| self.boi_token = tokenizer.boi_token | ||
| self.image_token = tokenizer.boi_token | ||
| 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,62 @@ 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`.") | ||
|
|
||
| output_kwargs = self._merge_kwargs( | ||
| Gemma3ProcessorKwargs, | ||
| tokenizer_init_kwargs=self.tokenizer.init_kwargs, | ||
| **kwargs, | ||
| ) | ||
| if text is not None and not isinstance(text, str): | ||
| if not isinstance(text, list) or not isinstance(text[0], str): | ||
| raise TypeError("Invalid input text. Please provide a string, or a list of strings") | ||
|
Comment on lines
+72
to
+74
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 not in validate_input?
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. 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. ah oke, tbh this could be a general check in Mixin/ Not on you ofc, let's merge |
||
|
|
||
| if isinstance(text, str): | ||
| text = [text] | ||
| elif not isinstance(text, list) or not isinstance(text[0], str): | ||
| raise TypeError("Invalid input text. Please provide a string, or a list of strings") | ||
| 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 | ||
|
|
||
| image_inputs = {} | ||
| def prepare_inputs_layout(self, images=None, text=None, **kwargs): | ||
| images, text, *_ = super().prepare_inputs_layout(images=images, text=text, **kwargs) | ||
| if images is not None: | ||
| images = self.image_processor.fetch_images(images) | ||
| batched_images = make_nested_list_of_images(images) | ||
| image_inputs = self.image_processor(images, **output_kwargs["images_kwargs"]) | ||
|
|
||
| images = make_nested_list_of_images(images) | ||
| # Create empty text to be replaced with placeholders | ||
| if not text: | ||
| text = [" ".join([self.boi_token] * len(images)) for images in batched_images] | ||
| text = [" ".join([self.boi_token] * len(image_list)) for image_list in images] | ||
| return images, text, None, None | ||
|
|
||
| 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) | ||
|
|
||
| if len(batched_images) != len(text): | ||
| if images is not None and text is not None: | ||
| if len(images) != len(text): | ||
| raise ValueError( | ||
| f"Received inconsistently sized batches of images ({len(batched_images)}) and text ({len(text)})." | ||
| f"Received inconsistently sized batches of images ({len(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, 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"]) | ||
| 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. | ||
|
|
||
| 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) | ||
| Gemma3 uses a different token as placeholder in input text than in the expanded text. | ||
| """ | ||
| 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): | ||
| """ | ||
|
|
@@ -157,6 +149,10 @@ def _get_num_multimodal_tokens(self, image_sizes=None, **kwargs): | |
|
|
||
| return MultiModalData(**vision_data) | ||
|
|
||
| @property | ||
| def image_token_ids(self) -> list[int]: | ||
| return [self.tokenizer.image_token_id] | ||
|
|
||
| @property | ||
| def model_input_names(self) -> list[str]: | ||
| return super().model_input_names + ["token_type_ids"] | ||
|
|
@@ -165,13 +161,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 = image_inputs["num_crops"][image_idx] | ||
| 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,56 +55,38 @@ 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) | ||
| 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) | ||
| text = self._validate_inputs(text) | ||
| 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) | ||
|
|
||
| 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}) | ||
|
|
||
| def _get_validated_text(self, text: str | list) -> list[str]: | ||
| def _validate_inputs(self, text: str | list) -> list[str]: | ||
| if isinstance(text, str): | ||
| return [text] | ||
| elif isinstance(text, list) and isinstance(text[0], 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.
Why are we swapping
boi_tokenintoimage_token_id?This is a breaking change for downstream libraries and it doesn't appear to be used anywhere else in this diff.
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.
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/video tokeninstead of smth else