Support for -1 for pad token id to follow Metas LLama implementation
Author: mallorbc
Pull request: no
Labels: name: wontfix

name: Feature request

### System Info

- `transformers` version: 4.31.0
- Platform: Linux-5.15.0-79-generic-x86_64-with-glibc2.31
- Python version: 3.9.17
- Huggingface_hub version: 0.16.4
- Safetensors version: 0.3.2
- Accelerate version: 0.21.0
- Accelerate config: 	- compute_environment: LOCAL_MACHINE
	- distributed_type: MULTI_GPU
	- mixed_precision: bf16
	- use_cpu: False
	- num_processes: 2
	- machine_rank: 0
	- num_machines: 1
	- gpu_ids: all
	- rdzv_backend: static
	- same_network: True
	- main_training_function: main
	- downcast_bf16: no
	- tpu_use_cluster: False
	- tpu_use_sudo: False
- PyTorch version (GPU?): 2.0.1+cu117 (True)
- Tensorflow version (GPU?): not installed (NA)
- Flax version (CPU?/GPU?/TPU?): not installed (NA)
- Jax version: not installed
- JaxLib version: not installed
- Using GPU in script?: Yes
- Using distributed or parallel set-up in script?: NA

### Who can help?

@Arthur

### Information

- [ ] 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

Load the LLama 2 tokenizer

```python

tokenizer = AutoTokenizer.from_pretrained("meta-llama/Llama-2-7b-chat-hf", token=access_token,trust_remote_code=args.trust_remote_code,add_eos_token=True,use_fast=True)
```

Notice that there is no padding token.  By default that means that many programs will go ahead and use the EOS token for the padding token unless one is added.

```python
print(tokenizer.pad_token_id)
```

Notice that in the llama repo, they set the padding token id to -1

https://github.com/facebookresearch/llama/blob/1a240688810f8036049e8da36b073f63d2ac552c/llama/tokenizer.py#L14-L25

Try setting the pad token to -1 to replicate how Meta trained their model.

```python
tokenizer.pad_token_id = -1
```

You will get an error like

```
IndexError: piece id is out of range.
```

### Expected behavior

I would expect this to work. Padding tokens are only used to determine what the attention mask does.  If you want to finetune the chat Llama model, using EOS is not a working solution.

 The only other alternative is to either choose a token that you will never use(probably) in your dataset, such as *** which is 18610, or add a new token and expand the model token embedding.  While these may work, they are not as elegant and may have issues when trying to use other frameworks that expect a certain embedding size, or when using things like LORA.

Comments:
Hey! Thanks for the input. We were also discussing wether or not we can do this in `transformers` in #25088. 
The real issue is the the `Llama` families do not have a `padding_token` and just a `pad_id`. Padding with a negative index works sure, but we can't add this to `tokenizers` for starters, but it is also not the way our tokenizers work. 

None of this would have happened if they had a padding tokens 😅 

Hey @ArthurZucker - I see that #25088 was merged. What ended up being the resolution for this? It's not clear from that PR if we should be using -1 for the padding token or doing something else. Thanks!

You should never use -1 as it is a bad industry practice. Not supported on a lot of libs, confusing and does not represent any string. The solution is to overcome vocab size limit by correctly padding the model to the closest SM of your architecture! (thus allowing token addition without performance loss. 
Closing as not planned BTW

Sorry, what is SM?

https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#hardware-implementation Streaming Multiprocessors 😉 

ty!

While I agree that you should not use -1 as a token id in ANY case, I think setting pad_id = -1 in the Llama3 tokenizer is an EQUALLY bad idea, as it will lead code that relies on pad_id to fail without explanation.

It seems like the "solution" is to pad with an arbitrary token. But this again bad design, because why should I use a token that means something as a padding token that isn't a padding token? 

There SHOULD be a pad token, even if it's set to 0 in all dimensions and never trained. Incidentally, PyTorch allows this in its implementation of Embedding, and this is what should have been done.

agreeing with you on this

So what is the current solution to this? 

I see in the TRL / SFT trainer documentation: 

"Make sure to have a pad_token_id which is different from eos_token_id which can result in the model not properly predicting EOS (End of Sentence) tokens during generation."

In most notebooks i've seen theses have been set to the same value.

Well, torch.nn.Embedding needs this:
```
        if padding_idx is not None:
            if padding_idx > 0:
                assert padding_idx < self.num_embeddings, 'Padding_idx must be within num_embeddings'
            elif padding_idx < 0:
                assert padding_idx >= -self.num_embeddings, 'Padding_idx must be within num_embeddings'
                padding_idx = self.num_embeddings + padding_idx
```
so actuallly setting to -1, will set it to `len(embedding)-1`:

```python
import torch
emb = torch.nn.Embedding(20,20, -1)
Embedding(20, 20, padding_idx=19)
```

this is terrible because if you resize, `-1` is no longer `len(embedding)-1` and etc.
You cannot use `21`. 

The solution is for people who train to leave a token `<unused>` or `<reserved>` to make sure that we can use one that already exists