Roberta Positional Embeddings Not Masked
Author: DomHudson
Pull request: no
Labels: name: wontfix

## Bug

Hi, I think for the RoBERTa model, the positional embeddings are created slightly wrong. In both this library and in `fairseq` the positional embeddings are sequential integers starting from `padding_idx + 1`. However, in `fairseq.utils.make_positions` all indicies of the input that correspond to the padding index are masked as they go into the positional embedding layer.

## Example

```python
import torch
padding_idx = 0

tensor = torch.as_tensor([
    [1, 13, 54, 0, 0],
    [1, 55, 12, 24, 0]
])
```

### In Fairseq

```
from fairseq import utils
utils.make_positions(tensor, padding_idx = padding_idx)
>>> tensor([
        [1, 2, 3, 0, 0],
        [1, 2, 3, 4, 0]
        ])
```

The positional sequence begins at padding_idx + 1 but all occurrences of the padding index in the input tensor are unmanipulated. This means the position embeddings are only adjusted for tokens which are not padding.

### In Transformers

This code comes from [modeling_roberta.py#L65](https://github.com/huggingface/transformers/blob/master/transformers/modeling_roberta.py#L65)
```python
# Position numbers begin at padding_idx+1. Padding symbols are ignored.
# cf. fairseq's `utils.make_positions`
position_ids = torch.arange(self.padding_idx+1, seq_length+self.padding_idx+1, dtype=torch.long, device=input_ids.device)
position_ids = position_ids.unsqueeze(0).expand_as(input_shape)
```

The output tensor of these function calls do not retain the masked tokens:
```
>>> tensor([
        [1, 2, 3, 4, 5],
        [1, 2, 3, 4, 5]
        ])
```
## Complete Example

```python

from fairseq import utils

tensor = torch.as_tensor([
    [1, 13, 54, 0, 0],
    [1, 55, 12, 24, 0]
])

def fairseq_make_positions(tensor, padding_idx):
    mask = tensor.ne(padding_idx).int()
    return (
        torch.cumsum(mask, dim=1).type_as(mask) * mask
    ).long() + padding_idx

def transformers_make_positions(input_ids, padding_idx):
    input_shape = input_ids.size()
    seq_length = input_shape[1]
    position_ids = torch.arange(padding_idx+1, seq_length+padding_idx+1, dtype=torch.long, device=input_ids.device)
    position_ids = position_ids.unsqueeze(0).expand(input_shape)
    return position_ids


print(fairseq_make_positions(tensor, padding_idx = 0))
print(transformers_make_positions(tensor, padding_idx = 0))
```
Output:
```
# Fairseq:
tensor([[1, 2, 3, 0, 0],
        [1, 2, 3, 4, 0]])
# Transformers:
tensor([[1, 2, 3, 4, 5],
        [1, 2, 3, 4, 5]])
```

## Expected behavior
I expect all padded tokens in the input tensor to be padded as they go into the positional embeddings layer as well as the token embeddings layer.

## Additional context

Happy to make a PR if this is agreed to be a bug, but this will mean the positional embeddings train slightly differently to the existing `RobertaEmbeddings` module.


Comments:
I think you are right. Awesome if you can fix this in a PR.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.


Closed in #1764