Bug when computing positional IDs from embeddings

[SabrinaRichter]

System Info

transformers/src/transformers/models/esm/modeling_esm.py

Line 243 in c0f8d05

self.padding_idx + 1, sequence_length + self.padding_idx + 1, dtype=torch.long, device=inputs_embeds.device

I think it should be 1, sequence_length + 1, dtype=torch.long, device=inputs_embeds.device
I don't see how the index of my padding token in the alphabet has anything to do with my positions in the sequence

Who can help?

No response

Information

• The official example scripts
• My own modified scripts

Tasks

• An officially supported task in the examples folder (such as GLUE/SQuAD, ...)
• My own task or dataset (give details below)

Reproduction

Expected behavior

[Rocketknight1]

Hi @SabrinaRichter, I actually had exactly the same thought as you when I first saw this! It seems obviously incorrect, but if you search the codebase, this line actually originates in the code for RoBERTa, and lots of people (including the ESM authors) have just been copying it ever since.

The reason, I think, was that RoBERTa (the original FAIR implementation) stored the special token embeddings and the position embeddings in the same tensor, which meant that the position embeddings started at the index after the final special token (which was the padding token). Therefore, the position ID for position x was x + self.padding_idx + 1. Of course, this is no longer the case for most models. but the offset persists because so many existing models have been trained with it.

Because of those pretrained checkpoints, we can't really "fix" this! We could consider a PR to replace self.padding_idx with a constant for some models, but the problem is that this would then break any models that used that code to train with a different self.padding_idx. 🙃

[github-actions]

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 are likely to be ignored.