Loss calculation bug in MBartForCausalLM
Author: ZL92
Pull request: no

### System Info

transformers==4.35.1

### Who can help?

@ArthurZucker @younesbelkada


### Information

- [x] The official example scripts
- [X] My own modified scripts

### Tasks

- [X] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

The story is that I followed the [tutorial](https://huggingface.co/docs/transformers/tasks/language_modeling) for building a causal language model and replaced the distleGPT2 with MbartForCausalLM. The model seems to copy the input during training. Then I found one bug in the loss calculation of Mbart. 

Specifically, the label is the copy of input_ids with [DataCollatorForLanguageModeling](https://github.com/huggingface/transformers/issues/27517#issue-1994919889) (with mlm=False). The LM models like GPT2 and Llama shift the labels while MBart doesn't.

The loss calculation of MbartForCausalLM
[code](https://github.com/huggingface/transformers/blob/f1185a4a73a03d238afce1b40456588d22520dd2/src/transformers/models/mbart/modeling_mbart.py#L2066)
```
loss = None
if labels is not None:
    labels = labels.to(logits.device)
    loss_fct = CrossEntropyLoss()
    loss = loss_fct(logits.view(-1, self.config.vocab_size), labels.view(-1))
```

While that in LlamaForCausalLM
[code](https://github.com/huggingface/transformers/blob/f1185a4a73a03d238afce1b40456588d22520dd2/src/transformers/models/llama/modeling_llama.py#L1056)
```
loss = None
if labels is not None:
    # Shift so that tokens < n predict n
    shift_logits = logits[..., :-1, :].contiguous()
    shift_labels = labels[..., 1:].contiguous()
    # Flatten the tokens
    loss_fct = CrossEntropyLoss()
    shift_logits = shift_logits.view(-1, self.config.vocab_size)
    shift_labels = shift_labels.view(-1)
    # Enable model parallelism
    shift_labels = shift_labels.to(shift_logits.device)
    loss = loss_fct(shift_logits, shift_labels)
```

I find the same problem also in BartForCausalLM. 

### Expected behavior

Add the shifting in MbartForCausalLM.

For example:    

```
loss = None
if labels is not None:
    labels = labels.to(logits.device)

    # Shift so that tokens < n predict n
    shift_logits = logits[..., :-1, :].contiguous()
    shift_labels = labels[..., 1:].contiguous()

    loss_fct = CrossEntropyLoss()
    loss = loss_fct(shift_logits.view(-1, self.config.vocab_size), shift_labels.view(-1))
```

Comments:
Hi @ZL92 
Thanks for bringing up this discussion
Per my understanding since these models (bart, mbart), are encoder-decoder base models, the logic is slightly different, instead of shifting the labels during the calculation of the loss, they are shifted before being fed to the decoder e.g. here for Bart: https://github.com/huggingface/transformers/blob/main/src/transformers/models/bart/modeling_bart.py#L1433



@younesbelkada Thanks for your reply! Sorry for the unclear description, I mean the Causal Language Model classes for Mbart and Bart, and I added the link to the source code into the description. 

If I understand correctly, in building causal language models, the [DataCollatorForLanguageModeling](https://github.com/huggingface/transformers/blob/514de24abfd4416aeba6a6455ad5920f57f3567d/src/transformers/data/data_collator.py#L744) copies the input_ids as labels, and do shifting in loss calculation. Llama and GPT2 do so, but Mbart and Bart do not.



Hi @ZL92 
Thanks ! 
OK I had a deeper look, so it appears that:

1- `MbartForCausalLM` has a `MbartDecoderWrapper` attribute here: https://github.com/huggingface/transformers/blob/f1185a4a73a03d238afce1b40456588d22520dd2/src/transformers/models/mbart/modeling_mbart.py#L1914
2- That decoder wrapper is a wrapper around `MbartDecoder` here: https://github.com/huggingface/transformers/blob/f1185a4a73a03d238afce1b40456588d22520dd2/src/transformers/models/mbart/modeling_mbart.py#L1899
3- That decoder do not call `shift_labels` indeed

Therefore the solution for that corner case (for now) is to subclass `DataCollatorForLanguageModeling` and do it inside the data collator call while we fix it, not sure however what should be the fix as always shifting input ids to the right will result in a non-backward compatible behaviour 

Hi @younesbelkada,

Thanks for your solution! 

I tried to shift the labels as shown in the following to have not that much modification of the Causal LM training pipeline. The training looks good now. 
```
loss = None
if labels is not None:
    labels = labels.to(logits.device)

    # Shift so that tokens < n predict n
    shift_logits = logits[..., :-1, :].contiguous()
    shift_labels = labels[..., 1:].contiguous()

    loss_fct = CrossEntropyLoss()
    loss = loss_fct(shift_logits.view(-1, self.config.vocab_size), shift_labels.view(-1))
```

Look forward to the fix! 

Thanks ! cc @ArthurZucker I wonder how we can fix that in a BC manner

If it's fix it's fine to break 😉 

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the [contributing guidelines](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.