-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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 video batching to videollava #32139
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. |
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 ❤️
Can we activate tests, it is being skipped now for PIL inputs?
@zucchini-nlp one thing confused me, you are passing numpy tests but don't pass |
Yes, I made bad decision when constructing tests tailoring only for torch and numpy. This one has better dummy video preparation, can copy from it maybe transformers/tests/models/llava_next_video/test_image_processing_llava_next_video.py Lines 99 to 124 in 3aefb4e
|
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.
Great, thanks!
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 for fixing!
FWIW - I think the current "numpify" and "torchify" logic is pretty bad and repeated everywhere. We should look into making this clearer and consolidate in the future!
cc @zucchini-nlp works with no problems