Fix Llava-NeXT / Llava-NeXT Video / Llava-OneVision's token unpadding mismatch
Author: sheryc
Pull request: yes

# What does this PR do?

<!--
Congratulations! You've made it this far! You're not quite done yet though.

Once merged, your PR is going to appear in the release notes with the title you set, so make sure it's a great title that fully reflects the extent of your awesome contribution.

Then, please replace this with a description of the change and which issue is fixed (if applicable). Please also include relevant motivation and context. List any dependencies (if any) that are required for this change.

Once you're done, someone will review your PR shortly (see the section "Who can review?" below to tag some potential reviewers). They may suggest changes to make the code even better. If no one reviewed your PR after a week has passed, don't hesitate to post a new comment @-mentioning the same persons---sometimes notifications get lost.
-->

Fixes https://github.com/huggingface/transformers/issues/35775 and the further discussions in https://github.com/huggingface/transformers/issues/34625.

This PR fixes the vision feature / vision token mismatch issue for LLaVA OneVision models. Specifically, the unpadding token calculation for the Processor and the model differs by a rounding funciton, where precision issue may happen to create a dimension mismatch. The PR adds the rounding function to the Processor to match the behavior with LlavaOnevision's and LlavaNext's model behaviors.

<!-- Remove if not applicable -->


## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case). (N/A)
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md#create-a-pull-request),
      Pull Request section?
- [x] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to it if that's the case. (https://github.com/huggingface/transformers/issues/35775)
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/main/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/main/docs#writing-source-documentation). (No new functions introduced.)
- [ ] Did you write any new necessary tests? (No new tests needed.)


## Who can review?

@zucchini-nlp

<!-- Your PR will be replied to more quickly if you can figure out the right person to tag with @

 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.

Models:

- text models: @ArthurZucker
- vision models: @amyeroberts, @qubvel
- speech models: @ylacombe, @eustlb
- graph models: @clefourrier

Library:

- flax: @sanchit-gandhi
- generate: @zucchini-nlp (visual-language models) or @gante (all others)
- pipelines: @Rocketknight1
- tensorflow: @gante and @Rocketknight1
- tokenizers: @ArthurZucker
- trainer: @muellerzr and @SunMarc
- chat templates: @Rocketknight1

Integrations:

- deepspeed: HF Trainer/Accelerate: @muellerzr
- ray/raytune: @richardliaw, @amogkam
- Big Model Inference: @SunMarc
- quantization (bitsandbytes, autogpt): @SunMarc @MekkCyber

Documentation: @stevhliu

HF projects:

- accelerate: [different repo](https://github.com/huggingface/accelerate)
- datasets: [different repo](https://github.com/huggingface/datasets)
- diffusers: [different repo](https://github.com/huggingface/diffusers)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Maintained examples (not research project or legacy):

- Flax: @sanchit-gandhi
- PyTorch: See Models above and tag the person corresponding to the modality of the example.
- TensorFlow: @Rocketknight1

 -->

Edit: updated the title to reflect the latest changes in this PR.

Comments:
@zucchini-nlp  I got the same issue: ValueError: Image features and image tokens do not match: tokens: 4589, features 4588 when using Llava-v1.6-vicuna-7b-hf (llava-next model) and the transformers version is 4.47.0. I followed PR: https://github.com/huggingface/transformers/pull/35779 and modified processing_llava_next.py shown in the following code. But it doesn't work. Is there anything difference between llava-next and llava-onevision? I thought they might be the same issue.

original_aspect_ratio = width / height
current_aspect_ratio = current_width / current_height
if original_aspect_ratio > current_aspect_ratio:
new_height = int(round(height * current_width / width, 7))
padding = (current_height - new_height) // 2
current_height -= padding * 2
else:
new_width = int(round(width * current_height / height, 7))
padding = (current_width - new_width) // 2
current_width -= padding * 2

> Great, thanks a lot for fixing!
> 
> Unpadding is having too many precision related bugs 🥲 Can you propagate the changes to llava-next and llava-next-video which use the same pad-unpad method? And then add `# Copied from transformers.models.llava_next.processing_llava_next._get_unpadded_features ` in other models, so it always stays the same.Since we already have the model-side copied and identical

`processing_llava_onevision` has some added processing regarding the ratio for interpolation, so after I add this "Copied from" comment, the makefile prompts me to fix copies. Should I ignore it or change that comment for `processing_llava_onevision`? @zucchini-nlp 

> @zucchini-nlp I got the same issue: ValueError: Image features and image tokens do not match: tokens: 4589, features 4588 when using Llava-v1.6-vicuna-7b-hf (llava-next model) and the transformers version is 4.47.0. I followed PR: #35779 and modified processing_llava_next.py shown in the following code. But it doesn't work. Is there anything difference between llava-next and llava-onevision? I thought they might be the same issue.
> 
> original_aspect_ratio = width / height current_aspect_ratio = current_width / current_height if original_aspect_ratio > current_aspect_ratio: new_height = int(round(height * current_width / width, 7)) padding = (current_height - new_height) // 2 current_height -= padding * 2 else: new_width = int(round(width * current_height / height, 7)) padding = (current_width - new_width) // 2 current_width -= padding * 2

There has been some other fixes (e.g. https://github.com/huggingface/transformers/pull/34779 for https://github.com/huggingface/transformers/issues/34625) for this problem integrated in `transformers` v4.47.1. Could you try upgrading first? If after the upgrade and the new code change the issue persists, could you please provide a code snippet to reproduce the issue? @Happy-Corpse 

> processing_llava_onevision has some added processing regarding the ratio for interpolation,

Right, it cannot be 100% copied from llava-next. Oke, you can then leave "Adapted from ..." so that anyone reading the code can refer to llava-next as the source of truth

Is there anything else I need to do? @zucchini-nlp 

Sorry, yeah let's merge it. I don't think we need one more review for this

Quick question, why do we have a 7 hardcoded, a comment would be nice to know why it's seven and not 12983

Yeah hardcoded, comes from an old issue afair when the rounding happened to the wrong decimal, and we needed more precision. So 7 was aribtrary large number to keep precision. Same rounding is present in modeling code, and the idea is to keep model-processor code identical to prevent precision errors


Hey @sheryc @zucchini-nlp I'm still getting this error even after installing the lastest version (4.49.0.dev0) by running `pip install git+https://github.com/huggingface/transformers.git`. There error is the following:

`ValueError: Image features and image tokens do not match: tokens: 6552, features 6556`

The mismatch between number of features and tokens is always very small, what makes me think that this is still related to these rounding issues somehow.

I'm trying to finetune either `unsloth/llava-1.5-7b-hf-bnb-4bit` or `unsloth/llava-v1.6-mistral-7b-hf-bnb-4bit` through Unsloth, to perform binary classification based on a list of 2 images.

Not sure if this helps, but here is some _pseudo-code_ that describes the prompt/image formatting and tokenization process that I'm performing. Images are PIL objects.
```python
messages = [
  {
    "role": "user",
    "content": [
      {"index": 0, "text": None, "type": "image"},
      {"index": 1, "text": None, "type": "image"},
      {"index": None, "text": instruction, "type": "text"},
    ]
  },
  {
    "role": "assistant",
    "content": [
      {"index": None, "text": label, "type": "text"},
    ]
  }
]

prompt = self.processor.apply_chat_template(
    messages,
    tokenize = False,
    add_generation_prompt = False,
)

inputs = self.processor(
    text = prompt,
    images  = [image1, image2],
    padding = True,
    return_tensors = "pt",
)
```

Hi @baraujo98, the models you used are Llava models. This PR only fixes the problem for Llava-NeXT, Llava-NeXT Video, and Llava-OneVision, whose processing is a bit different from Llava. I think it's worth a new issue? Also could you add a minimal code snippet to reproduce the error?

@sheryc I believe that `unsloth/llava-v1.6-mistral-7b-hf-bnb-4bit` is a LLaVa-NeXT model and that it uses the `transformers`' LLaVa-NeXT scripts.

I will try to write a minimal example nonetheless.



> @sheryc I believe that `unsloth/llava-v1.6-mistral-7b-hf-bnb-4bit` is a LLaVa-NeXT model and that it uses the `transformers`' LLaVa-NeXT scripts.
> 
> I will try to write a minimal example nonetheless.

Yes you're right, sorry

@baraujo98 right, I see the config on the hub wasn't updated with the `additional_tokens`. I opened a PR here (https://huggingface.co/unsloth/llava-1.5-7b-hf-bnb-4bit/discussions/1). Can you check if adding the attribute solves the issue? 

Thank you very much for the tip @zucchini-nlp. Unsloth doesn't allow me to load revisions of the model for some reason, but I was able to fix the issue for now by running `tokenizer.num_additional_image_tokens = 1` after loading the pretrained model. It is now able to train without throwing that error.

I opend a similar PR for the second model I mentioned: https://huggingface.co/unsloth/llava-v1.6-mistral-7b-hf-bnb-4bit/discussions/1

I am using Qwen2-VL-2b instruct and on inference I am getting this - 

---------------------------------------------------------------------------


ValueError: Image features and image tokens do not match: tokens: 841, features 1682. while you guys dont have a big gap in token vs features I have the difference by a factor of 2. I am using 800*800 resolution images and passing them in an array to the model

@ambar3497 Hi Ambar, this PR only solves the problem for llava-next/onevision models, you may consider opening a new issue and provide a code snippet to reproduce the error