Skip to content
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.

Sign up for GitHub

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

[Misc] Update Default Image Mapper Error Log #8977

Merged
merged 3 commits into from
Oct 2, 2024
Merged

[Misc] Update Default Image Mapper Error Log #8977

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3dec1a0
Update default mapper error log
alex-jw-brooks Sep 30, 2024
42d844e
Update vllm/multimodal/image.py
alex-jw-brooks Oct 1, 2024
79aacfa
Add class name of image processor to error log
alex-jw-brooks Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion vllm/multimodal/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ def _default_input_mapper(
.preprocess(data, return_tensors="pt") \
.data
except Exception:
logger.error("Failed to process image (%s)", data)
logger.error(
"Failed to process image (%s) with the default mapper. "
"This is most likely an edge-case with this model's image "
"processor in transformers, and not VLLM.", data)
alex-jw-brooks marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

@ywang96 ywang96 Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I wonder if showing data here is actually helpful at all.

Perhaps we can also print out the class of image_processor here? WDYT?

Copy link
Contributor Author

@alex-jw-brooks alex-jw-brooks Oct 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think printing the class name of the image_processor is a good idea! IMO there is a some value in printing the data since it does include stuff like the image colorspace & image size in it. This helps make cases like the 1x1 pixel issue for minicpmv more obvious, but it also will give us the shape to reproduce if there are bugs in complex image preprocessing logic that are entirely dependent on shape/aspect ratio, like in llava-next etc. It also gives us the colorspace, which is useful if there's weird stuff going on due to images being the wrong colorspace / dtype etc.

I updated it to include the name of the image processor class - Here is an example log for processing a 1x1 with llava next (which uses the default mapper and blows up ) as a result.

Failed to process image (<PIL.Image.Image image mode=RGB size=1x1 at 0x14FC56493E50>) with the default mapper. This is most likely an edge-case with this model's image processor in transformers (type: LlavaNextImageProcessor), and not vLLM.

raise

return MultiModalInputs(batch_data)
Expand Down
Loading