Question about tie_weights

[cjw-d]

Hi,

I noticed that the logic of the tie_weights function has changed in the transformers 5.0.0rc.

In v4.x, when tie_word_embeddings=True, weights between embed_tokens.weight and lm_head.weight were always tied, regardless of whether both tensors were present in the checkpoint.

However, in v5.0.0rc, if both embed_tokens.weight and lm_head.weight are explicitly present in the checkpoint, the model no longer ties them, resulting in two independent copies of the weights.

https://github.com/huggingface/transformers/blob/v5.0.0rc1/src/transformers/modeling_utils.py#L2362-L2369

This change affects training behavior: gradients will now be computed and applied separately to the two weight matrices, potentially leading to divergent updates and inconsistent model behavior compared to previous versions.

I'm concerned this might introduce subtle training instabilities or break backward compatibility for users who expect tied embeddings and output layers by default (as was the case in v4.x). I’m wondering whether this change could lead to any unexpected problems？For example, although embed_tokens.weight and lm_head. weight exist in the checkpoint, I still want to tie these two weights.

Thanks!

[Rocketknight1]

cc @Cyrilvallez but I think this is the intended behaviour when the checkpoint stores both weights!

[Cyrilvallez]

Hey @cjw-d! It's indeed the current intended behaviour, and we are aware that it's BC-breaking sometimes, that's why you now see a warning in such cases!

The problem here lies in the fact that due to another previous silent bug, some models did NOT have their embeddings tied by default, even though the default config field is True. After we refactored the tie_weights logic, this silent bug disappeared (that's what happen when you make things simpler and more logical 🤗). But the models that were previously victims of this silent bug, for example some mistral models DO NOT necessarily have tie_word_embeddings=False in their config even though they should (because it was skipped anyway du to the bug...), and they naturally have 2 very different weights for each one. Tying them anyway would completely destroy the model, as their weight are SUPPOSED to be different.

For this reason, I introduced the check and the warning. I thought about adding an equality check on top, to tie both weights anyway if both matrices have the same values, but ultimately decided that this was an expensive check to perform (as embeddings can be very large), and not absolutely needed, as the warning is always displayed and we don't expect many models that have this issue. Moreover, if both weights are present in the checkpoints, it means that the weights were NOT tied when saving the model (otherwise tied weights are removed from saved checkpoints), so it makes sense to not tie them back when loading.