Different result in AutoModelForCausalLM
Author: voidful
Pull request: no

# 🚀 Feature request
Models inside AutoModelForCausalLM have different behavior on loss calculation.

In BartForCausalLM
there is no shift in loss calculation
https://github.com/huggingface/transformers/blob/b013842244df7be96b8cc841491bd1e35e475e36/src/transformers/models/bart/modeling_bart.py#L1745
```
loss_fct = CrossEntropyLoss()
loss = loss_fct(logits.view(-1, self.config.vocab_size), labels.view(-1))
```

In RobertaForCausalLM
A shift is applied before loss calculation
https://github.com/huggingface/transformers/blob/b013842244df7be96b8cc841491bd1e35e475e36/src/transformers/models/roberta/modeling_roberta.py#L944
```
# we are doing next-token prediction; shift prediction scores and input ids by one
shifted_prediction_scores = prediction_scores[:, :-1, :].contiguous()
labels = labels[:, 1:].contiguous()
loss_fct = CrossEntropyLoss()
lm_loss = loss_fct(shifted_prediction_scores.view(-1, self.config.vocab_size), labels.view(-1))
```


## Motivation
I found a mistake when I switched the config from Roberta to BART in AutoModelForCausalLM. It turns out to be different labeling in loss. So, It would be nice to make CausalLM models handle label in the same way, either shift or not.
  

## Your contribution
I can make a PR to make sure that all the models will have a shift prediction. 

Comments:
Hi @voidful 

The reason we need to shift labels in roberta because the `labels` start with the `decoder_start_token_id` (`pad` or `bos`),
which are then passed directly to the decoder as `decoder_input_ids`, which is the reason we need to shift the `labels` when calculating the loss

Now in BART, `decoder_input_ids` are prepared inside the model by pretending the `labels` with `decoder_start_token_id`, so we don't need to shift the labels there
https://github.com/huggingface/transformers/blob/b013842244df7be96b8cc841491bd1e35e475e36/src/transformers/models/bart/modeling_bart.py#L1274

Hope this clears the difference.

> Hi @voidful
> 
> The reason we need to shift labels in roberta because the `labels` start with the `decoder_start_token_id` (`pad` or `bos`),
> which are then passed directly to the decoder as `decoder_input_ids`, which is the reason we need to shift the `labels` when calculating the loss
> 
> Now in BART, `decoder_input_ids` are prepared inside the model by pretending the `labels` with `decoder_start_token_id`, so we don't need to shift the labels there
> 
> https://github.com/huggingface/transformers/blob/b013842244df7be96b8cc841491bd1e35e475e36/src/transformers/models/bart/modeling_bart.py#L1274
> 
> Hope this clears the difference.

I got your point, but it seems to be apply on BartForConditionalGeneration, 
https://github.com/huggingface/transformers/blob/b013842244df7be96b8cc841491bd1e35e475e36/src/transformers/models/bart/modeling_bart.py#L1272

Maybe we can apply same strategy on BartForCausalLM:
https://github.com/huggingface/transformers/blob/b013842244df7be96b8cc841491bd1e35e475e36/src/transformers/models/bart/modeling_bart.py#L1594

Oh, I missed your point, great catch!

Feel free to open PR to add the same strategy to `BartForCausalLM`, i.e prepare `decoder_input_ids` using the `shift_tokens_right` and pass them as the `input_ids` to the decoder.

cc @patrickvonplaten 

Hmm, I'm not 100% whether everybody is on the same page here. `BartForCausalLM` was mostly created to be used in combination with `EncoderDecoderModel` and not as a standalone model. Also, Roberta requires both `input_ids` and `labels` as an input to correctly calculate the loss - the difference is just that that `input_ids` should be equal to `labels` with the labels being shifted under-the-hood. This is not the same thing as the `shift_tokens_right` function, which fully generates the `decoder_input_ids` from the labels...

I think I would be fine with changing the behavior of `BartForCausalLM` so that `labels==input_ids` can be input to the function, even if this would be a slight breaking change. It would align `BartForCausalLM` closer with `RobertaForCausalm, GPT2LMHeadModel, ...` which would then also allow `EncoderDecoderModel` to have a general `shift_tokens` function.

Does this make sense?

`BartForCausalLM` does accept `labels==input_id`, in general, all the decoders in `EncoderDecoder` accept that and that's what we have documented, pass the same input as `labels` and `decoder_input_ids`.

The reason I suggested using `shift_tokens_right`, because BART uses `eos` as `decoder_start_token` which the `shift_tokens_right` function handles. This is different from `RobertaForCausalm, GPT2LMHeadModel ...`