Skip to content
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

Use proto extensions with hashed field numbers for layer-specific parameters #2036

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jeffdonahue
Copy link
Contributor

This is the proof-of-concept mentioned by @shelhamer in #1896. DO NOT MERGE -- I'm not really sure if this is a good way to go, this PR is just for discussion for now.

This would facilitate self-contained independent development of layers by addressing one of the major current pain points with that -- conflicting proto field numbers. The idea is that instead of picking a field number consecutively after the previous one, which of course causes conflicts whenever other new layers are added, as long as your field name (e.g., convolution_param) doesn't conflict with anything else in LayerParameter, the field number is generated as a function of the name, and (with high probability) you get a unique number. (The collision odds for any given pair of extensions are 1 in 536,850,911; haven't done the birthday problem analysis to figure out what that actually means in practice, but if we were going to use this we might need to eventually add a "salt" field to escape collisions.)

See src/caffe/proto/reshape_param.proto.varextnum for an example of what it looks like to declare a LayerParameter extension this way. The script scripts/fill_proto_ext_num.py creates the actual reshape_param.proto file from the *.varextnum version.

Build notes: there is currently something finicky about my Makefile changes such that parallel builds with make -j sometimes don't work -- a workaround is to first do make proto (no -j) before doing make -j. I haven't attempted to add CMake support.

(start from 20000 as 19000 to 19999 are reserved by the protobuf
implementation -- see
https://developers.google.com/protocol-buffers/docs/proto (search for
'kFirstReservedNumber') for details
reshape_param declared using a proto extension
@jeffdonahue
Copy link
Contributor Author

As hacky/scary of an idea as this might be, it's looking more attractive as it looks like we currently have at least 11 mutually unmergable PRs, which would probably all be safely mergable in any order if not for the fact that they all grab LayerParameter ID 132. Thoughts?

@bhack
Copy link
Contributor

bhack commented Mar 10, 2015

I think it is needed cause id in proto are actually a natural way to have cross PR conflicts. Is there also a commit of ReshapeLayer?

shelhamer referenced this pull request Apr 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants