A question about shift_tokens_right in BART model
Author: liuslnlp
Pull request: no

## Environment info
<!-- You can run the command `transformers-cli env` and copy-and-paste its output below.
     Don't forget to fill out the missing fields in that output! -->
     
- `transformers` version: 3.10
- Platform: Ubuntu 16.04
- Python version: 3.7.3
- PyTorch version (GPU?): 1.6 GPU-version

### Who can help
@TevenLeScao @sshleifer
<!-- Your issue will be replied to more quickly if you can figure out the right person to tag with @
 If you know how to use git blame, that is the easiest way, otherwise, here is a rough guide of **who to tag**.
 Please tag fewer than 3 people.
 
 albert, bert, GPT2, XLM: @LysandreJik 
 tokenizers: @mfuntowicz
 Trainer: @sgugger
 Speed and Memory Benchmarks: @patrickvonplaten
 Model Cards: @julien-c
 Translation: @sshleifer
 Summarization: @sshleifer
 TextGeneration: @TevenLeScao 
 examples/distillation: @VictorSanh
 nlp datasets: [different repo](https://github.com/huggingface/nlp)
 rust tokenizers: [different repo](https://github.com/huggingface/tokenizers)
 Text Generation: @TevenLeScao
 blenderbot: @mariamabarham
 Bart: @sshleifer
 Marian: @sshleifer
 T5: @patrickvonplaten
 Longformer/Reformer: @patrickvonplaten
 TransfoXL/XLNet: @TevenLeScao 
 examples/seq2seq: @sshleifer
 examples/bert-loses-patience: @JetRunner
 tensorflow: @jplu
 examples/token-classification: @stefan-it
 documentation: @sgugger
 -->

## Information
Generally speaking, the decoder input of seq2seq model is `<sos> tok1 tok2 … tokn` and the target is `tok1 tok2 … tokn <eos>` (shift right). But in BART, the `shift_tokens_right` function will produce the following result:

decoder input: `<eos><sos> tok1 tok2 … tokn`
target:`<sos> tok1 tok2 … tokn <eos>`

Is this a bug or correct?

## To reproduce

```python
tokenizer = BartTokenizer.from_pretrained(model_path)
model = BartForConditionalGeneration.from_pretrained(model_path)

inputs = tokenizer.prepare_seq2seq_batch(
    src_texts=['good morning.', ],
    tgt_texts=['good bye.'],
    max_length=100, return_tensors='pt'
)
# This function is copied from modeling_bart.py
def shift_tokens_right(input_ids, pad_token_id):
    """Shift input ids one token to the right, and wrap the last non pad token (usually <eos>)."""
    prev_output_tokens = input_ids.clone()
    index_of_eos = (input_ids.ne(pad_token_id).sum(dim=1) - 1).unsqueeze(-1)
    prev_output_tokens[:, 0] = input_ids.gather(1, index_of_eos).squeeze()
    prev_output_tokens[:, 1:] = input_ids[:, :-1]
    return prev_output_tokens
tgt = inputs['labels'][0].tolist()
decoder_inputs = shift_tokens_right(inputs['labels'], tokenizer.pad_token_id)[0].tolist()
print("decoder inputs:", tokenizer.decode(decoder_inputs))
print("target:", tokenizer.decode(tgt))
```
The expected output is 
```
decoder inputs: 
good bye.
target: good bye.

```
but it actually outputs 
```
decoder inputs: 

good bye.
target: 
good bye.

```


Comments:
It's correct, copied from fairseq (authors code).
you can think of `shift_tokens_right` as `shift_tokens_right_and_wrap_eos_to_position0`.
If you have empirical evidence that there is a change that improves fine-tuning, I'd be happy to incorporate it.


After I updated my version for this one, I'm also confused by the decoder input. 

With this new encoding,  after finetuning, bart-large is outputting:

Example 1:

```
"labels": "
 There are too many traitors among our compatriots,
<pad><pad><pad>",
        "decoder_input_ids": "

 There are too many traitors among our compatriots,
<pad><pad>",
        "generated_ids": "































































































































































































































































































































































































"
```

Example 2:

```
"labels": "
 Freedom of speech is relative. We need take national conditions into account.
",
        "decoder_input_ids": "

 Freedom of speech is relative. We need take national conditions into account.",
        "generated_ids": "































































































































































































































































































































































































"
```



I'm trying to figure out why.

 I'm using the code provided by the examples/seq2seq folder. T5 works fine, but bart does not.

@leoribeiro if you are reporting a bug, could you explain what you did and what you expected more clearly in a separate issue?
Otherwise, I don't understand why you say the encoding is new. `shift_tokens_right` hasn't changed.

@sshleifer thank you for your reply. What I mean by new encoding is adding `
` at the beginning of the decoder inputs. I think that in a previous transformer version (2.11.0), the code for BART did not use `
` at the beginning of the decoder inputs, correct? In the 2.11.0 version, my experiments with `facebook/bart-large` were working with  the following code:

```
    def _step(self, batch):
        pad_token_id = self.tokenizer.pad_token_id
        source_ids, source_mask, y = batch["source_ids"], batch["source_mask"], batch["target_ids"]
        y_ids = y[:, :-1].contiguous()
        lm_labels = y[:, 1:].clone()
        lm_labels[y[:, 1:] == pad_token_id] = -100
        outputs = self(source_ids, attention_mask=source_mask, decoder_input_ids=y_ids, lm_labels=lm_labels,)

        loss = outputs[0]

        return loss
```

But now, with the following code, my experiments with `facebook/bart-large` are not working:
```
    def _step(self, batch: dict) -> Tuple:
        pad_token_id = self.tokenizer.pad_token_id
        src_ids, src_mask = batch["input_ids"], batch["attention_mask"]
        tgt_ids = batch["labels"]
        if isinstance(self.model, T5ForConditionalGeneration):
            decoder_input_ids = self.model._shift_right(tgt_ids)
        else:
            decoder_input_ids = shift_tokens_right(tgt_ids, pad_token_id)
        if not self.already_saved_batch:  # This would be slightly better if it only happened on rank zero
            batch["decoder_input_ids"] = decoder_input_ids
            self.save_readable_batch(batch)
        outputs = self(src_ids, attention_mask=src_mask, decoder_input_ids=decoder_input_ids, use_cache=False)
        lm_logits = outputs[0]
        if self.hparams.label_smoothing == 0:
            # Same behavior as modeling_bart.py, besides ignoring pad_token_id
            ce_loss_fct = torch.nn.CrossEntropyLoss(ignore_index=pad_token_id)

            assert lm_logits.shape[-1] == self.vocab_size
            loss = ce_loss_fct(lm_logits.view(-1, lm_logits.shape[-1]), tgt_ids.view(-1))
        else:
            lprobs = torch.nn.functional.log_softmax(lm_logits, dim=-1)
            loss, nll_loss = label_smoothed_nll_loss(
                lprobs, tgt_ids, self.hparams.label_smoothing, ignore_index=pad_token_id
            )
        return (loss,)
```

I'm trying to understand if those things are related. The weird thing is that the exact same code works for `facebook/bart-base`.  Please, see #8005. 

Moved to #8005, problem seems to be config related.