Regarding pix2struct's bugs in the attentions of the encoder's and decoder's outputs
Author: loveisp
Pull request: yes

This change addresses the following bug: incorrect indices were used for "all_attentions" ([link](https://github.com/huggingface/transformers/blob/539e2281cd97c35ef4122757f26c88f44115fa94/src/transformers/models/pix2struct/modeling_pix2struct.py#L1549)) and "all_cross_attentions" ([link](https://github.com/huggingface/transformers/blob/539e2281cd97c35ef4122757f26c88f44115fa94/src/transformers/models/pix2struct/modeling_pix2struct.py#L1550)) where the indices should have been 3 and 5, respectively, but the original code used 2 and 3.

code snippet:

```
from PIL import Image
import requests
from transformers import AutoProcessor, Pix2StructForConditionalGeneration

processor = AutoProcessor.from_pretrained("google/pix2struct-textcaps-base")
model = Pix2StructForConditionalGeneration.from_pretrained("google/pix2struct-textcaps-base")
url = "https://www.ilankelman.org/stopsigns/australia.jpg"
image = Image.open(requests.get(url, stream=True).raw)
inputs = processor(images=image, return_tensors="pt")

outputs = model(**inputs, decoder_input_ids=torch.tensor([[0]]), output_attentions=True)
print(outputs.cross_attentions[0].shape)
```

The shape of cross_attentions should be (batchsize, head num, text tokens, image tokens). After the modification, the shape is (1, 12, 1, 2048), which is correct. However, before the modification, the shape was (1, 12, 1, 1), which is clearly incorrect, as it represents the cross-attention between text and image.

The "attention" in the original code is also incorrect as it retrieves the position bias instead of the attention. Actually, there is a correct comment regarding this matter: [comment](https://github.com/huggingface/transformers/blob/539e2281cd97c35ef4122757f26c88f44115fa94/src/transformers/models/pix2struct/modeling_pix2struct.py#L1532) , but it is unclear why the subsequent code does not use the indices provided in the comment to retrieve the correct values.

Comments:
cc @younesbelkada 

The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_23985). All of your documentation changes will be reflected on that endpoint.

Now it has passed all tests, but this is just a temporary measure. In most cases, it can return the correct attentions and cross-attentions, but I have not figured out why there are index errors in a few cases, which causes the test to fail. In theory, as long as the output_attentions parameter is set to True, it should get enough parameters in the tuple. This is very strange. I will continue to investigate when I have time.

Hi @loveisp ! 
Again thanks for your contribution on this
Can you share with us why this PR got closed? The PR should also fix #24717 so it would be great to merge it :D 

> Hi @loveisp ! Again thanks for your contribution on this Can you share with us why this PR got closed? The PR should also fix #24717 so it would be great to merge it :D

Sorry I don't know how to merge it. Could you help me do that?

Hi @younesbelkada @amyeroberts @loveisp, sorry for bothering you. I have a question and would appreciate your insights. I was wondering why tensors are passed in a tuple instead of using dataclasses with None-able fields. It seems like using tuples can be error-prone and make bug detection and fixing more challenging due to the variable size. Additionally, it may require additional comments in the code. I'm considering creating an example PR to explore the possibility of using dataclasses, but before proceeding, I wanted to check if there are specific reasons for using tuples. Your input would be greatly appreciated. Thank you!

@artyomxyz There's no need to apologise for asking questions. People are busy so they may not always reply, but questions, especially if they're thoughtfully written like yours, are always welcome :) 

Yes, indexing with tuples is error prone! You'll notice that our models accept `return_dict` as an argument in their forward pass, and that by default they return a dataclass. We can't force passing and returning dataclasses as torchscript has only recently (and I'm not sure if fully) [started supporting this](https://github.com/pytorch/pytorch/issues/72901) and we need to maintain backwards compatibility with older library versions. c.f. 
* a discussion when I raised something similar: https://github.com/huggingface/transformers/pull/22970#discussion_r1195252303. 
* [Here](https://github.com/huggingface/transformers/blob/aac4c7996837b77abe91cd1ea734b6ef74117156/src/transformers/configuration_utils.py#L408C14-L408C14) in our config code where we force `return_dict=False` if using torchscript. 



Oh, thank you very much for explanation. It makes much more sense to me now