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

Replace masked code in all recurrent layers by T.switch command #554

Merged
merged 1 commit into from
Dec 31, 2015
Merged

Replace masked code in all recurrent layers by T.switch command #554

merged 1 commit into from
Dec 31, 2015

Conversation

avostryakov
Copy link
Contributor

Replace masked code in all recurrent layers by T.switch command

Closes #550.

@skaae
Copy link
Member

skaae commented Dec 24, 2015

It looks good to me. Thanks.

@benanne
Copy link
Member

benanne commented Dec 24, 2015

Looks good to me as well. I'll let you do the honours :)

@f0k
Copy link
Member

f0k commented Dec 29, 2015

Looks good, it's cleaner and shouldn't hurt (the previous implementation allows to do some tricks with non-binary masks, but probably nobody did that).
@craffel: Was there a specific reason for not using T.switch()? Did you observe that multiplication to be faster (contrary to the cursory test in #550)?

@skaae
Copy link
Member

skaae commented Dec 29, 2015

I think i did the masking implementation. I don't think there was a particular reason for not using T.switch

@craffel
Copy link
Member

craffel commented Dec 29, 2015

I think there was some reference implementation somewhere from the Theano folks who used the multiplication trick. Great that switch is faster, thanks for this.

@f0k
Copy link
Member

f0k commented Dec 31, 2015

All right, merging then! Thanks again!

f0k added a commit that referenced this pull request Dec 31, 2015
Replace masked code in all recurrent layers by T.switch command
@f0k f0k merged commit 3dc3ccd into Lasagne:master Dec 31, 2015
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.

None yet

5 participants