Layer factory. #1167
Layer factory. #1167
Conversation
|
+1, well implemented! |
|
So I have verified that both CPU_ONLY and with gpu cases pass on my 2 machines, but for some reason travis-ci still does not like registration - I don't know why. @shelhamer @jeffdonahue could you double-check? I guess it night be gcc 4.6 vs gcc 4.8 but I am not sure. Cmake still does not work. All we need to do is to do -Wl,--whole-archive when we link the caffe static library, though. |
|
@bhack Thanks for checking - so if I read it correctly, with gcc 4.8 all tests actually pass. It is expected that cmake will fail - I haven't changed the cmake part yet. I am not sure about 83.3, but it seems to be not due to the factory code... |
|
OK so it turned out to be a lint + some cmake hack. I have not finished the tool/ part for cmake, but will go explore the redwoods tomorrow - kindly don't merge yet. |
|
@Yangqing Yes the little lint issue was already fixed in my build. Cmake in Caffe as I remember is configured to produce only shared library so actually doesn't need extra flag if you don't want to add static library also in cmake (probably it is better to have cmake and make with the same library output). In cmake seems that was failing on test running and not on build. |
|
@Yangqing, would you like to create another macro to eliminate the repetitions in the layer creators? |
|
@bhack : cmake does use static linking (I think it gets added at some point?), but I've fixed the tools and python static linking in the commit above. |
3a33096 to
7336877
Compare
…reserve all symbols.
|
OK I think all are good now, Master @shelhamer when you approve I will pull. If you happen to know some McDonalds that are hiring, please let me know. |
include/caffe/layer_factory.hpp
Outdated
There was a problem hiding this comment.
can you put this in doxygen format starting with an @brief? (see layer.hpp for example)
|
just looked over this -- LGTM. awesome implementation, thanks @Yangqing! |
|
Pulling. Blame me if your python or matlab caffe is broken. |
|
Fixed in a36cceb. |
|
This seems to break clang++ compilation, as done in OS X, since there is no |
|
Hi +1 what @shelhamer said. Unable to build on my OS X 10.9.5 with Xcode 6.0 command line tools. --whole-archiv doesn't exist. It should go under some sort of a flag. |
|
@vimalthilak try #1215. |
Layer factory.
Layer factory.
Implemented a layer factory that allows one to register a layer more easily. Both the Makefile version and cmake version are changed. Since the test only covers the C++ parts, I cannot guarantee that python and matlab versions are right. However, if you encounter registerer errors, simply do --whole-archive when you link in libcaffe.a.
== Original PR message ==
So I've been wanting to replace the old, sort-of-working layer factory function with a real registrar. This PR implements it, and hopefully makes writing new layers easier (instead of modifying an evergrowing if then statement, simply register a new layer).
A side effect is that, with registration in the code, we cannot use static library any more. For all binaries inside Caffe this is fine - I changed the behavior so that libcaffe.a is gone, and all binaries are linked directly with .o files.
I am yet to test if .so works.I also added a test that dynamically links to libcaffe.so to make sure dynamic libraries work with registration.I am not an expert of CMake so I haven't changed the CMake file yet.CMake files are changed accordingly.In terms of documentations, I have written a truly marvellous documentation of this, which this PR message is too short to contain. (Just kidding. I am in the process of finishing the documentation for it.)Documentation of the registration process is written in layer_factory.hpp.Also, the PR may potentially break Windows builds. Just want to expose it first and see what you guys think about it.