Error when merging image features and text features？
Author: slatter666
Pull request: no
Labels: name: Vision

https://github.com/huggingface/transformers/blob/be9cf070ee2cb6a9f0d162e5be32d9d68b9df3af/src/transformers/models/llava/modeling_llava.py#L336C9-L336C107

```python
# 5. Fill the embeddings corresponding to the images. Anything that is not `text_positions` needs filling (#29835)
image_to_overwrite = torch.full(
    (batch_size, max_embed_dim), True, dtype=torch.bool, device=inputs_embeds.device
)
image_to_overwrite[batch_indices, text_to_overwrite] = False
image_to_overwrite &= image_to_overwrite.cumsum(-1) - 1 >= nb_image_pad[:, None].to(target_device)
```
I think the last line has something wrong in logic, look at the example
```python
the input_ids is as follows, 32000 is the image token index, 0 is the padding index, we suppose num_patches=2
[[32000, 32000, 1, 2, 3],
 [1, 32000, 2, 3, 0]]

then new_token_positions is:
[[1, 3, 4, 5, 6],
 [0, 2, 3, 4, 5]]

nb_image_pad is: [0, 1]

before the last step, image_to_overwrite is:
[[True, True, True, True, False, False, False],
 [False, True, True, False, False, False, True]]

after the last step, image_to_overwrite is:
[[True, True, True, True, False, False, False],
 [False, False, True, False, False, False, True]]

however, the right result should be: 
[[True, True, True, True, False, False, False],
 [False, True, True, False, False, False, False]]
```

I think the code is only for left padding , if we use right padding, there should be some modifications, and here is my code:
```
image_to_overwrite = torch.full(
    (batch_size, max_embed_dim), True, dtype=torch.bool, device=inputs_embeds.device
)
image_to_overwrite[batch_indices, text_to_overwrite] = False
if left_padding:
    image_to_overwrite &= image_to_overwrite.cumsum(-1) - 1 >= nb_image_pad[:, None].to(target_device)
else:
    image_to_overwrite &= torch.ones_like(image_to_overwrite, dtype=torch.bool).cumsum(-1) - 1 <= new_positions[:, -1:].to(target_device)
```


Comments:
cc @ArthurZucker @zucchini-nlp 

I believe you're under the `legacy_processing` flag, this is not recommended: https://github.com/huggingface/transformers/blob/be9cf070ee2cb6a9f0d162e5be32d9d68b9df3af/src/transformers/models/llava/modeling_llava.py#L464

Well I check the version of transformers I'm using, I use transformers==4.43.4, and I I notice that this code is not in this version. I find the logic in latest version is right, however I still doubt the correctness in old version

Hey @slatter666 !

Yes, we have been working on getting rid of this huuuge merging logic in favor of simple `masked_scatter`. The code is up on the latest v4.45, but we are in process if updating model checkpoints on the hub still. And we plan to support old-style checkpoints for a while, thus the older merging logic.

So, regarding your issue, we try to account for left-right padding here by changing new positions. Have you noticed any convergence issues in training or cuda-indexing errors? I'll verify once more to be sure, but afair for other llava models (with similar merging) this logic is tested and we havening changed anything lately. LMK if you have any short examples I can run to reproduce the bug :)

https://github.com/huggingface/transformers/blob/be9cf070ee2cb6a9f0d162e5be32d9d68b9df3af/src/transformers/models/llava/modeling_llava.py#L299-L300

Well I don't run the training process, it's just that I want to slightly modify some logic of attention mask for my own design, then I check the logic of llava code and I find the code is only for left padding

Can i get a small reproducer if possible, that would help us a lot to debug the code