Embedding scaling is tied to positional encoding for Transformer models

[guillaumekln]

When positional encoding is disabled, the embedding scaling is also disabled even though the operations are independent:

https://github.com/OpenNMT/OpenNMT-py/blob/1.0.0/onmt/modules/embeddings.py#L48

In consequence, Transformer models with relative position representations do not follow the reference implementation which scales the embedding by default.

[francoishernandez]

Nice catch.
Do you think this scaling factor makes much difference when training from scratch?
I agree it would be better to stick to the original implementation though.

[guillaumekln]

I don't think this can make an interesting difference, especially as the next operation is the layer normalization. So it's mostly about clarity.

[vince62s]

I really think it is not necessary. the scaling of embeddings is necessary in the case of Sinusoidal position encoding to make the addition of two elements of the same order range.
However there is an issue with dropout which is applied only with PE. It should be applied in even without. #2285

discussion here: https://datascience.stackexchange.com/questions/87906/transformer-model-why-are-word-embeddings-scaled-before-adding-positional-encod
confirms this point of view I think.