Skip to content

Added greedy decoder input #6

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
wants to merge 2 commits into from

Conversation

dmmiller612
Copy link
Contributor

In this PR, I added a greedy decoder function that generates the decoder input for inference. This is important for translating sentences as we don't know the target input beforehand. In the paper, they mentioned that they ran Beam Search with a k=4. In the greedy approach, k = 1.

@dmmiller612
Copy link
Contributor Author

Weird, I can see that a message was added to this thread from my email, but I can't see it here. Maybe Github is acting up?

@graykode
Copy link
Owner

graykode commented Feb 3, 2019

Hello, dmmiller612!!
Thanks for your pull request!
Your code is very nice for implemented Transformer.
I'll add self-attention Mask on Decoder in Transformer Code
However, My main point is show to beginner more easily!
So, Could you send me new Pull Request about new file name like Transformer(Greedy_decoder)-Torch.py ?
My mean is your Pr is very good, but not overlapping this code, apply Pr in new Code Transformer(Greedy_decoder)-Torch.py @dmmiller612
I will wait for your new Pull Request Thanks you :D

@dmmiller612
Copy link
Contributor Author

Sounds good. I'll try to get it done tonight.

@graykode
Copy link
Owner

graykode commented Feb 3, 2019

@dmmiller612 Good. I'll also add your code to ipynb file in Google Colab for beginner

@dmmiller612 dmmiller612 closed this Feb 3, 2019
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

Successfully merging this pull request may close these issues.

2 participants