MBART tokenizer not behaving as example
Author: BramVanroy
Pull request: no
Labels: name: bug

### System Info

- `transformers` version: 4.21.3
- Platform: Windows-10-10.0.19041-SP0
- Python version: 3.9.2
- Huggingface_hub version: 0.9.1
- PyTorch version (GPU?): 1.12.1+cu113 (True) 

### Who can help?

@patil-suraj @SaulLu

### Information

- [X] The official example scripts
- [ ] 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

I was looking at the [MBARTTokenizer](https://huggingface.co/docs/transformers/model_doc/mbart#transformers.MBartTokenizer.example) example on the website.

```python
from transformers import MBartTokenizer

tokenizer = MBartTokenizer.from_pretrained("facebook/mbart-large-en-ro", src_lang="en_XX", tgt_lang="ro_RO")
example_english_phrase = " UN Chief Says There Is No Military Solution in Syria"
expected_translation_romanian = "Şeful ONU declară că nu există o soluţie militară în Siria"
inputs = tokenizer(example_english_phrase, return_tensors="pt")
with tokenizer.as_target_tokenizer():
    labels = tokenizer(expected_translation_romanian, return_tensors="pt")
inputs["labels"] = labels["input_ids"]
```

The documentation states that the format of the input and output should be different:

> The tokenization method is `<tokens> <eos> <language code>` for source language documents, and `<language code> <tokens> <eos>` for target language documents.

but In practice, that is not the case. Both the inputs and labels are structured `[tokens] EOS LID`. 

```python
tokenizer.batch_decode(inputs["input_ids"])
# ['UN Chief Says There Is No Military Solution in Syria
en_XX']
tokenizer.batch_decode(inputs["labels"])
# ['Şeful ONU declară că nu există o soluţie militară în Siria
ro_RO']
```

I assume that this happens because the shifting-right is supposed to happen in the data collator, and so not present here. But 1. then the documentation is confusing; 2. why do we need `tokenizer.as_target_tokenizer()` then?

### Expected behavior

Either a change in documentation of the expected output of the example (and an explanation why we'd still need `tokenizer.as_target_tokenizer()`), or a change in implementation where the target tokenizer also deals with shifting right. 

**Additional question**: why does the English phrase in the example start with a space (and the translation does not). Is that a requirement? 

Comments:
The example [here](https://huggingface.co/docs/transformers/main/model_doc/mbart#training-of-mbart) also does not seem to work as intended. It yields

```Keyword arguments {'text_target': 'Şeful ONU declară că nu există o soluţie militară în Siria'} not recognized.```

and `input_ids` only contains the source text.

Hi @BramVanroy,

> I was looking at the [MBARTTokenizer](https://huggingface.co/docs/transformers/model_doc/mbart#transformers.MBartTokenizer.example) example on the website. [...]
Either a change in documentation of the expected output of the example (and an explanation why we'd still need tokenizer.as_target_tokenizer()), or a change in implementation where the target tokenizer also deals with shifting right.

Thank you very much for reporting this inconsistency, I confirm that there is indeed a misalignment between the documentation and the implementation about where to put the special tokens in the source and target texts. I am unfortunately not familiar with this model to know which of the 2 is right. @patil-suraj , would you remember what is the correct way to format the text for Mbart?

> The example [here](https://huggingface.co/docs/transformers/main/model_doc/mbart#training-of-mbart) also does not seem to work as intended. It yields
Keyword arguments {'text_target': 'Şeful ONU declară că nu există o soluţie militară în Siria'} not recognized.
and input_ids only contains the source text.

Indeed this second example does not behave as we would expect. We'll have to explore this more closely (I'm waiting for suraj's feedback on the first problem to see if I can help on this second point).

Hi @SaulLu @patil-suraj. Any update to this, one month later? Thanks!

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.

Not stale. Still waiting for update from @SaulLu or @patil-suraj.

They are both not working on Transformers so you will have to wait a very long time ;-)

The doc example with `text_target` should work on the latest version of Transformers, we have migrated the API. As for what should happen, I think we should just adapt the documentation to what the tokenizer is actually doing, since we won't really change that behavior to avoid a breaking change.

Would you like to make a PR with the changes?

Apologies if this is the wrong place to put it, but I have been having a similar problem with 
`Keyword arguments {'text_target': 'Par défaut, développer les fils de discussion'} not recognized.
{'input_ids': [47591, 12, 9842, 19634, 9, 0], 'attention_mask': [1, 1, 1, 1, 1, 1]}`
It is happening in your course / 'main nlp tasks' / translation.
Just copy / paste your code until (win11/wsl2/vscode):
`en_sentence = split_datasets["train"][1]["translation"]["en"]`
`fr_sentence = split_datasets["train"][1]["translation"]["fr"]`
`inputs = tokenizer(en_sentence, text_target=fr_sentence)`
`inputs`
The problem: the error and the missing targets
It does work in provided colab NB. Any suggestion on how to deal with it is much appreciated.

ps. I was able to solve it with workaround
`# Setup the tokenizer for targets`
`with tokenizer.as_target_tokenizer():`
        `labels = tokenizer(targets, max_length=max_length, truncation=True)`
 in preprocessing func. You mention it in the video but not in the write up.

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.

cc @ArthurZucker 

Bumping, because @ArthurZucker self-assigned this.

Thanks, will adresse this!
Most probably think that we will update the documentation as the shift right not included here has been pretty confusing

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.

Bump,  because it seems Arthur has self-assigned it.

Hey, I think I answered this question in #20931. Also my previous answer is still valid, the documentation should just be updated to add a tip about the fact that the target is shifted inside the modelling file! 



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.