Support various BERT relative position embeddings (2nd)
Author: zhiheng-huang
Pull request: yes
Labels: name: model card

# What does this PR do?

Creating a new PR for https://github.com/huggingface/transformers/pull/8108 to keep cleaner git history/commits.

The default BERT model `bert-base-uncased` was pre-trained with absolute position embeddings. We provide three pre-trained models which were pre-trained on the same training data (BooksCorpus and English Wikipedia) as in the BERT model training, but with different relative position embeddings (Shaw et al., Self-Attention with Relative Position Representations, https://arxiv.org/abs/1803.02155 and Huang et al., Improve Transformer Models with Better Relative Position Embeddings, https://arxiv.org/abs/2009.13658, accepted in findings of EMNLP 2020). We show how to fine-tune these pre-trained models on SQuAD1.1 data set. Our proposed relative position embedding method can boost the BERT base model (with default absolute position embedding) from f1 score of 88.52 to 90.54 with similar training/inference speed. It also boosts the `bert-large-uncased-whole-word-masking` model from 93.15 to 93.52 with 3 additional fine-tune epochs. See examples/question-answering/README.md for more details.

Fixes # (issue)

#8108

## Before submitting
- [ ] This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
- [X] Did you read the [contributor guideline](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md#start-contributing-pull-requests),
      Pull Request section?
- [ ] Was this discussed/approved via a Github issue or the [forum](https://discuss.huggingface.co/)? Please add a link
      to the it if that's the case.
- [ ] Did you make sure to update the documentation with your changes? Here are the
      [documentation guidelines](https://github.com/huggingface/transformers/tree/master/docs), and
      [here are tips on formatting docstrings](https://github.com/huggingface/transformers/tree/master/docs#writing-source-documentation).
- [x] Did you write any new necessary tests?


## Who can review?

@patrickvonplaten @LysandreJik @julien-c 


Comments:
Hey @zhiheng-huang,

it would be great if you could take a look at the failing tests :-) 

Hey @patrickvonplaten, I fixed all failed tests except check_code_quality. Currently the relative embedding is implemented for BERT only. In check_code_quality, `utils/check_copies.py` tries to copy the relative embedding implementation from BERT model to other models including `albert`, `electra`, `roberta` etc. I understand this may make the relative embedding methods ready to be used in those models. However, we haven't pre-trained those type of models with relative embedding and thus cannot assess their effectiveness. Please advise if I should fix this failing test (by ensuring relative embedding implementation copied to those BERT variants) or leave it as is. 

Hey @zhiheng-huang, 

Sadly there is still a problem with the git commit history. As you can see 54 files are changed in this PR. Could you make sure to keep the commit tree clean. It is not really possible to review the PR otherwise :-/ 

Try to make use of `git rebase` to avoid appending the master's commit history to your branch maybe

In the worst case, you can just make the changes to the files you intend to change without  `rebasing` or `merging` and then I can review and merge/rebase for you. 

Rebased and removed the unintended merge commit. @patrickvonplaten, can you comment on the `utils/check_copies.py` question so we can move forward?

Hi @patrickvonplaten @LysandreJik, I see one approval already, is it ready to merge? If not, can you point to the embedding (for example absolute position embedding) unit tests so I can try to come up with similar tests?

Regarding tests, I think adding integration tests in the `test_modeling_bert.py` would be nice. What do you think @patrickvonplaten?

The BERT model doesn't have any such tests right now, but you can take inspiration from the `RobertaModelIntegrationTest` class in `test_modeling_roberta.py`, which you can find [here](https://github.com/huggingface/transformers/blob/master/tests/test_modeling_roberta.py#L402).

You could add a couple of tests, each testing that you get the expected results: this will ensure the implementation will not diverge in the future. If you need a checkpoint, you can use `lysandre/tiny-bert-random`, which is a very small model (with random values), so it will be very light on the CI.

Let me know if you need anything.

@patrickvonplaten @LysandreJik 
1. Added forward test to ensure forward runs okay for `LayoutLM`, `Roberta`, `ELECTRA`, and `BERT` for three position embeddings: "absolute", "relative_key", "relative_key_query".
2. Added integration test for `BERT` check points `bert-base-uncased`, `zhiheng-huang/bert-base-uncased-embedding-relative-key`, and `zhiheng-huang/bert-base-uncased-embedding-relative-key-query` to ensure that models predictions match expected outputs.

@zhiheng-huang - Let me fix the CI later, don't worry about it :-) 

> This looks good to me. Thanks a lot for your PR!
> Any reason ALBERT and Longformer don't get this new functionality? (But RoBERTa and ELECTRA do?)

Great question! I ALBERT should get this functionality (I just added it - great catch!). Longformer has weird attention_scores which does not work with those embeddings.

Good to merge! Thanks a mille @zhiheng-huang! 

> Good to merge! Thanks a mille @zhiheng-huang!

Thanks! @patrickvonplaten @sgugger @LysandreJik