Finishing touches on @bhack's SliceLayer (#680) #755
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. |
| return LayerParameter_LayerType_SLICE; | ||
| } | ||
| virtual inline int ExactNumBottomBlobs() const { return 1; } | ||
| virtual inline int MinTopBlobs() const { return 2; } |
There was a problem hiding this comment.
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.
@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.
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.
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.
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.