Skip to content
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

Refactor of the attention example #243

Closed
wants to merge 2 commits into from
Closed

Conversation

emanjavacas
Copy link
Contributor

This pull request attempts to (i) avoid code duplication and (ii) make more explicit the difference between decoding during training and testing.
After some thoughts on how to accomplish this, I settled on python support for co-routines using generators (and their next and .send(arg) methods). I think this makes the code more clear and readable in terms of intentions, and also helps encouraging python literacy on co-routines.

minor ref for clarity

minor ref

removed get_loss
@yoavg
Copy link
Contributor

yoavg commented Jan 16, 2017

This is really cool, but I am a bit worried that the .send(arg) mechanism is a bit too obscure for a "tutorial" code (which is not about teaching generators).

Perhaps some dedicated, detailed comments explaining the architecture could help.

@emanjavacas
Copy link
Contributor Author

Yes, I was also worrying the same when I was coding it. Anyway, I think I will just make a blog post out of that producer-consumer example and I will try to add some inline comments as you suggest. So, I will close this and create a new pull request once I am done.

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