VLMs: `patch_size` -> `num_image_tokens` in processing
Author: zucchini-nlp
Pull request: yes

# What does this PR do?

Following https://github.com/huggingface/transformers/issues/33374, we'll use `num_image_tokens` instead of `patch_size` VLM in processors. The reason is because some image backbones add a CLS token while other do not, and we cannot infer the number of tokens needed for an image from number of patches reliably.

Yes, as noted in some prev discussions, we'll have to assume for llava-next style models that the image is a square and obtain patch-width/patch-height from `num_image_tokens`. If I'm not mistaken, the backbones we have in the library and are used by VLMs are all square sized, and since llava-next class if not used with other backbones apart from CLIP afaik, we should be good to assume it that way.

Modified tests are passing locally for me


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_33424). All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@amyeroberts 

Yes, you're right that we are trying to make this work for current existing vision models and there may be some other models that do not have hard requirement for having a square image and thus square patches.

The problem with CLS is that num of image tokens is now computed as `patches_height * patches_width` for all cases and we add `+1` by default to account for the CLS. But not all existing vision models add a cls token.

Since we have all VLMs on CLIP-like vision tower, I think we'll be safe in general either way we choose to implement. But apart from that, my main idea was that if some fancy architecture comes out where we won't be able to infer number of tokens from image/patch size. Then we can safely delegate setting correct number of image tokens for whatever vision model is used to users. I see this option as safer (imo) and more aligned with models like BLIP or Paligemma where we don't do special calculations to get how many image tokens are needed.

What do you think if we leave the patch_size as kwargs only for llava-next given that it is more quirky and requires to know how many patches each image is divided into? For all others we go with `num_image_tokens` to be on the safer side 

> But apart from that, my main idea was that if some fancy architecture comes out where we won't be able to infer number of tokens from image/patch size. Then we can safely delegate setting correct number of image tokens for whatever vision model is used to users. I see this option as safer (imo) and more aligned with models like BLIP or Paligemma where we don't do special calculations to get how many image tokens are needed.

This seems to me like we're engineering for a problem which doesn't yet exist. 

> For all others we go with num_image_tokens to be on the safer side

My main reservation with this is that yes, we can safely delegate the number of tokens, but we can no longer safely calculate patch_size. 

Would it be possible to have both arguments? Where if both are set we verify they're compatible? 

As I'm off I'll delegate this decision to @qubvel. I'll be happy with whatever you both decide! 

Hi @qubvel !

Can you look at the last discussion we had with Amy when you have time :)

I tried to summarize the pros and cons of existing options, feel free to add if I missed anything

### 1. `patch_size` and default `+1` for CLS token

 \+ Support for dynamic image size
 \- Not all architectures have CLS token added, we cannot reliably infer `num_image_tokens` one from `patch_size`
 \- [Potential] There might be an architecture where we can't infer `num_image_tokes` based on `patch_size` and CLS token (examples?)
 
### 2. `num_image_tokens`
\+ Do not depend on CLS token and can support custom `num_image_tokens`
\- Still need `patch_size` for some processors, we have the same problem - we cannot reliably infer one from another
\- From user's point of view it might be not obvious how to get `num_image_tokens` while patch size can be taken directly from the config

So, both of them are not ideal 🥲 Are there any other options? Maybe we can add to configs smth like `num_special_tokens`, wdyt?

@qubvel right, `patch_size` gave us flexibility to work with non-square images but we can't reliably get number of image tokens in some cases. I did a small overview of what types of VLMs are out there and

we can categorize VLMs image backbones into three types: 
- perceiver-like models have a fixes image seq length (Ideifcs, BLIP) so we are good here
- models that use simple ViT architecture and optionally anyres splitting/pixel shuffle. We can be sure abuot image seq length in all cases if position interpolation is not on. AFAIK none of existing VLM papers use interpolation, but they rather try anyres/multiple encoders etc. for high resolutions. We can also use `patch_size` with same level of certainty and might need extra kwargs for specific models (convnext, internViT) to account for donwsampling 
- flexible image size models where the image is resized smartly like in Qwen2-VL. These models resize so that it fits in one of the possible shapes and preserved max of the original image. We can't use image seq length for them, and we should infer number of tokens from each image's size and other params like downsample ratio or merge size


This means we can't use one version of processing for all VLMs to get number of image tokens. And I am still more inclined to `num_image_tokens` because
- we already use `image_seq_length` in other VLMs like Paligemma, Idefics, Chameleon and BLIP instead of trying to get number of tokens using heuristics
- regarding the above concerns with non-square images, it seems like we try to accoun for smth that the model checkpoints don't support. In other words, non-square can be the case for llava only when positions in ViT are interpolated which is not used in any of the models
- we still will have one model that needs special treatment. Qwen2-VL has dynamic seq length for each image depending on its size, so we leave it as is
- yeah, from user perspective seems easier to pass in `patch_size` and hope everything works as magic. We are actually going to add `self._get_image_features` for all VLMs to make it more modular. Maybe users who want to tinker can rely on that method later if they decide to change ViT patch sizes?

At the end we'll do `num_image_tokens` or `image_seq_length` (to be aligned with existing code) for models where pixel shapes are fixed based on crop-size/resize. And for those with dynamic resize, we'll need to also dynamically calculate image seq length . WDYT?

Unfortunately I can't come up with a better solution that will work out of the box for all possible VLMs in all cases, but imo we don't have to. As long as the current approach works with the current model, and is consistent library-wise


Thanks for the very detailed clarification! Indeed it looks like processors' logic can vary significantly across models, the questions here are
1) how to uniformize processors logic (would be easier for users and pipelines integration)
2) make a reliable method to compute image_tokens/patch_size

My suggestion was to use `num_special_tokens` or `num_additinal_tokens`, which could be used instead of `+1`. In that case, we have to provide two arguments `patch_size` and `num_special_tokens` (it could be 0/1/2/... and can be a property of a model config, to be computed dynamically based on enabled cls token for example):
```py
# for square images
num_image_tokens = (image_size / patch_size) ** 2 + num_special_tokens
```
For such a case we might dynamically compute the number of image tokens based on the resulting image size and patch_size to support interpolation. TBH, I'm not sure how it's scaling up to all architectures, it's just an idea to take into consideration 😄 

I'm happy to go with your option too! And maybe just more examples/docs will be enough to make users comfortable!

> how to uniformize processors logic (would be easier for users and pipelines integration)

Unfortunately relying on only one way to ave uniform/same kwargs is impossible at the moment since models have different requirements for vision backbone. Though we can use the `patch_size` + `num_additional_tokens` for VLMs with ViT backbone and `image_seq_length` for others where the length is fixed

Let me then add the `num_additional_tokens`, looks good to me and maybe less breaking (even though we didn't update model checkpoints yet)

Slow tests for expansion are passing all locally

yea, docs will be much needed given we already had some users asking about how to suppress the warning. I don't know if there is a place we can add info about VLMs in more details so I am thinking to add a WARNING section in model doc page for each touched model. Glad there are only ~5 models

Done, added the docs. LMK if it looks good to you and I'll request review from core maintainer. I hope after this PR we can update hub files and remove extra logic by v4.50 (deprecation target will be modified in https://github.com/huggingface/transformers/pull/34245)

Cool, merging soon and I'll update hub configs this week for llava-hf models + open PRs for BLIP models. We've had the deprecation warning for 3-4 releases and should be fine with updating configs