-
Notifications
You must be signed in to change notification settings - Fork 523
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
Make model_class
required arg, default processor_class
to AutoProcessor
#1077
Make model_class
required arg, default processor_class
to AutoProcessor
#1077
Conversation
Thank you! Would you mind running |
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.
Could you please verify that pytest -s tests/generate/test_generate.py -k "model_transformers_vision"
works on a GPU machine?
Thanks so much for taking care of this, we need people who make little fixes that have a big impact!
@lapp0 Yes that's a good catch! I edited Also, to have a CPU-based test that runs from GitHub actions, I added (thinking out loud) since |
This models tests are disabled unless GPU is available simply because they're too big for CI, not due to a lack of CPU-run capabilities. You can run them on CPU if you remove
If you simply add the models fixture to
I think we should move away from |
Multi-image context, using `apply_chat_template`
Got it!
Since In writing the test cases, I realized I got into a flow of duplicate instantiations of my conversation = [
{
"role": "user",
"content": [{"type": "text", "text": "What is this?"}, {"type": "image"}],
},
]
generator = outlines.generate.text(model)
sequence = generator(
conversation,
[image],
apply_chat_template=True,
) Now, given |
I don't see any problem with additional testing at all. However you can also test
I think we should leave this change out, as there are already two PRs working on this: |
…torAdapter" This reverts commit 6200d40.
My bad for missing those 2 existing PRs! Reverted that change, will stop now to keep this small PR actually small 😊 I am seeing the below error pop up a bunch when I run images = [[<PIL.Image.Image image mode=RGB size=210x140 at 0x7FAE23D7B670>], [<PIL.Image.Image image mode=RGB size=210x140 at 0...age image mode=RGB size=210x140 at 0x7FAE23D7B670>], [<PIL.Image.Image image mode=RGB size=210x140 at 0x7FAE23D7B670>]]
expected_ndims = 3
def make_list_of_images(images, expected_ndims: int = 3) -> List[ImageInput]:
"""
Ensure that the input is a list of images. If the input is a single image, it is converted to a list of length 1.
If the input is a batch of images, it is converted to a list of images.
Args:
images (`ImageInput`):
Image of images to turn into a list of images.
expected_ndims (`int`, *optional*, defaults to 3):
Expected number of dimensions for a single input image. If the input image has a different number of
dimensions, an error is raised.
"""
if is_batched(images):
return images
# Either the input is a single image, in which case we create a list of length 1
if isinstance(images, PIL.Image.Image):
# PIL images are never batched
return [images]
if is_valid_image(images):
if images.ndim == expected_ndims + 1:
# Batch of images
images = list(images)
elif images.ndim == expected_ndims:
# Single image
images = [images]
else:
raise ValueError(
f"Invalid image shape. Expected either {expected_ndims + 1} or {expected_ndims} dimensions, but got"
f" {images.ndim} dimensions."
)
return images
> raise ValueError(
"Invalid image type. Expected either PIL.Image.Image, numpy.ndarray, torch.Tensor, tf.Tensor or "
f"jax.ndarray, but got {type(images)}."
)
E ValueError: Invalid image type. Expected either PIL.Image.Image, numpy.ndarray, torch.Tensor, tf.Tensor or jax.ndarray, but got <class 'list'>.
../../../opt/miniconda3/envs/outlines-dev/lib/python3.10/site-packages/transformers/image_utils.py:205: ValueError |
Thanks! No need to address in this PR if the errors exist on Also please let me know when this PR is ready for review. |
@lapp0 ready for review! |
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.
Sorry for the delayed review. Thanks for the changes. Looks good to me!
Thank you for contributing! |
Closes #1076