-
Notifications
You must be signed in to change notification settings - Fork 18.7k
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
Reshape layer #108
Reshape layer #108
Conversation
@@ -91,6 +91,12 @@ message LayerParameter { | |||
// point would be set as rand_skip * rand(0,1). Note that rand_skip should not | |||
// be larger than the number of keys in the leveldb. | |||
optional uint32 rand_skip = 53 [ default = 0 ]; | |||
|
|||
// For the Reshape Layer one need to specify the new dimensions | |||
optional int32 num = 54 [default = 0]; |
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.
Shall re rename it reshape_num, reshape_channels, reshape_height and reshape_width to avoid confusion? Since num, channels, height and width may be too general.
Done renamed reshape params. The new params could be used by other layers |
@@ -91,6 +91,12 @@ message LayerParameter { | |||
// point would be set as rand_skip * rand(0,1). Note that rand_skip should not | |||
// be larger than the number of keys in the leveldb. | |||
optional uint32 rand_skip = 53 [ default = 0 ]; | |||
|
|||
// For the Reshape Layer one need to specify the new dimensions | |||
optional int32 new_num = 60 [default = 0]; |
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.
How about reshape_
instead of new_
just to make it really obvious?
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 did that at first, but then I thought that other layers may want to use the same parameters to specify the new dimensions of the output. See for instance #120
I think it's kind of redundant to have both reshape and flatten layer -- one option is to just add the extra params to FlattenLayer, change its name to ReshapeLayer, and have it default to the flattening behavior when all 0 (default) params are specified. What do you think? Also, what is the use case for changing "num"? It seems like it may be more natural to only allow reshaping a given data instance (channels/height/width), but maybe there's a need to merge or split data instances that I'm not considering? |
@jeffdonahue I think reshape layer could replace flatten layer if the default behavior is clear. That will make the default behave as flatten layer
The need to change num is the motivation I created this layer, I need to be able to concatenate different images and compute features across them. |
All makes sense and sounds good! |
@sguada could you implement the flattening convention in your #108 (comment) and rebase to make this a clean merge? Thanks! |
Let's bring in #219 first, and then this can replace the flatten layer once it is a clean merge. |
@sguada could you open a PR against |
When will this layer be available? |
@sguada can you PR this layer to dev and include the flattening behavior in your #108 (comment)? |
Replaced by #1263. |
200169109 fix
Added a reshape layer that allows to change the shape of the blobs, although it doesn't change the size. It is generalization of flatten_layer.