Add "tie_word_embeddings" config param
Author: patrickvonplaten
Pull request: yes

As discussed in #6628, this PR makes the word embedding tying dependent on a new parameter: 
`config.tie_word_embeddings` which is set to `True` by default and set to `False` for Reformer.

Also, some unnecessary code is removed in Albert and a similar param is deprecated in Transfo-XL. 

I don't see how this PR could break backwards compatibility as  the `tie_word_embeddings` param did not exist before and is set to `True` by default. 

Thanks a lot for posting the issue @stas00  .

Comments:
`tie_word_embeddings` doesn't sound a good name to me since it may be confused with freezing the word embeddings.

Maybe `embedding_as_softmax_weights` or something like that?

# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/6692?src=pr&el=h1) Report
> Merging [#6692](https://codecov.io/gh/huggingface/transformers/pull/6692?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/625318f52516b413126be1bb1cb6818231d2eca6?el=desc) will **decrease** coverage by `0.41%`.
> The diff coverage is `81.25%`.

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

```diff
@@            Coverage Diff             @@
##           master    #6692      +/-   ##
==========================================
- Coverage   79.49%   79.08%   -0.42%     
==========================================
  Files         156      156              
  Lines       28405    28399       -6     
==========================================
- Hits        22581    22458     -123     
- Misses       5824     5941     +117     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/6692?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_reformer.py](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19yZWZvcm1lci5weQ==) | `96.16% <ø> (+0.07%)` | :arrow_up: |
| [src/transformers/modeling\_transfo\_xl.py](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190cmFuc2ZvX3hsLnB5) | `79.69% <50.00%> (ø)` | |
| [src/transformers/configuration\_transfo\_xl.py](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX3RyYW5zZm9feGwucHk=) | `89.09% <60.00%> (-3.22%)` | :arrow_down: |
| [src/transformers/configuration\_reformer.py](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX3JlZm9ybWVyLnB5) | `100.00% <100.00%> (ø)` | |
| [src/transformers/configuration\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX3V0aWxzLnB5) | `96.66% <100.00%> (+0.02%)` | :arrow_up: |
| [src/transformers/modeling\_albert.py](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19hbGJlcnQucHk=) | `83.43% <100.00%> (-0.07%)` | :arrow_down: |
| [src/transformers/modeling\_mobilebert.py](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19tb2JpbGViZXJ0LnB5) | `89.45% <100.00%> (ø)` | |
| [src/transformers/modeling\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ191dGlscy5weQ==) | `88.05% <100.00%> (ø)` | |
| [src/transformers/modeling\_tf\_electra.py](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9lbGVjdHJhLnB5) | `25.13% <0.00%> (-73.83%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_mobilebert.py](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9tb2JpbGViZXJ0LnB5) | `24.55% <0.00%> (-72.36%)` | :arrow_down: |
| ... and [5 more](https://codecov.io/gh/huggingface/transformers/pull/6692/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/6692?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/transformers/pull/6692?src=pr&el=footer). Last update [625318f...c723d16](https://codecov.io/gh/huggingface/transformers/pull/6692?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


> `tie_word_embeddings` doesn't sound a good name to me since it may be confused with freezing the word embeddings.

I think the name `tie_word_embeddings` is alright because it quite clear that it is a flag to me and it forces the output word embeddings to point to the same graph node where the input word embeddings poitns to for which "tying' is a fitting word IMO. 

> > `tie_word_embeddings` doesn't sound a good name to me since it may be confused with freezing the word embeddings.
> 
> I think the name `tie_word_embeddings` is alright because it quite clear that it is a flag to me and it forces the output word embeddings to point to the same graph node where the input word embeddings poitns to for which "tying' is a fitting word IMO.

Fair enough!

What about the first part of `tie_weights` - the proposed PR, leaves it unmodified:
```
    def tie_weights(self):
        [...]
        output_embeddings = self.get_output_embeddings()
        if output_embeddings is not None:
            self._tie_or_clone_weights(output_embeddings, self.get_input_embeddings())
```
this needs to be configurable too.

The sub-class override with `pass()` in `reformer` removed both calls and not just `self._tie_or_clone_weights`, so this PR changes the behavior of `reformer` which will now tie input and output embeddings. I will need the same original behavior (neither of 2 calls) for fairseq transformer port.

I thought the conclusion in https://github.com/huggingface/transformers/issues/6628 was about a config option to activate or not `tie_weights`, but ended up with only one of its internal calls. But, I think this change is good - as it gives more refined control, though also need the first call to be configurable.

Perhaps it could be: `config.tie_in_out_embeddings` for the first call?


> What about the first part of `tie_weights` - the proposed PR, leaves it unmodified:
> 
> ```
>     def tie_weights(self):
>         [...]
>         output_embeddings = self.get_output_embeddings()
>         if output_embeddings is not None:
>             self._tie_or_clone_weights(output_embeddings, self.get_input_embeddings())
> ```
> 
> this needs to be configurable too.
> 
> The sub-class override with `pass()` in `reformer` removed both calls and not just `self._tie_or_clone_weights`, so this PR changes the behavior of `reformer` which will now tie input and output embeddings. I will need the same original behavior (neither of 2 calls) for fairseq transformer port.
> 
> I thought the conclusion in #6628 was about a config option to activate or not `tie_weights`, but ended up with only one of its internal calls. But, I think this change is good - as it gives more refined control, though also need the first call to be configurable.
> 
> Perhaps it could be: `config.tie_in_out_embeddings` for the first call?

Sorry I don't really follow here. Could you explain how this PR breaks backward compatibility for Reformer
and what first part of `tie_weights` is not modified? 

Here is a stripped down version of the code:

Before this PR:

```
# src/transformers/modeling_utils.py
    def tie_weights(self):
            self._tie_or_clone_weights(...) # part 1
            self._tie_encoder_decoder_weights(...) # part 2

# src/transformers/modeling_reformer.py
 def tie_weights(self):  pass
```
After this PR:
```
# src/transformers/modeling_utils.py
    def tie_weights(self):
            self._tie_or_clone_weights(...) # part 1
        if self.config.tie_word_embeddings: # part 2
            self._tie_encoder_decoder_weights(...)

# src/transformers/modeling_reformer.py
```
I removed all the other option checks to just show the gist of the change.

As you can see the first part of `tie_weights`, i.e. `_tie_or_clone_weights`, will now be called by reformer whereas it was not getting called before this PR when it overridden the whole `tie_weights` method. i.e. the first part also needs a config option.

Please let me know whether this clarification was useful, @patrickvonplaten. 


> # src/transformers/modeling_utils.py
>     def tie_weights(self):
>             self._tie_or_clone_weights(...) # part 1
>         if self.config.tie_word_embeddings: # part 2
>             self._tie_encoder_decoder_weights(...)
> 
> # src/transformers/modeling_reformer.py

I think it should be rather (after this PR):

```
# src/transformers/modeling_utils.py
    def tie_weights(self):
        if self.config.tie_word_embeddings:
            self._tie_or_clone_weights(...) # part 1
        self._tie_encoder_decoder_weights(...) # part2

# src/transformers/modeling_reformer.py
```

Weird. I see what happened. github was showing only part of the code in the diff, not showing the `if self.config.is_encoder_decoder and self.config.tie_encoder_decoder` so I thought the other part will always be run. 
`tie_encoder_decoder` and `is_encoder_decoder` are False (by default) for reformer therefore the other part won't be run either, so it's still a noop for reformer.

And then I mixed up the 2 parts when trying to explain what I thought I saw.

I will checkout the whole PR code in the future and not look at the partial picture shown in github.

So all is good.

Thank you for bearing with me.

Thank you for this fix, @patrickvonplaten