Fix: Guard against None num_query_tokens in Blip2Processor (to avoid TypeError)#42311
Fix: Guard against None num_query_tokens in Blip2Processor (to avoid TypeError)#42311Flakes342 wants to merge 13 commits intohuggingface:mainfrom
Conversation
|
cc @zucchini-nlp! I think it's fine without the warning message, but deferring to her on what behaviour we want from processors 😅 |
There was a problem hiding this comment.
@Flakes342 thanks for the PR!
To support older models from the hub which weren't updated, I think we should update the default value of num_query_tokens.
The modeling code assumes that input_ids have special image placeholders and with self.num_query_tokens=None the placeholders will not be added in the input text. Therefore the model will not see an input image
Let's add the default value of 32 which is the only value used in all official checkpoints. Ping me for another review when ready :)
|
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. |
|
@zucchini-nlp thr pr is ready for your review |
|
run-slow: blip_2 |
|
This comment contains models: ["models/blip_2"] |
CI ResultsModel CI Report❌ Failed tests
|
|
run-slow: blip_2 |
1 similar comment
|
run-slow: blip_2 |
|
Can you also fix the modeling code to be in line with the changes? In some places we check if the model config was updated or not with It can be later tested with Finally, you need to run |
|
[For maintainers] Suggested jobs to run (before merge) run-slow: blip_2 |
|
@bot /style |
|
Style fix is beginning .... View the workflow run here. |
What does this PR do?
Fixes #42203
Before submitting
Pull Request section?
to it if that's the case.
documentation guidelines, and
here are tips on formatting docstrings.
Fixed a bug where Blip2Processor assumes num_query_tokens is an int and does
max_length - self.num_query_tokenswhich raises TypeError whennum_query_tokensis None.This change:
Who can review?
@Rocketknight1