Exposing layer top and bottom names to python #2865

Merged
merged 1 commit into from Jan 8, 2016

Conversation

Projects
None yet
2 participants
Contributor

philkr commented Aug 5, 2015

This PR allows the python interface to access the top and bottom blob names, which makes the data flow a bit more explicit in python.

longjon added the Python label Aug 5, 2015

Contributor

longjon commented Aug 24, 2015

Seems like a reasonable thing to expose to pycaffe. A couple questions:

  1. Why expose these functions in C++ as well? Currently they're (obviously) not in use; if there's some refactoring to be done using them they might be worth having, but if not it seems we're exposing an unused interface.
  2. Why create custom classes instead of using OrderedDicts like elsewhere? Right now this seems like an unnecessary (and less functional) mixing of styles.
Contributor

philkr commented Aug 24, 2015

thanks for the feedback. As for the C++ exposition, I didn't see a way of exposing it to python without exposing something to C++, but I'm open to suggestions.
(2) I guess an ordered dict of lists could work, but I felt it might not be as easy as what is there. The main issue is that currently the top and bottom names are computed in the function Net<Dtype>::bottom_names. In order to put them into a OrderedDict one would need to call this function for every layer and store the result somewhere in the python Net, which can get messy quickly. Please let me know if you have any suggestions on how to handle this without creating a mess.

@longjon longjon commented on an outdated diff Jan 5, 2016

include/caffe/net.hpp
@@ -149,6 +149,16 @@ class Net {
inline const vector<vector<Blob<Dtype>*> >& top_vecs() const {
return top_vecs_;
}
+ /// @brief returns the ids of the top blobs of layer i
+ inline const vector<int> & top_ids(int i) const {
+ CHECK_LT(i, top_id_vecs_.size()) << "Invalid layer id";
@longjon

longjon Jan 5, 2016

Contributor

We also need to CHECK_GE(i, 0), yes?

@longjon longjon and 1 other commented on an outdated diff Jan 5, 2016

python/caffe/pycaffe.py
@@ -276,6 +276,22 @@ def _Net_batch(self, blobs):
padding])
yield padded_batch
+
+class _Net_IdNameWrapper:
+ """
+ A simple wrapper that allows the ids propery to be accessed as a dict
+ indexed by names. Used for top and bottom names
+ """
+ def __init__(self, net, func):
+ self.net, self.func = net, func
+
+ def __getitem__(self, name):
+ # Map the layer name to id
@longjon

longjon Jan 5, 2016

Contributor

No tabs please!

@philkr

philkr Jan 5, 2016

Contributor

tabs are the best!

Contributor

longjon commented Jan 5, 2016

The new patch looks pretty reasonable; comments as noted. Also:

  • I'm still not sure that constructing an OrderedDict, as used by many of the other Net methods would be messier than the current approach; the code would be pretty much as it is now (perhaps in comprehension form), cached in net if needed per your comment above. Then you could (e.g.) iterate through top_names in order, which is a fairly straightforward thing to desire, but won't work right now.
  • Please squash cleanups before merge.
Contributor

philkr commented Jan 5, 2016

Not sure where to compute the OrderedDict to store it in net. I have been using this for a while now. I usually iterate through _layer_names and layers anyway and rarely directly through top and bottom names. But if you see an easy change to make it work I'm very open to changing it.

Contributor

longjon commented Jan 5, 2016

Right, due to the awkwardness of the current method rewriting scheme, it would need to be done in a similar way to the other Net methods, i.e., you would compute the OrderedDict during the call. If that creates unacceptable per-call overhead, you could cache the result in the net object (e.g., by testing hasattr). Which maybe we should be doing on other calls as well, or maybe we should provide an __init__ callback, or use some other scheme, but this is the way it is now...

@longjon longjon added a commit that referenced this pull request Jan 8, 2016

@longjon longjon Merge pull request #2865 from philkr/top_bottom_names
Exposing layer top and bottom names to python
917ef33

@longjon longjon merged commit 917ef33 into BVLC:master Jan 8, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Contributor

longjon commented Jan 8, 2016

Okay, I'll go ahead and merge as is; while it would be nice to have OrderedDict functionality, this PR provides a useful interface and that functionality can be added in a future patch.

Thanks for this exposure @philkr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment