-
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
Paligemma - fix slow tests, add bf16 and f16 slow tests #30851
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 @molbap . Some nits.
Let's try the run-slow
feature to make sure everything is passing on CI runner.
OOO but the models are gated @ydshieh , the CI bot needs access for testing, not sure how to do it! (the run-slow works apart from that 💯 ) |
@molbap for that you can use
|
cc @ydshieh should be good to go! |
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.
Thank you @molbap ! LGTM.
I am not sure I understand the to replace what is used
part in this comment
Let' s make sure we test the preprocessing to replace what is used
(what is what
and what is used
here).
I mean it might be good to be more precise - but maybe I lack some context
Apparently it comes from Llava code - I have no idea what this comment means! maybe @younesbelkada ? |
In this case, we can merge, and once @younesbelkada has answered, we can update the comments in another PR (if needed) |
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 !
About your question, honestly I don't know, I think we can safely ignore it
If that comment is that confusing, let's just remove it in another PR (both here and in LLava testing file). WDTY? |
* fix slow tests, add bf16 and f16 slow tests * few fixes * [run-slow]paligemma * add gate decorator * [run-slow]paligemma * add missing gating * [run-slow]paligemma * [run-slow]paligemma
What does this PR do?
As per PR title.
cc @ydshieh