Lightning-AI
Wrong PositionalEncoding in the Transformer example
Bug description
Hi,
I think there are several mistakes in the implemention of the PositionalEncoding in the lightning/pytorch/demos/transformer.py.
Since the transformer is set to https://github.com/Lightning-AI/pytorch-lightning/blob/275822d5aff848b7f0a4298498e905457b8455c9/src/lightning/pytorch/demos/transformer.py#L47
- The shape of the pe should be [batch_size, len, dim]. This transpose operation is redundant. https://github.com/Lightning-AI/pytorch-lightning/blob/275822d5aff848b7f0a4298498e905457b8455c9/src/lightning/pytorch/demos/transformer.py#L97
- And the shape of
self.peshould beself.pe[:, :x.size(1)]https://github.com/Lightning-AI/pytorch-lightning/blob/275822d5aff848b7f0a4298498e905457b8455c9/src/lightning/pytorch/demos/transformer.py#L88 - The result of the addition above is not assigned to x, so the pe will not take effect.
Would you please let me know if I'm wrong?
What version are you seeing the problem on?
master
How to reproduce the bug
No response
Error messages and logs
# Error messages and logs here please
Environment
Current environment
#- Lightning Component (e.g. Trainer, LightningModule, LightningApp, LightningWork, LightningFlow):
#- PyTorch Lightning Version (e.g., 1.5.0):
#- Lightning App Version (e.g., 0.5.2):
#- PyTorch Version (e.g., 2.0):
#- Python version (e.g., 3.9):
#- OS (e.g., Linux):
#- CUDA/cuDNN version:
#- GPU models and configuration:
#- How you installed Lightning(`conda`, `pip`, source):
#- Running environment of LightningApp (e.g. local, cloud):
More info
No response
cc @borda
You are not on the master branch. Thank you for your reply. I checked the code permalinks and found that they will be changed automatically to branch 275822 from master after I copy and paste in the box. In fact, they are exactly the same.
I think it would be good to go over the example one more time to fix the correctness issues. The initial version was just ported from the PyTorch examples repo but the goal then was not to make it train well but to serve as dummy example for quick testing.
@Galaxy-Husky Do you want to make the adjustments in a PR?
Sure, I'd love to.
I think it would be good to go over the example one more time to fix the correctness issues. The initial version was just ported from the PyTorch examples repo but the goal then was not to make it train well but to serve as dummy example for quick testing.
Thank you for your explanation. I got it !
