Skip to content

Conversation

bhack
Copy link
Contributor

@bhack bhack commented Jul 12, 2014

Implement a split by dim layer.
Please take a look at #596 comments

This was referenced Jul 12, 2014
@bhack
Copy link
Contributor Author

bhack commented Jul 15, 2014

@sguada Can you give me a feedback?

@bhack bhack mentioned this pull request Jul 16, 2014
@bhack
Copy link
Contributor Author

bhack commented Jul 21, 2014

@shelhamer Can anybody give me a feedback?

@shelhamer
Copy link
Member

@sguada please review.

@bhack can I suggest SliceLayer as the name?

@sguada
Copy link
Contributor

sguada commented Jul 21, 2014

@bhack Although I think having fixed size slice/split can be useful in some specific situations, I think to make it really compatible as Concat reverse one would need to specify different sizes.

As mentioned in our previous conversation in #596 I would add another repeated param, slice_point that would allow to define the points for slicing, and therefore obtain different sizes blobs. So either there are no slice_point defined, in which case it will use equal size slices, or if there defined, then there should be one per top. @bhack if your think adding this feature would be too complex, we can leave it for another PR.

Agree with @shelhamer in that Slice could be a better name.

@bhack
Copy link
Contributor Author

bhack commented Jul 21, 2014

Renamed. I've also tried to add split index.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Takes one bottom blob, ...
creates as many top blobs as needed

Differentiate top test blob vector size

Rename to SplitLayer
Add slicing points
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add another test for set_slice_dim(1)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the next TestCPUSetupChannels. Default dim is 1

@sguada
Copy link
Contributor

sguada commented Jul 22, 2014

Please address some of the comments and wait for Travis to pass all the test, after that it could be merged.

jeffdonahue added a commit that referenced this pull request Jul 22, 2014
@jeffdonahue
Copy link
Contributor

Finished in #755. Thanks @bhack and @sguada!

@shelhamer shelhamer mentioned this pull request Aug 7, 2014
mitmul pushed a commit to mitmul/caffe that referenced this pull request Sep 30, 2014
RazvanRanca pushed a commit to RazvanRanca/caffe that referenced this pull request Nov 4, 2014
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.

5 participants