Revert "Fix weight loading issue"
Author: patrickvonplaten
Pull request: yes

Reverts huggingface/transformers#14016

Comments:
Sorry for reverting the PR here - it's on me! I merged it too quickly. We had some internal discussion and came to the conclusion that this hack is probably not worth the functionality it would give us here. 

Saving and loading a model with `tempfile` inside the `from_encoder_decoder_pretrained(...)` function is a big hack and it's questionable whether it's worth it.

Just to compare the current design to how it would look like if we revert the PR for @LysandreJik @sgugger @Rocketknight1 

If we leave `master` as it is, one can convert a PyTorch model checkpoint **correctly** as follows:

### current design

```python
from transformers import EncoderDecoderModel, TFEncoderDecoderModel

_model = EncoderDecoderModel.from_pretrained("patrickvonplaten/bert2bert-cnn_dailymail-fp16")
_model.encoder.save_pretrained("./encoder")
_model.decoder.save_pretrained("./decoder")

model = TFEncoderDecoderModel.from_encoder_decoder_pretrained(
    "./encoder", "./decoder", encoder_from_pt=True, decoder_from_pt=True
)

# then this works:
model.save_pretrained("./")
model = TFEncoderDecoderModel.from_pretrained("./")
``` 

If we remove the hack, the (in my opinion currently only way) to convert a PT checkpoint to TF is the following:

### design after removing hack

```python
from transformers import EncoderDecoderModel, TFEncoderDecoderModel, TFAutoModel, TFAutoModelForCausalLM

_model = EncoderDecoderModel.from_pretrained("patrickvonplaten/bert2bert-cnn_dailymail-fp16")
_model.encoder.save_pretrained("./encoder")
_model.decoder.save_pretrained("./decoder")

# all these lines are currently done automatically. There is not really a way around doing them if we remove the hack IMO
_encoder = TFAutoModel.from_pretrained("./encoder", from_pt=True)
_decoder = TFAutoModelForCausalLM.from_pretrained("./decoder", from_pt=True)
_encoder.save_pretrained("./encoder")
_decoder.save_pretrained("./decoder")

model = TFEncoderDecoderModel.from_encoder_decoder_pretrained("./encoder", "./decoder")

# then this works:
model.save_pretrained("./")
model = TFEncoderDecoderModel.from_pretrained("./")
```

So we can see that removing the hack would force the user to do the exact same thing we are doing right now

Give that the hack only lives in `modeling_tf_encoder_decoder.py` and having thought about it again, I'm actually in favor of not merging this PR, but I defer to @LysandreJik and @sgugger to decide here.

No problem for me. I leave HF members to make the decision. Just to make this clear to users would be fine on my side 😀

Agree to close this and keep the current hack, *as long as* we mention that the `TFEncoderDecoder` is experimental. 

In my opinion, TensorFlow is globally ill-suited for managing several models into a single one like it is done here, and it will always have some hacky/kind-of-broken edge cases.

I would advocate for keeping the work @ydshieh has done so far and see if the community appreciates/uses the feature before spending time refactoring this complex piece of software.

I agree with you two @LysandreJik and @patrickvonplaten, even if I'm really not a fan of the hack behind the scenes. Let's worry about making it better when we have a wide adoption of the TFEncoderDecoder :-)