Incorrect handling of EOS tokens in DataCollatorForLanguageModeling when pad_token is set to eos_token
Author: PatrykNeubauer
Pull request: no

### System Info

Doesn't seem to be version specific, but:
- `transformers` version: 4.29.2
- Platform: Linux-5.15.107+-x86_64-with-glibc2.31
- Python version: 3.10.11
- Huggingface_hub version: 0.14.1
- Safetensors version: not installed
- PyTorch version (GPU?): 2.0.1+cu118 (False)
- Tensorflow version (GPU?): 2.12.0 (False)
- Flax version (CPU?/GPU?/TPU?): 0.6.9 (cpu)
- Jax version: 0.4.8
- JaxLib version: 0.4.7
- Using GPU in script?: No
- Using distributed or parallel set-up in script?: No

### Who can help?

@sgugger I think?

### Information

- [X] The official example scripts
- [X] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [X] My own task or dataset (give details below)

### Reproduction

When using `DataCollatorForLanguageModeling` for CLM with `pad_token` set to `eos_token` as it's shown [here](https://huggingface.co/docs/transformers/tasks/language_modeling ) - all EOS tokens in the labels are overwritten with -100, instead of just the ones used for padding.

In colab [here](https://colab.research.google.com/drive/13JsslKctbc9JEWbsJEJ6xru4zm6QxmLt?usp=sharing).

1. Prepare sentences that should be included in a single batch with explicit EOS tokens. (here for GPT-2)
```python
sentences = ["Short sentence
", "Now that's a longer sentence
"]
```
2. Tokenize them.
```python
tokenizer = AutoTokenizer.from_pretrained('gpt2')

tokenized_sentences = [tokenizer(sentence) for sentence in sentences]
```
3. Collate them with `DataCollatorForLanguageModeling`.
```python
tokenizer.pad_token = tokenizer.eos_token # 50256
data_collator = DataCollatorForLanguageModeling(tokenizer=tokenizer, mlm=False)
batch = data_collator(tokenized_sentences)
```

Batch is: 
```python
{'input_ids': tensor([[16438,  6827, 50256, 50256, 50256, 50256, 50256], 
                      [3844, 326, 338, 257, 2392, 6827, 50256]]), 
'attention_mask': tensor([[1, 1, 1, 0, 0, 0, 0], 
                          [1, 1, 1, 1, 1, 1, 1]]), 
'labels': tensor([[16438,  6827,  -100,  -100,  -100,  -100,  -100],
                  [3844, 326, 338, 257, 2392, 6827, -100]])}
```

Notice how even though `attention_mask` properly has `1` for the EOS tokens, they still got set to `-100`.

### Expected behavior

I'd expect that only the EOS tokens added as padding should be set to -100 in the labels, resulting in the following batch:
```python
{'input_ids': tensor([[16438,  6827, 50256, 50256, 50256, 50256, 50256], 
                      [3844, 326, 338, 257, 2392, 6827, 50256]]), 
'attention_mask': tensor([[1, 1, 1, 0, 0, 0, 0], 
                          [1, 1, 1, 1, 1, 1, 1]]), 
'labels': tensor([[16438,  6827,  50256,  -100,  -100,  -100,  -100],
                  [3844, 326, 338, 257, 2392, 6827, 50256]])}
```
(notice the `50256` in labels)

I wanted to fine-tune GPT-2 for a rather specific use-case with very short texts, so I've added EOS to my samples and it took me quite a bit of time to spot what was the issue, causing the model to fail at generating short texts. After I spotted it, as a workaround I just set labels manually and use the standard collator instead, which does what I wanted.

I feel like this is a simple change [here](https://github.com/huggingface/transformers/blob/v4.24.0/src/transformers/data/data_collator.py#L738), to use the attention mask instead of `[labels == self.tokenizer.pad_token_id]`. 
I'd like to make a PR with that, but I just want to make sure that this is indeed a bug, and not expected behaviour.

Comments:
You can do the change on your local fork for your example (or you could use a different token ID for the EOS and padding), but this is the right behavior in `DataCollatorForLanguageModeling`: labels corresponding to the pad token ID should be ignored in the loss computation and are thus set to -100.

Makes sense, but then perhaps that part of the course - setting `pad_token ` to `eos_token`  - might be misleading? 

Ran into a similar issue as @PatrykNeubauer. Although it might be the "right behavior", it is pretty easy to make a mistake if you want to have `
` be a valid predicted token at the end of your text and also follow the suggestion of setting `pad_token = eos_token`.

I think there might be a few fairly simple, valid approaches to solve this problem:
1. Do what @PatrykNeubauer suggested about using the attention mask. Presumably, if someone passes a pad token with a non-zero attention mask they did this themselves on purpose. I believe this wouldn't change any behavior when following the standard workflow.
2. If `labels` is already defined in the `examples` passed to `torch_call` use these labels for the non-padded `input_ids` in `examples`. Then for each padding token added to `input_ids` add -100 to the labels to pad in the same way.
3. Add an `add_eos_token` argument to `DataCollatorForLanguageModeling` which will add an `eos_token` to the end of final padded output with the correct eos token label. This way we can keep padding the same way but still allow `eos_token` to be added easily and treated correctly. I think outside of this current issue it would also be a nice addition to have an easy way to automatically add the eos token.

@sgugger as far as I can tell there isn't a great way to (1) do dynamic padding and (2) use a causal language model with an end-of-sequence token. Generally causal models don't have a padding token so we need to use another token. As pretty much every model has an end-of-sequence token it is a natural choice, but then we run into the above issue. I also tried using `DataCollatorWithPadding` but it seems to have an issue with including `labels`, which results in an error (I can give more details if you like).

I think the best approach that currently works is what you suggested, setting pad to a token other than eos, but this seems sort of hacky as we can't assume some piece of text will be a single token for all causal language model tokenizers. This method is also not mentioned in any tutorial or documentation as far as I can tell which makes it seem like this isn't often used and perhaps is thought to be inelegant. 

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.