-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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
LLaVA OV: fix unpadding precision #34779
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 for the fix! Just a question re types
src/transformers/models/llava_onevision/processing_llava_onevision.py
Outdated
Show resolved
Hide resolved
sorry @qubvel , wrong tag 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.
Thanks, does it fix #34625 entirely? (let's close it if so!)
Hey @zucchini-nlp,
Error: Now, I looked at the commits of this PR and it looks good to me. The issue might be with your last PR #33424. Kindly look into this. |
@Scyther-07 hey, which transformers version you are using? |
It's 4.39.3. |
@Scyther-07 hmm, the 4.39.3 should throw error indeed and you need at least v4.43 to bypass the error. In fact we are currently changing the way inputs for VLMs are processed, thus I'd recommend to use the latest transformers after release. It will be v4.47 around next 1-2 weeks, not released yet now :) |
Yeah, it worked. Too foolish of me. Thanks for the help! |
* fix * propagate * type check
* fix * propagate * type check
What does this PR do?
Fixes #34625. There was a small precision error in unpadding because the modeling code casts the size to list, while processing code works with tensors. This PR casts everything to list to match the calculations