Adding Adapters
Author: zphang
Pull request: yes
Labels: name: wontfix

From: https://arxiv.org/pdf/1902.00751.pdf

Open to feedback!

* Implementing adapters requires a couple more hyperparameters that need to go into the BertConfig. Do let me know if there is an alternative to modifying the core Config object (maybe a subclass would work better?)
* If `use_adapter` is False, the adapter modules are not created, so there should be no issue with changes in `state_dicts`/weights if they're not enabled.
* Added a utility function for extracting the adapter parameters from the model, to facilitate tuning only the adapter layers. In practice, a user should tune the adapter layers (+layer norm) and the final classifier layers, the latter of which varies depending on the model. 
* I believe this should work seamlessly with RoBERTa.

Comments:
# [Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289?src=pr&el=h1) Report
> Merging [#1289](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/pytorch-transformers/commit/0d1dad6d5323cf627cb8d7ddd428856ab8475f6b?src=pr&el=desc) will **increase** coverage by `0.22%`.
> The diff coverage is `39.39%`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289/graphs/tree.svg?width=650&token=9qOlN6Hb1c&height=150&src=pr)](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1289      +/-   ##
==========================================
+ Coverage   80.77%   80.99%   +0.22%     
==========================================
  Files          57       57              
  Lines        8092     8072      -20     
==========================================
+ Hits         6536     6538       +2     
+ Misses       1556     1534      -22
```


| [Impacted Files](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pytorch\_transformers/configuration\_bert.py](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289/diff?src=pr&el=tree#diff-cHl0b3JjaF90cmFuc2Zvcm1lcnMvY29uZmlndXJhdGlvbl9iZXJ0LnB5) | `88.57% <100%> (+1.47%)` | :arrow_up: |
| [pytorch\_transformers/modeling\_bert.py](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289/diff?src=pr&el=tree#diff-cHl0b3JjaF90cmFuc2Zvcm1lcnMvbW9kZWxpbmdfYmVydC5weQ==) | `85.17% <31.03%> (-3.16%)` | :arrow_down: |
| [pytorch\_transformers/modeling\_xlnet.py](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289/diff?src=pr&el=tree#diff-cHl0b3JjaF90cmFuc2Zvcm1lcnMvbW9kZWxpbmdfeGxuZXQucHk=) | `77.42% <0%> (+2.89%)` | :arrow_up: |
| [pytorch\_transformers/modeling\_roberta.py](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289/diff?src=pr&el=tree#diff-cHl0b3JjaF90cmFuc2Zvcm1lcnMvbW9kZWxpbmdfcm9iZXJ0YS5weQ==) | `75.22% <0%> (+10.26%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289?src=pr&el=footer). Last update [0d1dad6...cd97cad](https://codecov.io/gh/huggingface/pytorch-transformers/pull/1289?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


I think our goal here should be that you'd be able to extend the relevant classes without having modify to modify the lib's code.

Thoughts @LysandreJik @thomwolf?

Yes, I agree with @julien-c.

Having adapters is a nice addition but we should have a mechanism that lets us extend the base code (of each model) instead of modifying it for each type of adapting mechanism.

One way we could do that is to have standardized pointers to a selection of relevant portions of the models (also asked by Matt Newman for some AllenNLP extension of the models).

Would that be on the roadmap soon? I can resubmit my PR after there's a more generalized approach for extending model functionality.

Thinking about it again, I think the approach you proposed is the right one if we want to integrate adapters in the library as a permanent option.

But I have two questions:
- do we want to integrate adapters as a permanent option? My quick tests with adapters for the NAACL tutorial on transfer learning were giving mixed results. Do you have clear indications and benchmarks that they are useful in practice, @zphang?
- if we do integrate them in the library we would want to have them for all the models.

If we don't want to integrate them as a permanent option then we would have to find a general way to extend the models easily so people can add stuff like this in a simple way. This is open for comments.

In the meantime, I've actual moved to using an implementation that doesn't involve modifying the Transformers library. Here's an example (pardon the hacks):

https://github.com/zphang/nlprunners/blob/9caaf94ea99102a9980012b934a4373dc4996108/nlpr/proj/adapters/modeling.py#L55-L135

It involves swapping out relevant portions of the model with modified/similar layers. Given the differences between the major transformer models, I think this would be the more sustainable and less intrusive (as long as the underlying transformer code doesn't change too often). 

Performance-wise, my experience has been that adapters work roughly as advertised: consistently slightly less well than fine-tuning the whole model, but only the adapter layers + classifier head need to be tuned.

I think the repository you are linking to is private Jason

Oops! Here's a gist of the code: https://gist.github.com/zphang/8eb4717b6f74c82a8ca4637ae9236e21

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.


Closing for now, we'll continue the work on this topic in the more general #2165