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

Have create_param() set the broadcast pattern #715

Merged
merged 2 commits into from
Jul 4, 2016

Conversation

f0k
Copy link
Member

@f0k f0k commented Jun 30, 2016

This PR makes create_param() set the broadcast pattern when creating a shared variable. We assume our network parameters never change shape, so we can at least fix the broadcast pattern (shared variables don't allow setting a fixed shape). This saves some headaches when users have a DenseLayer of 1 unit and try to do something fancy with the output (as here: https://groups.google.com/d/msg/lasagne-users/MQfjYI-tV2I/pf-z_NnyBQAJ), since now Theano can infer that the output is broadcastable on the last axis.

@coveralls
Copy link

coveralls commented Jun 30, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling f60389d on f0k:bcast-params into cf88e29 on Lasagne:master.

@benanne
Copy link
Member

benanne commented Jun 30, 2016

Sounds good, should we maybe have a comment in the code that explains why we are doing this (basically what you just said)? broadcastable patterns don't come up too often so it might baffle some people who read that piece of code.

@f0k
Copy link
Member Author

f0k commented Jul 1, 2016

Sounds good, should we maybe have a comment in the code that explains why we are doing this (basically what you just said)?

Good point, did that!

@coveralls
Copy link

coveralls commented Jul 1, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 4d4e0b0 on f0k:bcast-params into cf88e29 on Lasagne:master.

@benanne
Copy link
Member

benanne commented Jul 2, 2016

LGTM, will merge soon if there are no further comments :)

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

3 participants