Skip to content

Transformer tutorial error #682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dhpollack opened this issue Oct 8, 2019 · 5 comments
Closed

Transformer tutorial error #682

dhpollack opened this issue Oct 8, 2019 · 5 comments

Comments

@dhpollack
Copy link

https://github.com/pytorch/tutorials/blob/master/beginner_source/transformer_tutorial.py#L89

Since you are using the CrossEntropyLoss, you shouldn't do the log_softmax in the above line. You can either switch to the NLLLoss or remove the log_softmax to fix this.

@zhangguanheng66
Copy link
Contributor

@dhpollack Thanks for bringing us the issue. Could you submit a PR and fix it? You could assign @zhangguanheng66 and @SethHWeidman for reviewing.

@dhpollack
Copy link
Author

@zhangguanheng66 I think that I'll have time for a PR. Do you mind if I change other things? Personally, I found the batching a bit awkward, why not use pytorch's built in Dataset and DataLoader? Also, you could keep the targets in their original dimensions and transpose the outputs and use the multidimensional functionality of the loss function (easier to see in code then explained in words)... there were a couple small things like that which I saw but didn't think were worth github issues, but if I were writing the tutorial, I would have changed.

@zhangguanheng66
Copy link
Contributor

Yes. This part is very true. The current wiki-text dataset is not compatible with DataLoader. My plan is to fix the wikitext dataset in pytorch/text first and and we could fix the tutorial. Again, the wikitext dataset is based on a wired pattern and I plan to fix it first.

It would be great if you could submit a quick fix for the CrossEntropyLoss part and I will let you know once the wikitext dataset is updated.

@dhpollack
Copy link
Author

@zhangguanheng66 alright, here's the quick fix. I don't have the proper rights to assign you as a reviewer.

@soumith
Copy link
Member

soumith commented Oct 13, 2019

fixed via #695

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants