Add support for LayerNorm before residual connection in transformer
Author: jarednielsen
Pull request: yes

Recent work has shown that placing the layer norm before the residual connection in a transformer leads to better gradient propagation and more stable training. For example, [Learning Deep Transformer Models for Machine Translation](https://arxiv.org/abs/1906.01787) shows that when stacking more layers, the Pre-LN transformer performs better than the Post-LN transformer. A more detailed discussion is found in [On Layer Normalization in the Transformer Architecture](https://openreview.net/pdf?id=B1x8anVFPr), which aids understanding despite being rejected from ICLR 2020 for lack of novelty.

_In my own experiments pretraining from scratch with TFAlbert, I have found pre-layer normalization to have greater training stability and improved performance than the default post-layer normalization._

<img width="331" alt="Screen Shot 2020-04-23 at 11 37 36 AM" src="https://user-images.githubusercontent.com/4564897/80131074-e8f0d380-8556-11ea-9459-0d2c255c4428.png"> <img width="280" alt="Screen Shot 2020-04-23 at 11 41 05 AM" src="https://user-images.githubusercontent.com/4564897/80131507-95cb5080-8557-11ea-8fcf-7757c15ab604.png">

**This is not a final PR, just opening the discussion. Some caveats:**
- If approved, I can extend this to all transformer models, not just ALBERT.
- Loading an existing pretrained model with `config.pre_layer_norm = True` leads to poor performance. A separate pretrained model would need to be made available with this configuration option. I would be happy to do so for ALBERT, but this would be outside my pay grade for all the models. Since the current focus of the repo is on finetuning rather than pretraining, I understand if this isn't a complexity tradeoff you want to make.

What do you think about adding this configuration option?



Comments:
Because I just stumbled upon it: a counterpoint in [Understanding the Difficulty of Training Transformers](https://arxiv.org/abs/2004.08249), where the author founds that while this tends to stabilize the convergence, it also results in worse performances for the runs that do converge.

(Still, nice PR, I was toying with the idea of doing it too :-)

> Because I just stumbled upon it: a counterpoint in [Understanding the Difficulty of Training Transformers](https://arxiv.org/abs/2004.08249), where the author founds that while this tends to stabilize the convergence, it also results in worse performances for the runs that do converge.
> 
> (Still, nice PR, I was toying with the idea of doing it too :-)

Hi, author here (On Layer Normalization in the Transformer Architecture),

For Pre-LN, a slightly larger dropout rate is usually needed for median scale tasks, such as translation. This is not required for BERT. Pre-LN has larger expressiveness power than Post-LN and when the data scale is small, it is more likely to overfit. 

When you use a larger dropout in translation tasks, Pre-LN is no worse than Post-LN. [There are three 'dropout' places in Transformer, For translation tasks, we apply 0.1 to all the three places which make Pre-LN faster to converge and achieve better/competitive performance]. 

You can reproduce the experiments in our paper.



Oh, thanks for chiming in, I must read the paper again then!

Bumping this discussion. Any thoughts @LysandreJik @julien-c ?

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.


Not stale

> Not stale

I think the pre-ln version should be added to the branch. As far as I know, more and more works are developed based on this architecture. BTW, although our paper is rejected by ICLR, it is accepted by ICML:)

Bumping this, any thoughts @thomwolf ?

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.


Not stale

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](https://github.com/huggingface/transformers/blob/master/CONTRIBUTING.md) are likely to be ignored.