Seq2seq trainer
Author: sgugger
Pull request: yes

# What does this PR do?

This PR graduates `Seq2SeqTrainer` and moves it inside the transformers library. By doing so, it moves some of the features of the `Seq2SeqTrainer` inside the main `Trainer` and leaves some in the subclass. More precisely, the following features will be available in the general Trainer:
- label smoothing is passed to the main `Trainer` (so it can be used in all classification problems), with an easier API, bug fixes (the current implementation did not work with -100 has an ignore index for the loss, now it does; also the current implementation returns a loss that is not averaged thus being too big, see below)
- the ability to pick any scheduler
- the ability to use Adafactor instead of AdamW

The sortish-sampling and predict with generate are left in the subclass.

There are also a few breaking changes in the `Seq2SeqTrainer` API to make its init match the one of `Trainer` mainly the init does not take a `config` and a `DataArguments`, instead:
- the token IDs are taken from the tokenizer
- the arguments for generation are passed to `evaluate` or `predict`
- the `ignore_pad_token_for_loss` is passed in the init but is deprecated since it should be removed once BART and subclasses use -100.

About label smoothing and the mean vs sum. The current implementation takes the sum over the batch size and sequence length (only counting tokens that have a label != -100). This gives something that does not have the same scale as the usual cross entropy loss (which is the mean on the same dimensions) thus would require a special learning rate to be useful. With the fix, the label-smoothed loss had the same scale as the non-smoothed loss, which means the same command with the same learning rate should produce comparable results.


Comments:
Oh and this PR does not delete the old `Seq2SeqTrainer` just yet since I'm planning to merge it just before my vacation. That way if something goes horribly wrong, people can still use the old `Seq2SeqTrainer`. The plan would be to delete it in January after the new `Seq2SeqTrainer` has been tested.

>  change naming from `max_target_length` -> `max_length`. Think it's clearer this way that the args of `predict` and `eval` correspond 1-to-1 to the `max_length` of `generate()`

@patrickvonplaten - did you mean to suggest to change the new arguments to evaluate/predict that this PR adds or to rename `--max_target_length`, `--val_max_target_length` cl args?

> suggest

I was more referring to the args of the functions, but more generally I think it would actually be better if there would be only one `max_length` in the data_args - so leave the `max_length` that we have now and completely remove the `source_max_length` argument. IMO, `source_max_length` should be fully defined by the tokenizer of the model. I don't really see a need to let the user define the maximum input length, but this is probably better to be done in a separate PR. On the other hand, we also do have a `max_seq_length` argument in `run_mlm.py` so not 100% sure what's best here...What is your opinion here @stas00 @sgugger @patil-suraj ?

I think it's better to keep `max_source_length`, `max_target_length` since in some cases the input length could be way shorter than the tokenizer or model's max length and these two could be used to truncate the text .

We can get rid of `val_max_target_length` and `test_max_target_length` in `DataTrainingArguments`, since in almost all scripts we are using the same length for all three arguments (`max_target_length`,  `val_max_target_length`, `test_max_target_length`). Then we could pass the same `max_target_length`  to both `evaluate` and `predict` methods.

sorry about the miscommunication.

I don't know whether this is a good time, but should we add `min_length` as well here while this part of the API is being redesigned? Surely if generate has `min_length` someone might need to redefine it too? But I'm totally fine to deferring this until and if someone asks for it.

The best way to decide about the naming is to show a few use cases - and then it's clear whether these are needed or not.

Please don't forget that Sam or whoever added those in first place had a good reason for it, so it'd be hard to make a decision in the void.

Perhaps such decision shouldn't be rushed - but made into an RFC, invite input from users who have a lot more use cases?