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

merged pull#1271 for locally connected layer #3068

Closed
wants to merge 6 commits into from
Closed

Conversation

raingo
Copy link

@raingo raingo commented Sep 15, 2015

Merge #1271 to master branch. Everything is copied from there, except some little adjustments for the master branch

@jackculpepper, @shelhamer please have a look.

@raingo raingo changed the title merged pull#1271; and passed the tests merged pull#1271 for locally connected layer Sep 15, 2015
@jklontz
Copy link
Contributor

jklontz commented Oct 30, 2015

👍 Hope this gets merged!

@swamiviv
Copy link

Is it possible to merge this to master ?

@JordanCheney
Copy link

would love to see this merged into master!

@jackculpepper
Copy link
Contributor

Thanks for bringing it up to date. Glad to know it was useful to others! I haven't been actively working on this recently but would be happy to pick it up again.

We should probably get rid of the test filler from include/caffe/filler.hpp. The test that uses it:

https://github.com/BVLC/caffe/pull/3068/files#diff-01bba204a2be65f3a1e5d5ba33b8b077R81

..was useful during debugging, but the operation performed by that filler could be moved into the test itself.

@dougsouza
Copy link

Could somebody please resolve the conflicts and merge this?

@jklontz
Copy link
Contributor

jklontz commented May 31, 2016

We're offering a bounty to anyone willing to complete the remaining work on this pull request. See caffe-users for details: https://groups.google.com/forum/#!topic/caffe-users/N50S97Mw2VQ

@matthieudelaro
Copy link

@jklontz: #4251
Here is an attempt to finish the remaining work to merge raingo's implementation of locally connected layers into master branch:

  • merge conflicts have been solved
  • the filler (declared for test purposes only) has been moved inside the test itself

@kevin-li
Copy link

kevin-li commented Jun 28, 2016

Would be REALLY excited to see this merged! My project needs this!

@GabrielLin
Copy link

I hope it can be merged.

@matthieudelaro
Copy link

Me too. But here is the thing: commits must be rebased (just one commit per author), and an optimization in CUDA is expected (I hope BVLC will accept it without it though, so that at least people can use it, and then anyone can improve) #4251. I hadn't time to finish this, and I won't be able to complete it before months. So... if anyone is up for rebasing, you are welcome :)

@raingo
Copy link
Author

raingo commented Sep 7, 2016

Closing this. Because caffe has everything needed to implement locally connected layer. Check the following layer combinations:

BxRxC = Im2colLayer(BxCxHxW)
BxD = ReshapeLayer(BxRxC)
BxD1 = InnerProductLayer(BxD)
BxC1xH1xW1 = ReshapeLayer(BxD1)

There may be mistakes, but you got the idea.

@raingo raingo closed this Sep 7, 2016
@knsong
Copy link
Contributor

knsong commented Oct 10, 2016

The idea is good! BTW, shouldn't there be a Crop layer at the very top? @raingo

@Banus
Copy link

Banus commented Nov 13, 2016

@raingo, I wasn't able to implement the combination you proposed.
The output of Im2colLayer is a matrix (H1 x W1, Kw x Kh x C) where (Kw, Kh) is the shape of the filter kernel, and in LocalLayer each row needs to be multiplied by a different weight matrix in order to generate the (H1 x W1, C1) output.
InnerProductLayer, by acting on the flattened input, can scale each row by a different set of weights, but unfortunately it sums all the results together. You can see the issue from the different number of parameters: the weight matrix for the InnerProductLayer is (C x H1 x W1 x Kw x Kh, C1 x H1 x W1), while the LocalLayer requires only C1 x C x H1 x W1 x Kw x Kh weights (H1 and W1 appear only once).
A ConvolutionLayer also doesn't help because of the weight sharing across different input rows.

@knsong
Copy link
Contributor

knsong commented Nov 14, 2016

@Banus @raingo Here is a related question on StackOverflow.

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.