mBart prefix and suffix for language id
## Environment info
- `transformers` version: 3.5.1

### Who can help
 mBART: @patrickvonplaten
 documentation: @sgugger

## Information

It seems that there is error or inconsistency between [mbart tokenizer](https://github.com/huggingface/transformers/blob/55e8d0cea25be18b044523b30f4bef58fec63289/src/transformers/tokenization_mbart.py) and comments/docs inline the code. 

comments on code explain
```
def build_inputs_with_special_tokens(
        self, token_ids_0: List[int], token_ids_1: Optional[List[int]] = None
    ) -> List[int]:
        """
        Build model inputs from a sequence or a pair of sequence for sequence classification tasks by concatenating and
        adding special tokens. An MBART sequence has the following format, where ``X`` represents the sequence:
        - ``input_ids`` (for encoder) ``X [eos, src_lang_code]``
        - ``decoder_input_ids``: (for decoder) ``[tgt_lang_code] X [eos]``
```

but `set_tgt_lang_special_tokens`  considers that `decoder_input_ids` is **X [eos][tgt_lang_code]** instead of **[tgt_lang_code] X [eos]**

```
def set_tgt_lang_special_tokens(self, lang: str) -> None:
        """Reset the special tokens to the target language setting. Prefix [tgt_lang_code], suffix =[eos]."""
        self.cur_lang_code = self.lang_code_to_id[lang]
        self.prefix_tokens = []
        self.suffix_tokens = [self.eos_token_id, self.cur_lang_code]
```

Shouldn't it be:
```
def set_tgt_lang_special_tokens(self, lang: str) -> None:
        """Reset the special tokens to the target language setting. Prefix [tgt_lang_code], suffix =[eos]."""
        self.cur_lang_code = self.lang_code_to_id[lang]
        self.prefix_tokens = [self.cur_lang_code]
        self.suffix_tokens = [self.eos_token_id]
```




doublechecking the paper, it is expected that the language id as suffix so there is not a bug in the code but a error in the inline documentation.<|||||>Hey @RQuispeC - thanks a lot for checking! Do you feel like opening a PR to fix the doc? That would be super helpful :-)<|||||>Just added the PR @patrickvonplaten  :) <|||||>Hi, I've ran into the same issue - but it seems that the documentation was correct, the target language tag should be the bos (and maybe also eos?). 
eos is set to be the target language id here: https://github.com/pytorch/fairseq/blob/c8a0659be5cdc15caa102d5bbf72b872567c4859/fairseq/tasks/translation_from_pretrained_bart.py#L116
but then for inference, it is also used as bos by default, as far as I can tell:
https://github.com/pytorch/fairseq/blob/c8a0659be5cdc15caa102d5bbf72b872567c4859/fairseq/sequence_generator.py#L173
The paper states "A language id symbol <LID> is used as the initial token to predict the sentence. "
I get very strange predictions with the pretrained mBART model unless I set "decoder_start_token_id" to the target language id in model.generate().<|||||>Hi, I didn't check FAIR's code but if you check the figure 1 of the paper ("Multilingual Denoising Pre-Training (mBART)") you can see that the language id is at:
* eos of input (source lang text)
* bos of decoder (I guess this is what author's mean with "A language id symbol is used as the initial token to predict the sentence. ")
* eos of output (target lang text)

The documentation refers to input and output, but not the decoder . `decoder_start_token_id` sets the decoder input symbol so it's expected that not using it gives weird results.
That's what I understand, Am I missing something?<|||||>The tokenizer produces `X [eos, tgt_lang_code]` for the target text, but to be consistent with the pretrained model, it should be `[tgt_lang_code] X [tgt_lang_code] `  (refering to the situation where you have a target side text)? `decoder_start_token_id` is only for inference, not for training, right?
 At least that's what I understand from the paper/fairseq code.<|||||>`decoder_start_token_id` is used during training and testing, it's the first token of the decoder, I found this easier to understand with the figure 1 of the paper.

Where did you get the input used for the pretrained model?<|||||>I don't have acces to the input used for pretraining (that would be so much easier :) ), I'm just trying to understand the format from the paper and code. 
Maybe a stupid question, but how can I set `decoder_start_token_id` for training (in the forward pass of the module)? I could only find it as an argument of generate() (I'm using version 3.1.0).
Also I've noticed that the models seems to have problems in inference if I replace `eos` with the language token on the target side (it randomly stops generating mid sentence), so I guess `eos` has to be present for generate() to function properly (I did set `eos_token_id` to the language tag)? 
<|||||>Not stupit question :)
You can find an example of you to set source and target languages here 
https://github.com/huggingface/transformers/blob/master/examples/seq2seq/finetune_trainer.py#L222
```
        model.config.decoder_start_token_id = tokenizer.lang_code_to_id[data_args.tgt_lang]
```
Not sure if it's available in version 3.1, I was using version 3.5

The caveat is that (at least in up to version 3.5) the hugging face implementation only allows 1 language  for all your data in source and 1 language for all your data in the target, hence `decoder_start_token_id`, `tgt_lang_code` and `src_lang_code` are just one string. 
Depending in your problem this may be fine. For instance, if you are working on a translation then you are good to go with hugging face implementation because for translation you usually aim to translate one language (e.g. english) to another (e.g. spanish).

In my case I needed something more generic. I was working on a project that needed to train multiple languages at the same time, so my training data looked something like:

* source

| text | lang |
|:--------- | ----------: |
| my source text 1 | en_XX | 
| my source text 2 | es_XX | 
| my source text 3 | fr_FR | 

* target


| text | lang |
|:--------- | ----------: |
| my target text 1 | en_XX|
| my target text 2 | es_XX|
| my target text 3 | fr_FR|

In this case `decoder_start_token_id`, `tgt_lang_code` and `src_lang_code` should be `['en_XX', 'es_XX', 'fr_FR']` but this is not supported by hugging face. I implemented a custom version of `MBartForConditionalGeneration` and `MBartTokenizer`.

About the `eos` I have just used it as in the seq2seq example so I'm not sure about the behavior you mention.



<|||||>Thank you for the explanations! There are major differences to the (old) version I'm using, so I think the best way for me is to update my code to work with the newest huggingface version. Hopefully, that will solve the problem :) But just in case, where would I find your custom versions of `MBartForConditionalGeneration` and `MBartTokenizer` ?<|||||>sorry for answering so late,  I was really busy at work.

I'm not sure if can open source it at this time but the main ideas was to check the flow of the parameters `tgt_lang_code` and `src_lang_code` then update the function calls to support vectors of strings instead of only strings.<|||||>I find this problem too
```
class MyMBartTokenizer(MBartTokenizer):
    def set_tgt_lang_special_tokens(self, lang: str) -> None:
        """Reset the special tokens to the target language setting. No prefix and suffix=[eos, tgt_lang_code]."""
        self.cur_lang_code = self.lang_code_to_id[lang]
        self.prefix_tokens = [self.cur_lang_code]
        self.suffix_tokens = [self.eos_token_id]
```