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

Makefile improvements #277

Merged
merged 18 commits into from
Apr 4, 2014
Merged

Conversation

jeffdonahue
Copy link
Contributor

A bunch of changes (any of which are open for discussion...I'm especially unsure if (3) is worthwhile to others, but it was useful for me) to the Makefile including:

  1. The most significant change (responsible for probably 75+% of the diff): corrected targets/dependencies. e.g. you can now do make ./build/tools/train_net.bin to generate only that binary and any dependencies from a totally clean directory, and then rerunning it will give you make: build/tools/train_net.bin' is up to date. unless a source file it depends on (e.g. tools/train_net.cpp) has been modified -- similarly for make all , make test, etc. Please let me know if anyone thinks I may have missed anything; I've never tried to write a 'real' Makefile before so I could easily have overlooked or done things in a very weird/wrong way...

  2. TEST_GPUID moves to Makefile.config.example; doesn't seem like we want changes to it tracked by git.

  3. A superclean target that deletes all files ending with extensions that we build (.so .a .o .bin .testbin .pb.cc .pb.h _pb2.py .cuo), which I wrote because whenever I build an old Caffe version I end up with tons of these files littered throughout my directory that make clean won't get rid of because they aren't in build/ or one of the other few known locations. (Also added supercleanlist target to list files that make superclean would delete, in case you want to be careful.)

  4. All core generated files now go somewhere in build/, including libcaffe.{a,so} (under build/lib/) and proto-generated .pb.{cc,h} files. The only generated files that don't go in build/ are the Python and MATLAB wrappers (including proto-generated Python), which stay where they are currently.

(Edit: I forgot) 5) .testbins are now in build/test/*.testbin because it annoyed me having to tab complete 5 times to get build/src/caffe/test/*.testbin.

TODO: test MATLAB build.

@jeffdonahue
Copy link
Contributor Author

I've tested the MATLAB build; this is now ready for review.

@@ -32,8 +29,15 @@ TEST_HDRS := $(shell find src/$(PROJECT) -name "test_*.hpp")
TOOL_SRCS := $(shell find tools -name "*.cpp")
# EXAMPLE_SRCS are the source files for the example binaries
EXAMPLE_SRCS := $(shell find examples -name "*.cpp")
# BUILD_INCLUDE_DIR contains any generated header files we want to include.
BUILD_INCLUDE_DIR := $(BUILD_DIR)/src
Copy link
Contributor

Choose a reason for hiding this comment

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

Header files should live in $(BUILD_DIR)/include

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had it that way before the last commit (3396b45) but changed it back because it requires copying the pb.h file to the build dir after the fact (proto itself is unable to put the .pb.cc and .pb.h files it generates in different directories, as far as I know) which just seemed messy to me. I can just revert that commit if people like the separate build/include dir.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with this.

@kloudkl
Copy link
Contributor

kloudkl commented Apr 1, 2014

@jeffdonahue, thanks for your efforts to re-organize the directory structure of the build directory! It is obvious that tracking where the building results of each individual target should be is not scalable. The Makefile will only continue to grow messier with more source code files being added. We should seriously consider solving the problem #215.

@jeffdonahue
Copy link
Contributor Author

@kloudkl Why do you think it isn't scalable to keep track of the build targets? It's at least not "obvious" to me that it won't scale - it's not as if the Makefile needs to be modified every time we add a new layer; wildcards take care of that. Once I figured out how Makefiles work, this was actually not very difficult to do (and I gained a newfound admiration for vanilla make, personally).

@jeffdonahue
Copy link
Contributor Author

btw, if anyone checks this out for review and initially gets a ton of errors from running make all (or nearly any other target), these are (probably) because of the old .pb.cc and .pb.h proto-generated files lingering around in your src/ and include/ dirs (which will now be in build/ as a result of this PR). make superclean should get rid of these files and make building any target smooth.

@jeffdonahue
Copy link
Contributor Author

Actually just fixed (commit 6b3d257) the behavior in the above comment by including build/ before src/ and include/; shouldn't be any need to do make superclean now (though it's still useful if you want your src/ and include/ dirs clean).

@kloudkl
Copy link
Contributor

kloudkl commented Apr 3, 2014

The philosophy of "it just works" can still take us a very long way.

@@ -1,16 +1,13 @@
# The makefile for caffe. Extremely hacky.
# The makefile for caffe. Pretty hacky.
Copy link
Member

Choose a reason for hiding this comment

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

What a reformation!

@shelhamer
Copy link
Member

This deletes the CIFAR-10 data files since they end in .bin. Perhaps exclude the data/ dir from superclean.

@shelhamer
Copy link
Member

Need to add python/caffe/proto/ to .gitignore.

@jeffdonahue
Copy link
Contributor Author

Thanks for the careful review Evan! I made your two suggested changes in the last commit.

shelhamer added a commit that referenced this pull request Apr 4, 2014
@shelhamer shelhamer merged commit a9516a4 into BVLC:dev Apr 4, 2014
@shelhamer
Copy link
Member

Makefile is now officially "Pretty Hacky." Thanks Jeff!

@jeffdonahue jeffdonahue deleted the makefile-improvements branch April 6, 2014 23:24
@shelhamer shelhamer mentioned this pull request May 20, 2014
mitmul pushed a commit to mitmul/caffe that referenced this pull request Sep 30, 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.

3 participants