-
Notifications
You must be signed in to change notification settings - Fork 18.6k
Finishing touches on @bhack's SliceLayer (#680) #755
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
Conversation
Differentiate top test blob vector size Rename to SplitLayer Add slicing points
suggested by @sguada). Test for both num & channels in forward & backward; use GaussianFiller so that tests are non-trivial.
|
@jeffdonahue thanks for the finishing touches, nice job @bhack |
|
@jeffdonahue A general little stylistic question. How do we prefer to handle repeated element in Caffe? We want to copy to a vector with a for loop or we want to use std::copy? |
|
@bhack std::copy seems more reasonable. Please fix any for-loop copies as you see fit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sort of confused here when I use this layer, and traced back to here... Why MinTopBlobs() returns 2? (why not 1?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ronghanghu Slice is a knife for a blob. A division by 1 exist in math. But is it operative useful when you are cooking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right :)
I was trying to use slice layer as a component in another layer to automatically slice a blob of shape T x ... into T blobs of shape 1 x ..., and run into this error when T == 1. I'll handle the special case myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not allow for 1 top blob? I understand that it doesn't make sense to use a SliceLayer for that purpose, but if a user asks for it, the layer could act as a pass-through, doing nothing at all. This makes the layer more general.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are some derived reuse of this layer then could be extended to hande the Top 1 case in a way that not impact performance (cause it is a useless computation) so the logic doesn't need to be reproduced everywhere.
Finishes #680. Thanks for contributing this layer @bhack, and thanks for your review @sguada. Will merge once Travis passes.