Skip to content

Conversation

shelhamer
Copy link
Member

Export blobs and params with their names through the python wrapper.
Note that pairs of params with the same layer name are the params & bias.

note that pairs of params with the same layer name are the
params & bias
@shelhamer
Copy link
Member Author

@longjon let me know if there's as succinct a way to do this as your earlier code accomplished and if you have general thoughts on what to do with the python wrapper. @sergeyk too.

@longjon
Copy link
Contributor

longjon commented Feb 15, 2014

The code looks fine, although I don't know why we don't use iterators, and in CaffeNet::params I would push the layer blobs directly instead of calling net_->params, since the latter couples the ordering in the wrapper to the ordering in caffe (although of course those should always be the same).

More generally though I'm not sure where the wrapper should land on the spectrum between mirroring exactly the C++ interface and choosing the most Pythonic/convenient interface.

We could:

  1. Mirror the C++ interface exactly
  2. Mirror the C++ interface, but use properties instead of getters
  3. Provide blobs and layers as dict or OrderedDict instead of pairs of lists or lists of pairs
  4. Mirror the C++ interface as in 2 but provide a nicer interface as well, implemented in Python

I guess I feel like 3 right now. In any case it seems odd to add the names directly to the blobs, as this changes the semantics of a blob between C++ and Python (plus, as you noted, layer blobs don't have unique names)...

@shelhamer
Copy link
Member Author

I'm for the pythonic side of the spectrum. If you're looking for the C++ interface, code in C++. I agree with your 3–I'll figure out how to push an OrderedDict through python::boost.

Regarding names in blobs, I did that because it felt awkward to have a list of blob names and a list of blobs and the same for params. Replacing it all with an OrderedDict feels right.

Thanks for the feedback.

@shelhamer
Copy link
Member Author

I think this is desirable as it is, and refactoring to return an OrderedDict can follow. Let's merge? @longjon

@longjon
Copy link
Contributor

longjon commented Feb 25, 2014

Sure, I have no problem merging this, as I think it is an improvement, although I hope it's clear to users that the interface is not yet stable...

@shelhamer
Copy link
Member Author

With that in mind, I've merged it to dev @ 37f340b for the adventurous. Thanks for the wrapper discussion in this thread.

@shelhamer shelhamer closed this Feb 25, 2014
@shelhamer shelhamer deleted the py-names branch March 1, 2014 06:26
@longjon longjon mentioned this pull request May 15, 2014
12 tasks
Brainiarc7 added a commit to Brainiarc7/caffe that referenced this pull request Mar 4, 2015
Fix BVLC#112 entry to proper CUDNN_POOLING_AVERAGE_COUNT_EXCLUDE_PADDING check.
thatguymike pushed a commit to thatguymike/caffe that referenced this pull request Mar 16, 2016
Memory consuming optimization for BN cuDNN algos
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants