Deprecate `clean_up_tokenization_spaces` for BLOOM
Author: thomasw21
Pull request: yes

# What does this PR do?

Currently in `transformers`:
```python
>>> tok.decode(tok.encode("Hello , there"))
'Hello, there' # notice the missing space between "Hello" and ","
>>> tok.decode(tok.encode("Hello , there"), clean_up_tokenization_spaces=False)
'Hello , there'
```

In order too prevent issues such as this: https://huggingface.co/bigscience/bloom/discussions/153#6397907b71eb2455d898e0a4 we suggest to add a warning, suggesting to users to use `clean_up_tokenization_spaces=False` instead.

As BLOOM tokenizer was developped in order to be lossless encoding mechanism, it should make sense to always remove that option IMO, therefore I'm suggesting to deprecate that option from BLOOM tokenizer. Other option would be to change the default to `True`.


Comments:
The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/transformers/pr_20846). All of your documentation changes will be reflected on that endpoint.

Thanks for your PR! After thinking a little more about it and in terms of user experience, I'm happy to have the warning if you think the use-case is frequent and the default behavior is misleading. 

However, I'm not too sure about deprecating/updating the value in v5. I think the current behavior isn't necessarily a bug, as the argument to toggle is clearly displayed in the docs (and I have no problem with making it more prominent, such as with the warning). Switching to `False` means that we'll start diverging between BLOOM and other tokenizers (like GPT-2) which work very similarly as of now.

I'd be in favor of adding the warning mentioning to toggle it in this PR, and to wait until @sgugger is back so that we have a second opinion on the matter before mentioning that we will move it to `False` by default. Would that be ok for you @thomasw21?

@LysandreJik Sure! This isn't blocking anything really, the real issue is here: https://github.com/huggingface/text-generation-inference/issues/12 

IMO as the tokenizer was build to be lossless, it's weird that by default it isn't. Would it make more sense to move `clean_up_tokenization_spaces` to be in `tokenizer` instead? Something like a special decoder? https://huggingface.co/docs/tokenizers/components#decoders . I understand that this is breaking, but we should be able to slightly migrate to newer setups using deprecation cycles?

Interesting proposal, WDYT @Narsil?

I think it's ok to move slowly, but touching `cleanup_tokenization_spaces` and its default are BIG changes.

Personally, I think borderline too big to migrate in V5 (it's just a really big change, that's unfortunately probably not worth the effort). 

That being said, making it modifiable on a tokenizer per tokenizer basis (so updating Bloom alone) is still Ok, and is definitely a good way forward.

Personally I would focus on this user's need first, which would be solved by implementing `return_full_text=False`, it seems the lowest hanging fruit to solve the user's need. We can move forward on the "decoder"  (or any other type of config change) later. 



Okay so in terms of actions:
 - [x] Assume it's not a `transformers` bug but a `text-generation-inference` bug right now. https://github.com/huggingface/text-generation-inference/issues/12
 - [ ] Start thinking of a way to support `clean_up_tokenization_spaces` and `skip_special_tokens` in the tokenizer directly? Typically you want in order of priority: `user defined argument`, `tokenizer specific config`, `methods default`
 
 Would that make sense?

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.