Pass encoder outputs into GenerationMixin
Author: ymfa
Pull request: yes

# What does this PR do?

For encoder-decoder models such as T5, `GenerationMixin.generate()` currently runs both the encoder and the decoder. This PR allows one to pass already-computed `encoder_outputs` into this method, thus only the decoder will be run.

The flexibility to skip the encoder in the generation utilities is useful for several different scenarios, such as:
- The T5 encoder can encode `inputs_embeds` instead of `input_ids`. However, this is not possible within the generation utilities because only `input_ids` is accepted. With the changes in this PR, one can encode the `inputs_embeds` separately, and pass the encoder outputs to `generate()`. This is a partial solution to the issue https://github.com/huggingface/transformers/issues/6535.
- In some applications, the same encoder outputs are reused in different decoding processes. It would be computationally efficient not having to recompute the encoder outputs.
- This would also allow altering the encoder outputs for the purpose of incorporating additional information, etc. (In general, I think it is good practice to offer this option, where the encoding process is "decoupled" from the generation utilities.)


## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [x] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md#start-contributing-pull-requests),
      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.
- [x] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/docs#writing-source-documentation).
- [ ] Did you write any new necessary tests?


## Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors which may be interested in your PR.

<!-- 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:

- albert, bert, xlm: @LysandreJik
- blenderbot, bart, marian, pegasus, encoderdecoder,  t5: @patrickvonplaten, @patil-suraj
- longformer, reformer, transfoxl, xlnet: @patrickvonplaten
- fsmt: @stas00
- funnel: @sgugger
- gpt2: @patrickvonplaten, @LysandreJik
- rag: @patrickvonplaten, @lhoestq
- tensorflow: @jplu

Library:

- benchmarks: @patrickvonplaten
- deepspeed: @stas00
- ray/raytune: @richardliaw, @amogkam
- text generation: @patrickvonplaten
- tokenizers: @n1t0, @LysandreJik
- trainer: @sgugger
- pipelines: @LysandreJik

Documentation: @sgugger

HF projects:

- nlp datasets: [different repo](https://github.com/huggingface/nlp)
- rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)

Examples:

- maintained examples (not research project or legacy): @sgugger, @patil-suraj
- research_projects/bert-loses-patience: @JetRunner
- research_projects/distillation: @VictorSanh

 -->


Comments:
Hi @ymfa , thanks a lot for the PR!

- The `generate` method does allow you to pass `input_embeds` as a keyword argument (`**model_kwargs` arg). If `input_embeds` is passed then T5 or any other model will use those instead of `input_ids`. If you look at the `generate` method signature, you can see that `input_ids` is optional.

-  I think we can allow to pass `encoder_outputs` directly to `generate`. Pinging @patrickvonplaten 

Hey @ymfa thanks for your PR and your thoughtful explanations in the description! 

The design philosophy of `generate()` is that the 99% cases should be covered by `generate()` and for specific cases the sub-generated generate functions should be called directly as it is the case in the examples here:
https://github.com/huggingface/transformers/blob/0d909f6bd8ca0bc1ec8f42e089b64b4fffc4d230/src/transformers/generation_utils.py#L1592

As you can see, you only need to add a couple more lines when directly using `beam_search(...)` instead `generate(...)`. Could this solve your use case?





Hi @patil-suraj , thanks for your comment. 

I do realise that I can pass additional arguments via `**model_kwargs`. However, it doesn't work if I pass `inputs_embeds` instead of `input_ids`, because there are a number of places in `generate()` that depend on `input_ids`. 

Specifically, this is the error I got (model is T5ForConditionalGeneration):
```
>>> model.generate(inputs_embeds=input_embeded)
Traceback (most recent call last):
...
ValueError: `bos_token_id` has to be defined when no `input_ids` are provided.
```

If I do pass `bos_token_id` (which shouldn't be necessary), this is the error (it's because the `input_ids` are created from `bos_token_id` and passed to the encoder):
```
>>> model.generate(inputs_embeds=input_embeded, bos_token_id=0)
Traceback (most recent call last):
...
ValueError: You cannot specify both inputs and inputs_embeds at the same time
```

So in the end, I am now using the method in this PR to achieve this purpose.

Thanks @patrickvonplaten . 

To be honest I haven't found a way to make `beam_search()` work even for the simple use case of passing ids only. In this example (model is T5ForConditionalGeneration), `input_ids_batch` is the just 5 identical `input_ids` stacked together for the beam size.
```
>>> input_ids_batch.shape
torch.Size([5, 15])
>>> model.beam_search(input_ids_batch, beam_scorer)
Traceback (most recent call last):
...
  File "...python3.7/site-packages/transformers/models/t5/modeling_t5.py", line 871, in forward
    raise ValueError(f"You have to specify either {err_msg_prefix}inputs or {err_msg_prefix}inputs_embeds")
ValueError: You have to specify either inputs or inputs_embeds
```

When I pass the `encoder_outputs`:
```
>>> model.beam_search(decoder_input_ids_batch, beam_scorer, encoder_outputs=encoder_outputs)
Traceback (most recent call last):
...
  File "...python3.7/site-packages/transformers/models/t5/modeling_t5.py", line 498, in forward
    scores += position_bias
RuntimeError: The size of tensor a (3) must match the size of tensor b (15) at non-singleton dimension 3
```

I don't think it would be a simpler PR if I change `beam_search()` instead of `generate()`. It would also be less general as it doesn't apply to the other decoding methods, and it would require the user to prepare `beam_scorer` and `decoder_input_ids_batch`. 

>     scores += position_bias

I think you just need to broadcast your `decoder_input_ids_batch` to be of the same size as `encoder_outputs`, then it should work. 

Overall, I'm not really in favor of adding this new special use case. To me it's not naturally to do `model.generate(None, decoder_input_ids=..., encoder_outputs=encoder_outputs)` lot of people won't understand that the first argument has in fact to be `None` since it corresponds to the encoder input ids. For such a case it should be relatively easy to make it work with `beam_search` to be honest. The philosophy here is that that both `decoder_input_ids` and `encoder_outputs.last_hidden_state` have to be of the sample batch dimensions, so `encoder_outputs.shape][0] == decoder_input_ids.shape[0]` . Then the command is as simple as:

```
model.beam_search(decoder_input_ids, beam_scorer, encoder_outputs)
```

The problem is that if we allow to many specific use cases for `generate()` the method becomes quite cluttered with if-statements again and I would like to avoid it. In this case, I think it's much cleaner to directly call the `beam_search` method tbh.

What do you think @patil-suraj ?

I've made a change so that the method signature of `generate()` remains the same as before. There is not "clutterness" added into this method now. This PR can be regarded as a fix, by properly handling the case when `encoder_outputs` is passed as one of the `model_kwargs`.

You're right that decoder_input_ids and encoder_outputs.last_hidden_state have to be of the sample batch dimensions. However, this means both of them need to be broadcast according to the beam size, which is not done automatically. A user would have to broadcast these tensors and objects manually in order to call `beam_search()`.

To be honest, I have been using the patch I submitted here for doing both beam search and sampling. The easy-to-use generation utility is one of my main reasons to choose the transformers package. I don't think it is a good idea to limit the potential based on "cleanness." On the contrary, I even suggest refactoring `generate()` more systematically, so that `input_ids` is no longer used like a central "currency" in this method.

Hey @ymfa, 

It's actually a good point that changing between different generation methods while using `encoder_outputs` is not very user-friendly and "pre-computing" encoder_outputs is quite a common use case for all seq2seq models. Also since only the "helper" methods `_prepare...` are changed, I think I'm fine with the PR now! Thanks for being persistent here!

@patil-suraj @LysandreJik - it would be nice you could take a look as well.

I think `generate` now deserves its own doc page where we could explain this and maybe some more details like what the method supports, what are its limitations and what features won't be supported etc. It's changed significantly after the refactor and the design now follows a strict philosophy. It would be better to document that. What do you think @patrickvonplaten?

@ymfa  The PR is good to merge, I'll merge it once you add helpful comments in `_prepare_input_ids_for_generation`  as suggested by Patrick and Lysandre.