gpt2 generation crashes when using `past` for some output lengths
Author: joelb-git
Pull request: no

## 🐛 Bug

<!-- Important information -->

Model I am using (Bert, XLNet....): gpt2

Language I am using the model on (English, Chinese....): English

The problem arise when using:
* [ ] the official example scripts: (give details)
* [x] my own modified scripts: (give details)

The tasks I am working on is:
* [ ] an official GLUE/SQUaD task: (give the name)
* [x] my own task or dataset: (give details)

I am using `run_generation.py` to generate text from gpt2, but with the code slightly changed to make use of `past` to cache hidden states.  It crashes in some cases.

## To Reproduce

Steps to reproduce the behavior:

Apply the three line change to the `run_generation.py` script:

```
$ git checkout f88c104d8 .   # current head of master
$ git diff
diff --git a/examples/run_generation.py b/examples/run_generation.py
index 2d91766..bfbf68a 100644
--- a/examples/run_generation.py
+++ b/examples/run_generation.py
@@ -113,9 +113,10 @@ def sample_sequence(model, length, context, num_samples=1, temperature=1, top_k=
     context = context.unsqueeze(0).repeat(num_samples, 1)
     generated = context
     with torch.no_grad():
+        past = None
         for _ in trange(length):

-            inputs = {'input_ids': generated}
+            inputs = {'input_ids': generated, 'past': past}
             if is_xlnet:
                 # XLNet is a direct (predict same token, not next token) and bi-directional model by default
                 # => need one additional dummy token in the input (will be masked), attention mask and target mapping (see model docstring)
@@ -136,6 +137,7 @@ def sample_sequence(model, length, context, num_samples=1, temperature=1, top_k=
                 inputs["langs"] = torch.tensor([xlm_lang] * inputs["input_ids"].shape[1], device=device).view(1, -1)

             outputs = model(**inputs)  # Note: we could also use 'past' with GPT-2/Transfo-XL/XLNet/CTRL (cached hidden-states)
+            past = outputs[1]
             next_token_logits = outputs[0][:, -1, :] / (temperature if temperature > 0 else 1.)

             # repetition penalty from CTRL (https://arxiv.org/abs/1909.05858)
```

```
$ python examples/run_generation.py --prompt "Who was Jim Henson ? Jim Henson was a" --model_type gpt2 --model_name_or_path gpt2 --length 40
...
11/06/2019 11:38:36 - INFO - __main__ -   Namespace(device=device(type='cpu'), length=40, model_name_or_path='gpt2', model_type='gpt2', n_gpu=0, no_cuda=False, num_samples=1, padding_text='', prompt='Who was Jim Henson ? Jim Henson was a', repetition_penalty=1.0, seed=42, stop_token=None, temperature=1.0, top_k=0, top_p=0.9, xlm_lang='')
 88%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████▍                | 35/40 [00:04<00:00,  8.68it/s]
Traceback (most recent call last):
  File "examples/run_generation.py", line 262, in <module>
    main()
  File "examples/run_generation.py", line 247, in main
    device=args.device,
  File "examples/run_generation.py", line 139, in sample_sequence
    outputs = model(**inputs)  # Note: we could also use 'past' with GPT-2/Transfo-XL/XLNet/CTRL (cached hidden-states)
  File "/Users/joelb/anaconda3/envs/huggingface/lib/python3.7/site-packages/torch/nn/modules/module.py", line 541, in __call__
    result = self.forward(*input, **kwargs)
  File "/Users/joelb/views/transformers/transformers/modeling_gpt2.py", line 546, in forward
    inputs_embeds=inputs_embeds)
  File "/Users/joelb/anaconda3/envs/huggingface/lib/python3.7/site-packages/torch/nn/modules/module.py", line 541, in __call__
    result = self.forward(*input, **kwargs)
  File "/Users/joelb/views/transformers/transformers/modeling_gpt2.py", line 438, in forward
    position_embeds = self.wpe(position_ids)
  File "/Users/joelb/anaconda3/envs/huggingface/lib/python3.7/site-packages/torch/nn/modules/module.py", line 541, in __call__
    result = self.forward(*input, **kwargs)
  File "/Users/joelb/anaconda3/envs/huggingface/lib/python3.7/site-packages/torch/nn/modules/sparse.py", line 114, in forward
    self.norm_type, self.scale_grad_by_freq, self.sparse)
  File "/Users/joelb/anaconda3/envs/huggingface/lib/python3.7/site-packages/torch/nn/functional.py", line 1484, in embedding
    return torch.embedding(weight, input, padding_idx, scale_grad_by_freq, sparse)
RuntimeError: index out of range: Tried to access index 1024 out of table with 1023 rows. at /Users/distiller/project/conda/conda-bld/pytorch_1570710797334/work/aten/src/TH/generic/THTensorEvenMoreMath.cpp:418
```
<!-- If you have a code sample, error messages, stack traces, please provide it here as well. -->

## Expected behavior

<!-- A clear and concise description of what you expected to happen. -->

The crash does not occur with the default `--length 20`.

I'm trying to make this work for faster generation speed.  So I tested the change with a smaller length that does not crash, but I observe the speed with caching is slightly slower than without.

## Environment
```
Platform Darwin-18.7.0-x86_64-i386-64bit
Python 3.7.4 (default, Aug 13 2019, 15:17:50)
[Clang 4.0.1 (tags/RELEASE_401/final)]
PyTorch 1.3.0
Tensorflow 2.0.0
```

## Additional context

<!-- Add any other context about the problem here. -->


Comments:
@joelb-git Can you check the lowest ```length``` at which this occurs? 

Also, is this an issue that only occurs when ```past``` is used? 

Can you check if same thing happens when running

```python examples/run_generation.py --prompt "Who was Jim Henson ? Jim Henson was a" --model_type gpt2 --model_name_or_path gpt2 --length 40```

on the unchanged ```run_generation.py``` file

@enzoampil

> @joelb-git Can you check the lowest length at which this occurs?

`--length 36` is the smallest length that produces the failure.

> Also, is this an issue that only occurs when past is used?

Yes.  Also, the results when using `past` when it does not crash are
different from without using `past`.

Without `past`:
```
$ python examples/run_generation.py --prompt "Who was Jim Henson ? Jim Henson was a" --model_type gpt2 --model_name_or_path gpt2 --length 20
 man of God. Jim Henson was a man who had built his family into this nation. His
```

With `past`:
```
$ python examples/run_generation.py --prompt "Who was Jim Henson ? Jim Henson was a" --model_type gpt2 --model_name_or_path gpt2 --length 20
 man of almostWho was Jim Henson? Jim Henson was a man of almostWho was Jim
```

> Can you check if same thing happens when running
> 
> python examples/run_generation.py --prompt "Who was Jim Henson ? Jim Henson was a" --model_type gpt2 --model_name_or_path gpt2 --length 40
> 
> on the unchanged run_generation.py file

On the unchanged file, this does not crash.

I also note a documentation issue:

https://huggingface.co/transformers/model_doc/gpt2.html
```
Outputs:
past:

list of torch.FloatTensor (one for each layer) of shape
(batch_size, num_heads, sequence_length, sequence_length) ...
```

The shape that I observe has an extra prefixed dimension:

```
(2, batch_size, num_heads, sequence_length, sequence_length)
```


I have the exact same issue (although I am using distilgpt2).

After some experimenting I found out that the generation (starting from BOS, i.e. without a prompt) always crashes when the generated sequence goes from length 44 to 45. At this point, I suspect that the hidden states in past are concatenated. For the sum of all numbers from 1 to 44 is 990. So if you add 45 again, you exceed the maximum number of positions in GPT2 (1024).

This would also explain why your results with and without past are different @joelb-git . Also it seems that the prompt ("Who was Jim Henson? ...") is repeated in the output when past is used. That might also be an artifact of concatenation?

I also get a similar issue. I suspect the error has something to do with the past attention weights being added to the present - since the error is a matrix multiplication shape mismatch. 

The issue may lay in the modelling_gpt2.py starting on line 181.
``` 
x = self.c_attn(x)
query, key, value = x.split(self.split_size, dim=2)
query = self.split_heads(query)
key = self.split_heads(key, k=True)
value = self.split_heads(value)
if layer_past is not None:
    past_key, past_value = layer_past[0].transpose(-2, -1), layer_past[1]  # transpose back cf below
    key = torch.cat((past_key, key), dim=-1)
    value = torch.cat((past_value, value), dim=-2)
present = torch.stack((key.transpose(-2, -1), value))
**attn_outputs = self._attn(query, key, value, attention_mask, head_mask)**
``` 

Where the error occurs in the method I have wrapped with **.
Going into the method, the error occurs in the line alsop marked with **:

```
def _attn(self, q, k, v, attention_mask=None, head_mask=None):
    w = torch.matmul(q, k)
     if self.scale:
         w = w / math.sqrt(v.size(-1))
     nd, ns = w.size(-2), w.size(-1)
     b = self.bias[:, :, ns-nd:ns, :ns]
     **w = w * b - 1e4 * (1 - b)**
```
The error occurs because this code tried to multiply **'w'** of shape ([1, 12, 1024, 2048]) by **'b'** of shape ([1, 1, 0, 1024]). However, I am unsure what are the appropiate shapes thus cannot provide the solution.

Hope this helps.



@adigoryl This is definitely related.
One can run this very simple script to reproduce the error:
```
import torch
from transformers import GPT2Tokenizer, GPT2LMHeadModel

tokenizer = GPT2Tokenizer.from_pretrained('distilgpt2')
model = GPT2LMHeadModel.from_pretrained('distilgpt2')
device = 'cuda:0'
model.to(device)

input_ids = torch.tensor(tokenizer.encode(
    "
")).unsqueeze(0).to(device)
inputs = {'input_ids': input_ids}

with torch.no_grad():
    past = None
    for i in range(45):
        print(i)
        logits, past = model(**inputs, past=past)
        logits = logits[0, -1]

        next_token = logits.argmax()
        input_ids = torch.cat([input_ids, next_token.view(1, 1)], dim=1)

        inputs = {'input_ids': input_ids}
print(tokenizer.decode(input_ids[0].tolist()))
```

It works fine for `range(44)` but at `range(45)` it crashes with the error @adigoryl describes:
```
Traceback (most recent call last):
  File "test.py", line 17, in <module>
    logits, past = model(**inputs, past=past)
  File "/mounts/Users/martin/.local/lib/python3.6/site-packages/torch/nn/modules/module.py", line 541, in __call__
    result = self.forward(*input, **kwargs)
  File "/mounts/Users/martin/.local/lib/python3.6/site-packages/transformers/modeling_gpt2.py", line 566, in forward
    head_mask=head_mask)
  File "/mounts/Users/martin/.local/lib/python3.6/site-packages/torch/nn/modules/module.py", line 541, in __call__
    result = self.forward(*input, **kwargs)
  File "/mounts/Users/martin/.local/lib/python3.6/site-packages/transformers/modeling_gpt2.py", line 470, in forward
    head_mask=head_mask[i])
  File "/mounts/Users/martin/.local/lib/python3.6/site-packages/torch/nn/modules/module.py", line 541, in __call__
    result = self.forward(*input, **kwargs)
  File "/mounts/Users/martin/.local/lib/python3.6/site-packages/transformers/modeling_gpt2.py", line 250, in forward
    head_mask=head_mask)
  File "/mounts/Users/martin/.local/lib/python3.6/site-packages/torch/nn/modules/module.py", line 541, in __call__
    result = self.forward(*input, **kwargs)
  File "/mounts/Users/martin/.local/lib/python3.6/site-packages/transformers/modeling_gpt2.py", line 211, in forward
    attn_outputs = self._attn(query, key, value, attention_mask, head_mask)
  File "/mounts/Users/martin/.local/lib/python3.6/site-packages/transformers/modeling_gpt2.py", line 154, in _attn
    w = w * b - 1e4 * (1 - b)
RuntimeError: The size of tensor a (1035) must match the size of tensor b (1024) at non-singleton dimension 3
```

`w` having length 1035 (which is the sum of all numbers from 1 to 45) also made me think that there is some concatenation going on that shouldn't be happening.

As I suspect a wrong idea about the interface and thus some concatenation going on that shouldn't be happening, I modified my example script in the following way:
```
import torch
from transformers import GPT2Tokenizer, GPT2LMHeadModel

tokenizer = GPT2Tokenizer.from_pretrained('distilgpt2')
model = GPT2LMHeadModel.from_pretrained('distilgpt2')
device = 'cuda:0'
model.to(device)

input_ids = torch.tensor(tokenizer.encode(
    "
")).unsqueeze(0).to(device)
inputs = {'input_ids': input_ids}

with torch.no_grad():
    past = None
    for i in range(45):
        print(i)
        logits, past = model(**inputs, past=past)
        logits = logits[0, -1]

        next_token = logits.argmax().view(1, 1)
        input_ids = torch.cat([input_ids, next_token], dim=1)

        inputs = {'input_ids': next_token}
print(tokenizer.decode(input_ids[0].tolist()))
```
So only `next_token` is given as an input to the gpt2 model. This permits me to generate sequences of arbitrary length while using the `past` parameter.
The only thing I would still really like to know is where this "bug" came from in the first place: do we as users have a wrong idea about the interface and should we use it in a way where `past` and `input_ids` do not overlap? Or were we right about the interface from the start and there really *is* a bug?

Surprisingly, both versions give me natural results when presented with the prompt `"
Who was Jim Henson? Jim Henson was a"`:
Without modification: `Who was Jim Henson? Jim Henson was a great father and a great father.`
With my modification: `Who was Jim Henson? Jim Henson was a great actor, and he was a great actor. He was a great actor. He was a great`

The same when using `gpt2` instead of `distilgpt2`:
Without modification: `Who was Jim Henson? Jim Henson was a writer and director of the film "The Last Man on Earth" and the sequel to the film "`
With modification: `Who was Jim Henson? Jim Henson was a writer, director, producer, and producer of the hit television series "The Henson Report." He`

So how are we supposed to use it?

Hi, when the past is computed for certain tokens these tokens should not be passed as input ids. @mnschmit your last example displays correct usage of the `past` argument. I've clarified the documentation in d409aca, please let me know if it should be clarified further.

Thank you for looking into it.


@LysandreJik great, thank you!
With the extended documentation, it would be clear to me now but, additionally, a usage example might be useful for new users. Feel free to put my short example (or parts of it) at some appropriate place if you like!

You're right, having a snippet showing usage would definitely improve user experience on this point. I'll update the documentation later today, thank you!

I've added a section in the [`quickstart` section of the documentation](https://huggingface.co/transformers/quickstart.html#using-the-past). Feel free to re-open if it's still not clear enough.

That's a great new part of the documentation. Thank you!

Yes, indeed, thanks for that doc change!  And thanks for all who helped diagnose.

I applied the suggestions to my local version of `run_generation.py`.
It no longer crashes for large generation lengths, and generation is
indeed now faster (about 1.7 times faster in my tests).

Here is the relevant portion of the modified `run_generation.py` script, changed for gpt2 only:

```
def sample_sequence(model, length, context, num_samples=1, temperature=1, top_k=0, top_p=0.0, repetition_penalty=1.0,
                    device='cpu'):
    context = torch.tensor(context, dtype=torch.long, device=device)
    context = context.unsqueeze(0).repeat(num_samples, 1)
    generated = context.clone().detach()
    past = None
    with torch.no_grad():
        for _ in range(length):
            output, past = model(context, past=past)
            next_token_logits = output[:, -1, :]
            filtered_logits = top_k_top_p_filtering(next_token_logits, top_k=top_k, top_p=top_p)
            next_token = torch.multinomial(F.softmax(filtered_logits, dim=-1), num_samples=1)
            generated = torch.cat((generated, next_token), dim=1)
            # when using `past`, the context for the next call should be only
            # the previous token
            context = next_token
    return generated
```

I encouter similar issue, and find the problem is cased by input_ids and length, beacuse wrongly use of special token [CLS] when tokenize input_ids 

I am trying a BERT-GPT2(not BART) architecture in which I am passing BERT's hidden states of 12 layers to past. After reading doc's I find that past should be of different shape as compared to shape of hidden states from BERT. Since I need BERT's encoding layer for encoding and GPT2 decoding function, tell me how should I manage it.