Repository navigation
🚨 Image-text pipeline expects correctly formatted chat - #42359
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
cc @ebezzam as discussed internally. I didn't move the |
| messages = [ | ||
| { | ||
| "role": "user", | ||
| "content": [ | ||
| {"type": "text", "text": "What’s the difference between these two images?"}, | ||
| {"type": "image", "url": image_ny}, | ||
| {"type": "image", "url": image_chicago}, | ||
| ], | ||
| } |
There was a problem hiding this comment.
much better. It's already documented but shall we advertise also "path" here in addition to "url"? Just for completeness.
There was a problem hiding this comment.
The chat template are tested with local path and url in processing tests. Since the pipeline internally only calls apply_chat_template, I think it isn't necessary
yonigozlan
left a comment
yonigozlan
left a comment
There was a problem hiding this comment.
Nice to see this cleaned up! I don't see anything wrong with removing that for v5 if everything is now correctly handled inside chat templates.
| if images is not None: | ||
| raise ValueError( | ||
| "Invalid input: you passed `chat` and `images` as separate input arguments. " | ||
| "Images must be placed inside the chat message's `content`. For example, " | ||
| "'content': [" | ||
| " {'type': 'image', 'url': 'image_url'}, {'type': 'text', 'text': 'Describe the image.'}}" | ||
| "]" | ||
| ) |
There was a problem hiding this comment.
Should we mention that this is a deprecated behavior in >v5? Otherwise users might not understand why their previously working code is now crashing
There was a problem hiding this comment.
Right, I merged assuming I've read all comments. Sorry. Yeah, saying it's broken for v5 might be more clear, ig users will realize v5 broke a lot of stuff 🙃
|
Big fan of this cleanup! |
ebezzam
left a comment
ebezzam
left a comment
There was a problem hiding this comment.
@zucchini-nlp I think there's still a fix needed to handle chat templates with URL?
| self.messages = messages | ||
|
|
||
|
|
||
| def add_images_to_messages(messages: dict, images: Union[str, list[str], "Image.Image", list["Image.Image"]] | None): |
There was a problem hiding this comment.
@zucchini-nlp I don't think the standardization is as simple as removing this function causes unfortunately :/
As this test to fails because add_images_to_messages was actually modifying the chat template when there were URLs
# RUN_SLOW=1 pytest tests/pipelines/test_pipelines_image_text_to_text.py::ImageTextToTextPipelineTests::test_model_pt_chat_template_image_url
@slow
@require_torch
def test_model_pt_chat_template_image_url(self):
pipe = pipeline("image-text-to-text", model="llava-hf/llava-interleave-qwen-0.5b-hf")
messages = [
{
"role": "user",
"content": [
{
"type": "image_url",
"image_url": {
"url": "https://cdn.britannica.com/61/93061-050-99147DCE/Statue-of-Liberty-Island-New-York-Bay.jpg"
},
},
{"type": "text", "text": "Describe this image in one sentence."},
],
}
]
outputs = pipe(text=messages, return_full_text=False, max_new_tokens=10)[0]["generated_text"]
> self.assertEqual(outputs, "A statue of liberty in the foreground of a city")
E AssertionError: "A close-up of a person's face with a" != 'A statue of liberty in the foreground of a city'
E - A close-up of a person's face with a
E + A statue of liberty in the foreground of a city
tests/pipelines/test_pipelines_image_text_to_text.py:380: AssertionError
| "The number of images in the chat messages should be the same as the number of images passed to the pipeline." | ||
| ) | ||
| # Add support for OpenAI/TGI chat format | ||
| elif content_type == "image_url": |
There was a problem hiding this comment.
I think this was the part rewrittting
There was a problem hiding this comment.
fixing it to support OpenAI format in pipelines
…2359) * don't support images as separate arg * adjust the test case
6 participants
What does this PR do?
As per title, we can break for v5 and enforce a correct usage of chat templates. I will request review after adjusting tests