Some changes to simplify the generation function
Author: yjernite
Pull request: yes

This PR proposes to simplify generation in `modeling_utils.py` in the following ways:

1. Removing some redundant code in `_generate_no_beam_search`: finished sequences are padded at generation time, and do not need to be padded again before returning
2. Initializes the cache in its permanent form directly for both `_generate_beam_search` and `_generate_no_beam_search`: this removes the need for a first step test in `modeling_bart.py` and `modeling_t5.py` (and presumably future cached seq2seq architectures)
3. Took all of the logit post-processing out of `_generate_beam_search` and `_generate_no_beam_search` and put it in a single `finalize_generation_logscores` function that can be used in both instead of duplicating the code

The following slow test pass in addition to the basic suite:
```
RUN_SLOW=1 pytest tests/test_modeling_bart.py
RUN_SLOW=1 pytest tests/test_modeling_gpt2.py
RUN_SLOW=1 pytest tests/test_modeling_t5.py
```

#### Small breaking changes
1. The previous versionof `_generate_no_beam_search` seemed to be adding padding twice. We removed is as is seemed redundant, but noting it here just in case.
2. This PR moves the addition of the CTRL length penalty from before to after the `log_softmax` in `_generate_beam_search`. This changes scores a little bit but apparently doesn't drastically alter model behavior. Per @patrickvonplaten :
>  Now, the only thing that will slightly change with this function is that the CTRL enforce penalty for beam search decoding. I tried the new order out on a couple of tensors and the changes are minimal. Also, the repetition penalty is very hacky anyways and we already changed the function from its original formula of the CTRL paper.

Comments:
# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/5031?src=pr&el=h1) Report
> Merging [#5031](https://codecov.io/gh/huggingface/transformers/pull/5031?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/7291ea0bff57a017e71b1ea8ec01ff19da298bf0&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `90.32%`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/transformers/pull/5031/graphs/tree.svg?width=650&height=150&src=pr&token=9qOlN6Hb1c)](https://codecov.io/gh/huggingface/transformers/pull/5031?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #5031      +/-   ##
==========================================
+ Coverage   77.24%   77.26%   +0.01%     
==========================================
  Files         133      133              
  Lines       22146    22128      -18     
==========================================
- Hits        17107    17097      -10     
+ Misses       5039     5031       -8     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/5031?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/5031/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ191dGlscy5weQ==) | `91.06% <88.88%> (+0.32%)` | :arrow_up: |
| [src/transformers/modeling\_bart.py](https://codecov.io/gh/huggingface/transformers/pull/5031/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19iYXJ0LnB5) | `96.25% <100.00%> (-0.02%)` | :arrow_down: |
| [src/transformers/modeling\_encoder\_decoder.py](https://codecov.io/gh/huggingface/transformers/pull/5031/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19lbmNvZGVyX2RlY29kZXIucHk=) | `92.20% <100.00%> (ø)` | |
| [src/transformers/modeling\_t5.py](https://codecov.io/gh/huggingface/transformers/pull/5031/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190NS5weQ==) | `84.16% <100.00%> (-0.07%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/5031/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl91dGlscy5weQ==) | `87.26% <0.00%> (+0.62%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/5031?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/transformers/pull/5031?src=pr&el=footer). Last update [7291ea0...f507cd7](https://codecov.io/gh/huggingface/transformers/pull/5031?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Okey I took a deeper look at the PR! Quite hard to see what's all going on there, so a nice refactoring would be very welcome :-). 

I very much like the small changes that are done here that clearly improve the readability and clean the code.

The big change of unifying the score computation I am not yet 100% convinced it helps a lot. I agree that we should refactor the generate() function, but not sure whether this is going in the right direction. 

Pro's
- The function reduces duplicated code

Con's
- The function adds more computational cost to the `.generate()` function, which is already quite expensive and heavily used in GPT2 by an additional softmax function. Thinking about how big the output embedding matrices are, this could be significant no?

I do agree though that we should unify all these functions that are applied to the scores.
IMO, we have to very careful with everything that touches sampling in `_no_generate_beam_search` (greedy decoding is less used here) and everything that touches `argmax` in `_generate_beam_search` (summarization and translation rely on that).

My proposal would be the following: 
Let's unify all functions that are applied after the `F.log_softmax(next_token_logits, dim=-1)` line into one function and that expects the scores for beam search and the logits for no beam search (I like @sshleifer's naming - I would just say `postprocess_next_token_scores`). This function should be independent of wheter we sample or use the argmax:

```python
def postprocess_next_token_scores(
        self,
        scores,
        input_ids,
        batch_size,
        num_beams,
        no_repeat_ngram_size,
        bad_words_ids,
        cur_len,
        min_length,
        eos_token_id,
        repetition_penalty,
    ):
        
        # repetition penalty (from CTRL paper https://arxiv.org/abs/1909.05858)
        if repetition_penalty != 1.0:
            self.enforce_repetition_penalty_(
                next_token_logits, batch_size, num_beams, input_ids, repetition_penalty,
        )
        
        # set eos token prob to zero if min_length is not reached
        if eos_token_id is not None and cur_len < min_length:
            scores[:, eos_token_id] = -float("inf")

        if no_repeat_ngram_size > 0:
            # calculate a list of banned tokens to prevent repetitively generating the same ngrams
            num_batch_hypotheses = batch_size * num_beams
            # from fairseq: https://github.com/pytorch/fairseq/blob/a07cb6f40480928c9e0548b737aadd36ee66ac76/fairseq/sequence_generator.py#L345
            banned_batch_tokens = calc_banned_ngram_tokens(
                input_ids, num_batch_hypotheses, no_repeat_ngram_size, cur_len
            )
            for i, banned_tokens in enumerate(banned_batch_tokens):
                scores[i, banned_tokens] = -float("inf")

        if bad_words_ids is not None:
            # calculate a list of banned tokens according to bad words
            banned_tokens = calc_banned_bad_words_ids(input_ids, bad_words_ids)

            for i, banned_tokens in enumerate(banned_tokens):
                scores[i, banned_tokens] = -float("inf")

        return scores
```

This way for 1) we have a function that is applicable for both sampling and greedy search and only should contain function that do so and 2) there is no additional computation cost for the softmax.

The temperature function should then for both beam search and no beam search only be applied in the `if do_sample=True` statement (no need to do this for argmax).

Now, the only thing that will slightly change with this function is that the CTRL enforce penalty for beam search decoding. I tried the new order out on a couple of tensors and the changes are minimal. Also, the repetition penalty is very hacky anyways and we already changed the function from its original formula of the CTRL paper. Also, I haven't seen that anybody used the function really for beam search. What do you think @sshleifer?.

`repetition_penalty!=1` only used by CTRL, `model_cards/mrm8488/t5-base-finetuned-summarize-news/README.md:` and `model_cards/gaochangkuan/model_dir/README.md` 
So I think minimal changes to when it is computed are fine.

I changed the `finalize_logits` to @patrickvonplaten 's suggested `postprocess_next_token_scores`

Following @sshleifer 's sage advice, I'm leaving the BART starting hack and the temperature in the main body of the generate function for now, will leave dealing with those for a future PR :) 


Great, I'm happy with the PR - I think it already makes generate a bunch more readable. 
Can we note in the PR description, that we have slight breaking changes for beam search sampling when running with the repetition penalty? and it would be nice to make the function call more robust by using keyword arguments the same way it is done with `_no_beam_search_generate()`



> Great, I'm happy with the PR - I think it already makes generate a bunch more readable.
> Can we note in the PR description, that we have slight breaking changes for beam search sampling when running with the repetition penalty? and it would be nice to make the function call more robust by using keyword arguments the same way it is done with `_no_beam_search_generate()`

Done and done, will merge today.

In future, we also need to run 
```bash
pytest RUN_SLOW=1 tests/test_modeling_marian.py
```