SamLynnEvans / Transformer

Transformer seq2seq model, program that can build a language translator from parallel corpus
Apache License 2.0
1.34k stars 346 forks source link

Computational Error on PositionalEncoder() #7

Open jacobastern opened 5 years ago

jacobastern commented 5 years ago

I read your blog post in TowardsDataScience on this model, and I think there may be a computational error in line 27 of Transformer/Embed.py. In the paper and in other implementations, like this one, we should have PE_(pos, 2i+1) = math.cos(pos / (10000 * ((2 i)/d_model))), not math.cos(pos / (10000 * ((2 (i + 1))/d_model))), as the code currently stands.

orena1 commented 5 years ago

I think @jastern33 is correct, this is the result from this github: image

And this is from - http://nlp.seas.harvard.edu/2018/04/03/attention.html image