Fix some processors - #47608
Conversation
| self.image_token = tokenizer.image_token if hasattr(tokenizer, "image_token") else image_token | ||
| self.image_token_id = ( | ||
| tokenizer.image_token_id | ||
| if getattr(tokenizer, "image_token_id", None) | ||
| else tokenizer.convert_tokens_to_ids(self.image_token) | ||
| ) | ||
| self.image_token = "<image>" | ||
| self.image_token_id = tokenizer.convert_tokens_to_ids(self.image_token) |
There was a problem hiding this comment.
model has saved <IMG_ATOM> as image token, i.e. the token used by model. We should actually have the same image token in processing and in modeling, but I missed this in ovis when releasing
This not perfect hardcoded value allows us to actually expand image placeholders
There was a problem hiding this comment.
Looks like it breaks fast ci?
|
run-slow: florence2, ovis2 |
|
This comment contains models: ["models/florence2", "models/ovis2"] |
CI recapDashboard: View test results in Grafana |
|
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. |
|
These two tests were failing already before I broke processing, so I didn't fix them (out of scope for this PR) |
vasqu
left a comment
There was a problem hiding this comment.
Thanks let's fix fast CI please 🤗
| self.image_token = tokenizer.image_token if hasattr(tokenizer, "image_token") else image_token | ||
| self.image_token_id = ( | ||
| tokenizer.image_token_id | ||
| if getattr(tokenizer, "image_token_id", None) | ||
| else tokenizer.convert_tokens_to_ids(self.image_token) | ||
| ) | ||
| self.image_token = "<image>" | ||
| self.image_token_id = tokenizer.convert_tokens_to_ids(self.image_token) |
There was a problem hiding this comment.
Looks like it breaks fast ci?
oh no, this is why we need to be stricter with token id values. Lemme see if it's breaking, maybe I will just override |
|
[For maintainers] Suggested jobs to run (before merge) run-slow: florence2, ovis2 |
* fix * oke, this was a mismtach between values in testing and processor files
* fix * oke, this was a mismtach between values in testing and processor files
What does this PR do?
Daily CI complained after I merged VLM processors' update, fixing it