-
Notifications
You must be signed in to change notification settings - Fork 27.4k
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
fix pixtral processor #34486
fix pixtral processor #34486
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. |
Tests (nonslow, just launched the slow ones) are green - there was a mismatch in the tests between images per batch and images per sample which I think I fiixed, added a test as well for previous issue. cc @ArthurZucker for review |
all green incl. slow now - fixed slow tests that were broken before (well one fix for a wrong config key at init, one that actually can't work, and a skip for torchscript) lmk! |
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.
Thanks, LGTM let's not revert integration test
|
||
|
||
@require_torch | ||
class PixtralVisionModelIntegrationTest(unittest.TestCase): |
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 did this one have to go away ? 😓
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.
many reasons, it was looking up an image url that did not exist (pixtral-vl instead of llava-vl) to predict something that pixtral model does not predict, also it was testing VisionModel
that does not support generate
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.
Yep it needs to be in the Llava
tests! Can you move it around 👀
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.
sure!
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.
transformers/tests/models/llava/test_modeling_llava.py
Lines 620 to 663 in 913330c
def test_pixtral(self): | |
model_id = "hf-internal-testing/pixtral-12b" | |
model = LlavaForConditionalGeneration.from_pretrained(model_id) | |
processor = AutoProcessor.from_pretrained(model_id) | |
IMG_URLS = [ | |
Image.open(requests.get("https://picsum.photos/id/237/400/300", stream=True).raw), | |
Image.open(requests.get("https://picsum.photos/id/231/200/300", stream=True).raw), | |
Image.open(requests.get("https://picsum.photos/id/27/500/500", stream=True).raw), | |
Image.open(requests.get("https://picsum.photos/id/17/150/600", stream=True).raw), | |
] | |
PROMPT = "<s>[INST]Describe the images.\n[IMG][IMG][IMG][IMG][/INST]" | |
# image = Image.open(requests.get(url, stream=True).raw) | |
inputs = processor(text=PROMPT, images=IMG_URLS, return_tensors="pt").to("cuda") | |
generate_ids = model.generate(**inputs, max_new_tokens=500) | |
ouptut = processor.batch_decode(generate_ids, skip_special_tokens=True, clean_up_tokenization_spaces=False)[0] | |
# fmt: off | |
EXPECTED_GENERATION = """ | |
Describe the images. | |
Sure, let's break down each image description: | |
1. **Image 1:** | |
- **Description:** A black dog with a glossy coat is sitting on a wooden floor. The dog has a focused expression and is looking directly at the camera. | |
- **Details:** The wooden floor has a rustic appearance with visible wood grain patterns. The dog's eyes are a striking color, possibly brown or amber, which contrasts with its black fur. | |
2. **Image 2:** | |
- **Description:** A scenic view of a mountainous landscape with a winding road cutting through it. The road is surrounded by lush green vegetation and leads to a distant valley. | |
- **Details:** The mountains are rugged with steep slopes, and the sky is clear, indicating good weather. The winding road adds a sense of depth and perspective to the image. | |
3. **Image 3:** | |
- **Description:** A beach scene with waves crashing against the shore. There are several people in the water and on the beach, enjoying the waves and the sunset. | |
- **Details:** The waves are powerful, creating a dynamic and lively atmosphere. The sky is painted with hues of orange and pink from the setting sun, adding a warm glow to the scene. | |
4. **Image 4:** | |
- **Description:** A garden path leading to a large tree with a bench underneath it. The path is bordered by well-maintained grass and flowers. | |
- **Details:** The path is made of small stones or gravel, and the tree provides a shaded area with the bench invitingly placed beneath it. The surrounding area is lush and green, suggesting a well-kept garden. | |
Each image captures a different scene, from a close-up of a dog to expansive natural landscapes, showcasing various elements of nature and human interaction with it. | |
""" | |
# fmt: on | |
# check that both inputs are handled correctly and generate the same output | |
self.assertListEqual(ouptut, EXPECTED_GENERATION) |
looks like pixtral is already tested under llava tests properly - since it's a llava model it's enough to test it there, no?
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.
ah then good job and sorry!
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.
no worries- it's hard to see when tests are measuring a model through another one, but it's convenient on our side. I can move all pixtral related tests to pixtral testing so we don't forget it, wdyt? It's a bit more loc but it's easier to track down
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.
Good to go ! Thanks 🤗
* fix pixtral processor * test out full length batches + remove undue ValueError * fix up processing * fix tests * fix * last fixup * style * [run-slow] pixtral * [run-slow] pixtral * fix config key * skip torchscript tests * [run-slow] pixtral * add missing key * [run-slow] pixtral * fix docs * [run-slow] pixtral * fix wrong url for integration test * [run-slow] pixtral * pixtralVisionModel does not have a lm head * [run-slow] pixtral
* fix pixtral processor * test out full length batches + remove undue ValueError * fix up processing * fix tests * fix * last fixup * style * [run-slow] pixtral * [run-slow] pixtral * fix config key * skip torchscript tests * [run-slow] pixtral * add missing key * [run-slow] pixtral * fix docs * [run-slow] pixtral * fix wrong url for integration test * [run-slow] pixtral * pixtralVisionModel does not have a lm head * [run-slow] pixtral
* fix pixtral processor * test out full length batches + remove undue ValueError * fix up processing * fix tests * fix * last fixup * style * [run-slow] pixtral * [run-slow] pixtral * fix config key * skip torchscript tests * [run-slow] pixtral * add missing key * [run-slow] pixtral * fix docs * [run-slow] pixtral * fix wrong url for integration test * [run-slow] pixtral * pixtralVisionModel does not have a lm head * [run-slow] pixtral
* fix pixtral processor * test out full length batches + remove undue ValueError * fix up processing * fix tests * fix * last fixup * style * [run-slow] pixtral * [run-slow] pixtral * fix config key * skip torchscript tests * [run-slow] pixtral * add missing key * [run-slow] pixtral * fix docs * [run-slow] pixtral * fix wrong url for integration test * [run-slow] pixtral * pixtralVisionModel does not have a lm head * [run-slow] pixtral
What does this PR do?
Should fix #34204 .