Tokenizer failing to encode chatml correctly
Author: ozreact
Pull request: no

### System Info

- `transformers` version: 4.31.0
- Platform: Linux-5.14.0-284.18.1.el9_2.x86_64-x86_64-with-glibc2.34
- Python version: 3.10.12
- Huggingface_hub version: 0.16.4
- Safetensors version: 0.3.1
- Accelerate version: 0.21.0
- Accelerate config: 	not found
- PyTorch version (GPU?): 2.0.1+cu118 (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?: No

Note: also tested and broken on:

- 641adca
- 4.30.2
- 4.30.1
- 4.30.0
- 4.29.2
- 4.29.1
- 4.29.0
- 4.28.1
- 4.28.0
- 4.27.4

### Who can help?

@ArthurZucker @younesbelkada

### 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

I'm attempting to finetune Llama2 with a ChatML format.  No matter how I approach it, it seems to be failing to encode/decode correctly.  I see multiple issues and PRs that are related, but this specific format seems to be hitting all of them with none of the workarounds being effective.

A repro is available here:

https://gist.github.com/ozreact/a4b565cd2c7fac65d6cb76c78dbdf9e2

#24565 recommends setting `legacy=false`, and further says that this only addresses a subset of issues with the slow tokenizer only.  It also mentions that `decode` isn't fixed, so validating that the encoding step is working is fiddly.

This format, when newlines are used, is also impacted by #21120.

#25073 also breaks this.

#25176 recommends setting `legacy=True` to fix an invalid unk token that effectively over-writes a final token in a partial ChatML response, but this conflicts with attempting to fix the issues in #24565.

### Expected behavior

ChatML instruction format should 'just work', tokenize correctly, and decode correctly.

Comments:
Hi ! Could you demonstrate this issue with a small but complete code snippet. That will help us a lot, thanks in advance.

Yep, I had it linked above.

https://gist.github.com/ozreact/a4b565cd2c7fac65d6cb76c78dbdf9e2

Just replace the model path.

OK, thank you. I thought it was a full repository 😅 

You can also try #25224, should fix it (deals with extra space, unk and decoding extra space)

Gave #25224 a shot.  The slow tokenizer still outputs spaces around special tokens.  The fast tokenizer is pretty close:

```
# IN

user Hello world

assistant
# OUT

 user Hello world

 assistant
```

Looks like it still wants to emit a space after a BOS token.  I _think_ this may be expected behavior?

Edit: Also seeing odd behavior with newlines:

```
# IN

 user Hello world
\n
 assistant\n
# OUT

 user Hello world
 \n
 assistant\n
```

It seems that you are not using `spaces_between_special_tokens`, the following is what I got for `use_fast=True` 

```python 
>>> tokenizer.decode(tokenized["input_ids"], spaces_between_special_tokens = False)
'
 user Hello world

 assistant Hello user
'
```
So this solves part of the space issue. This argument is set to `True` by default. 

`spaces_between_special_tokens` brings it closer, I get the same results as you for an example string with no newlines.

The newlines still throw it off, e.g.:

```
'
system\nYou a AI
\n
user\nHi
\n
assistant\nHello
'
'
 system\nYou a AI
 \n
 user\nHi
 \n
 assistant\nHello
'
```

obtained via:

```
>>> type(t)
<class 'transformers.models.llama.tokenization_llama_fast.LlamaTokenizerFast'>
>>> cml
'
system\nYou a AI
\n
user\nHi
\n
assistant\nHello
'
>>> tokenized = t(cml, add_special_tokens=False)
>>> tokenized
{'input_ids': [32000, 1788, 13, 3492, 263, 319, 29902, 32001, 29871, 13, 32000, 1404, 13, 18567, 32001, 29871, 13, 32000, 20255, 13, 10994, 32001], 'attention_mask': [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]}
>>> t.decode(tokenized["input_ids"], spaces_between_special_tokens = False)
'
 system\nYou a AI
 \n
 user\nHi
 \n
 assistant\nHello
'
```

I've somewhat solved this by manipulating the output.  These two `replace` calls result in the encode/decode for all my test cases to pass (absent `spaces_between_special_tokens`):

```
def decode_to_str(self, tokenized: BatchEncoding) -> str:
        ret = self.tokenizer.decode(tokenized["input_ids"])
        ret = ret.replace("
 ", "
")
        ret = ret.replace("
 \n", "
\n")
        return ret
```

It would ofc still be ideal for a somewhat-naive encode to == decode.

Not sure if decode is also very important when you are training you compare the genereated ids.
But yes decoding adds spaces. Will try to adresse this 😉 

> 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.

this issue still needs to be addressed 

This was fixed on main for `transformers`: with `meta-llama/Llama-2-7b-hf` and `use_fast=False, legacy=False` I am getting the correct decoded output.
For `tokenizers`, it's a different issue that is gonna take more time to get fixed

> This was fixed on main for `transformers`: with `meta-llama/Llama-2-7b-hf` and `use_fast=False, legacy=False` I am getting the correct decoded output. For `tokenizers`, it's a different issue that is gonna take more time to get fixed

So to clarify, we must use_fast=False and legacy=False for it to work?

For decoding yes, for encoding fast and slow should give the same results 

I also recommend you to use the new ChatTemplating introduced in #25323

@winglian please see for axolotl ^

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.

BTW we'll add the `add_prefix_space` to the tokenizer config and encode call to easily de-activate this behavior.
Other fix is also here: #26678 it will no longer add random spaces if you tokenize a full sequence.