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

CPU-only build #561

Merged
merged 7 commits into from
Jul 17, 2014
Merged

CPU-only build #561

merged 7 commits into from
Jul 17, 2014

Conversation

shelhamer
Copy link
Member

Note: the CUDA libraries are still required for now.

  • add CPU_ONLY flag to Makefile to skip GPU compilation
  • stub GPU methods to die and complain if their use is attempted in CPU-only build
  • hide GPU code from CPU-only build by preprocessor guards
  • collect CUDA includes into a single single header that does actual CUDA or CUDA alternate stubs
  • define CPU-only Caffe singleton and SyncedMemory
  • separate all CUDA code by guards for CUDA-less CPU-only build

This adds a CPU-only switch to the Makefile.config that builds Caffe without compiling any GPU code. When this flag is uncommented

  • make builds libcaffe.so and libcaffe.a without any GPU compilation at all (not a single call to nvcc).
  • make test builds the tests as usual.
  • make runtest only runs the CPU tests.

Mostly I was curious what this would take since it partially addresses #150. With this build we should be able to configure a Travis-CI hook for CPU testing of PRs and such.

Not that we should necessarily merge this as-is or at all however, especially with the grossness of 81791ec, and the macro pollution I added in 871b0ce.

I imagine the #415, BVLC:device-abstraction work would make carrying out the CPU / CUDA separation at compilation simpler. Perhaps this is useful all the same?

What does everybody think?

@kloudkl
Copy link
Contributor

kloudkl commented Jun 30, 2014

This is very useful. It shortens the compilation time when I develop on a machine without the GPU.
But the tests still require CUDA driver that can't be installed on such machines.

[ RUN      ] CommonTest.TestCublasHandler
F0630 14:49:24.400686  9753 test_common.cpp:18] Check failed: error == cudaSuccess (35 vs. 0)  CUDA driver version is insufficient for CUDA runtime version
*** Check failure stack trace: ***
    @     0x2b631e6299fd  google::LogMessage::Fail()
    @     0x2b631e62b89d  google::LogMessage::SendToLog()
    @     0x2b631e6295ec  google::LogMessage::Flush()
    @     0x2b631e62c1be  google::LogMessageFatal::~LogMessageFatal()
    @           0x51a175  caffe::CommonTest_TestCublasHandler_Test::TestBody()
    @           0x598acd  testing::internal::HandleExceptionsInMethodIfSupported<>()
    @           0x5908a1  testing::Test::Run()
    @           0x590986  testing::TestInfo::Run()
    @           0x590ac7  testing::TestCase::Run()
    @           0x590e1e  testing::internal::UnitTestImpl::RunAllTests()
    @           0x59864d  testing::internal::HandleExceptionsInMethodIfSupported<>()
    @           0x58fefe  testing::UnitTest::Run()
    @           0x41392d  main
    @     0x2b6320be5ec5  (unknown)
    @           0x417327  (unknown)
make: *** [runtest] Aborted (core dumped)

@shelhamer
Copy link
Member Author

@kloudkl thanks for pointing that out. Such tests will need to be separated out for Travis-CI integration until a custom build bot system is configured.

@bhack
Copy link
Contributor

bhack commented Jun 30, 2014

@jeffdonahue has opened a cmake branch from the cmake pr. How we want to handle PR and commits like this one that edit Makefile? Is needed to duplicate any effort but with a risk of having Cmake and Makefile out of sync often in the future.

@kloudkl
Copy link
Contributor

kloudkl commented Jun 30, 2014

In the short term, keeping them in sync for the CPU and GPU platforms is not very hard.

CMake is a flexible cross platform build tool. I believe that it will finally help Caffe support many more platforms in the long term. For example, it is easier to use CMake to cross compile Caffe for the Android or iOS mobile platforms that are usually deployed on the ARM or other hardware architectures. The Windows support needed by some users can be handled too.

Meanwhile, the CMake system will stay highly readable and manageable. In general, the CMake build scripts for different platforms will all be in their own modules instead of cramming together.

@bhack
Copy link
Contributor

bhack commented Jun 30, 2014

My experience with other project is that maintain two build system is not a good choice.
I agree with @kloudkl that cmake could open to more cross platform solution and it is quite common for c++ projects.
There are some open pr targeting only Makefile.

@@ -12,6 +12,8 @@ namespace caffe {

const float kBNLL_THRESHOLD = 50.;

#ifndef CPU_ONLY

Copy link
Contributor

Choose a reason for hiding this comment

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

was this intentional? seems like it should be in either all the .cu files or none of them. (and probably it should be in none of them as we can just not attempt to build .cu files in cpu only mode)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, these were accidental. Thanks for the catch. Will fix shortly.

Le lundi 30 juin 2014, Jeff Donahue [email protected] a écrit :

In src/caffe/layers/bnll_layer.cu:

@@ -12,6 +12,8 @@ namespace caffe {

const float kBNLL_THRESHOLD = 50.;

+#ifndef CPU_ONLY
+

was this intentional? seems like it should be in either all the .cu files
or none of them. (and probably it should be in none of them as we can just
not attempt to build .cu files in cpu only mode)


Reply to this email directly or view it on GitHub
https://github.com/BVLC/caffe/pull/561/files#r14361634.

This was referenced Jul 2, 2014
@huyng huyng mentioned this pull request Jul 11, 2014
3 tasks
jeffdonahue pushed a commit to jeffdonahue/caffe that referenced this pull request Jul 12, 2014
jeffdonahue pushed a commit to jeffdonahue/caffe that referenced this pull request Jul 14, 2014
@shelhamer
Copy link
Member Author

@jeffdonahue the TestDtypesAndDevices types defeats the test selection by --gtest_filter I made use of in this PR for the cpu-only make test. Do you know a workaround?

I introduced a further preprocessing guard in 080893d for the device typdefs to exclude the GPU when CPU_ONLY is true.

@jeffdonahue
Copy link
Contributor

Look at the "make runtestnogpu" I added for Travis to use -- does that do
what you want? It filters out the "2" and "3" type tests (FloatGPU and
DoubleGPU).
On Jul 14, 2014 7:21 AM, "Evan Shelhamer" [email protected] wrote:

@jeffdonahue https://github.com/jeffdonahue the TestDtypesAndDevices
types defeats the test selection by --gtest_filter I made use of in this
PR for the cpu-only make test. Do you know a workaround?


Reply to this email directly or view it on GitHub
#561 (comment).

@shelhamer
Copy link
Member Author

Thanks for the pointer! I took the new filter for the CPU-only make runtest instead of the guard. Once a CPU-only build is merged how about rolling make runtestnogpu into make runtest with the CPU_ONLY flag controlling the build?

@jeffdonahue in general how do you feel about this approach to a CPU-only build? Too much of a hack, or acceptable while BVLC:device-abstraction is readied? Is it worth seeing this approach through?

If so, my next step is to isolate the CUDA includes into a single header that will conditionally actually include CUDA or stub out the GPU functions instead for CPU-only. Then we can eliminate the CUDA dependencies for CPU-only Caffe.

@shelhamer
Copy link
Member Author

p.s. The CPU-only build and testing takes 69 s with make -j8.

  • make: 16s
  • test: 26s
  • runtest: 27s

@jeffdonahue
Copy link
Contributor

Totally in favor of removing runtestnogpu and controlling it via the flag. I think the approach is good, I wouldn't think there's any way around using ifdefs.

@jeffdonahue
Copy link
Contributor

I had one other thought -- you could also add ifdefs to not use the two new GPU test types I defined in test_caffe_main.hop. then you could skip the new 2/3 things in the gtest_filter because it would compile without the GPU tests.

@shelhamer
Copy link
Member Author

Note: worked @ 9a28c2b except for timing (see 48469ca) but broken by more drastic separation attempted in 4caec0b that tries to remove CUDA dependency for CPU-only installation.

Once I address the tests, tools, and throw in a few more guards this should be set.

@shelhamer
Copy link
Member Author

We have CPU-only brewing with no CUDA dependency!

@jeffdonahue please review for merge. There are more ifdef guards scattered about than I would like, but I think BVLC:feature-abstraction should concentrate them and make the code less messy.

@jeffdonahue
Copy link
Contributor

Sweet, I'll check it out today.
On Jul 16, 2014 7:21 AM, "Evan Shelhamer" [email protected] wrote:

We have CPU-only brewing with no CUDA dependency!

@jeffdonahue https://github.com/jeffdonahue please review for merge.
There are more ifdef guards scattered about than I would like, but I think
BVLC:feature-abstraction should concentrate them and make the code less
messy.


Reply to this email directly or view it on GitHub
#561 (comment).

INCLUDE_DIRS += ./src ./include $(CUDA_INCLUDE_DIR)
INCLUDE_DIRS += $(BUILD_INCLUDE_DIR) ./src ./include
ifneq ($(CPU_ONLY), 1)
INCLUDE_DIRS += $(CUDA_INCLUDE_DIR)
Copy link
Contributor

Choose a reason for hiding this comment

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

add indent between ifneq and endif

@jeffdonahue
Copy link
Contributor

Looks good! Made a couple small comments on the makefile changes -- please glance at them, make any fixes, and merge.

@jeffdonahue
Copy link
Contributor

I have a concern that I hadn't thought about previously -- while having a CPU-only build is great for many reasons, are we sure it's the best idea to use it exclusively in our Travis build? While Travis can't run the GPU unit tests to check for functional correctness (since the VMs don't have any CUDA GPU access), it does at least test that the full system, including CUDA code, compiles. Should we consider having Travis do the full non-cpu-only build as well as the cpu-only one that it actually tests?

@shelhamer
Copy link
Member Author

Agreed, we should run both the CPU-only build and CPU + GPU builds
distinctly by Travis. A little slower, but better coverage and catches
potential issues.

...like the fact that I pushed a broken state of the normal build here
-- sorry! It's fixed by deleting a single line, but I can't seem to push
with the connection here at the moment.

Will fix that, adjust the Travis build to do both builds, and address your
comments then merge (tomorrow).

Le jeudi 17 juillet 2014, Jeff Donahue [email protected] a écrit :

I have a concern that I hadn't thought about previously -- while having a
CPU-only build is great for many reasons, are we sure it's the best idea to
use it exclusively in our Travis build? While Travis can't run the GPU unit
tests to check for functional correctness (since the VMs don't have any
CUDA GPU access), it does at least test that the full system, including
CUDA code, compiles. Should we consider having Travis do the full
non-cpu-only build as well as the cpu-only one that it actually tests?


Reply to this email directly or view it on GitHub
#561 (comment).

shelhamer added a commit that referenced this pull request Jul 17, 2014
CPU-only build

no GPU, no CUDA, no problem!
@shelhamer shelhamer merged commit 6b82d65 into BVLC:dev Jul 17, 2014
@shelhamer shelhamer deleted the cpu-only-build branch July 17, 2014 10:35
@shelhamer
Copy link
Member Author

Thanks for the quick review and build consideration @jeffdonahue!

@kloudkl give this a try sometime and let me know if you run into any trouble.

I'll follow-up with documentation soon.

@akosiorek akosiorek mentioned this pull request Jul 25, 2014
@shelhamer shelhamer mentioned this pull request Aug 8, 2014
mitmul pushed a commit to mitmul/caffe that referenced this pull request Sep 30, 2014
CPU-only build

no GPU, no CUDA, no problem!
RazvanRanca pushed a commit to RazvanRanca/caffe that referenced this pull request Nov 4, 2014
CPU-only build

no GPU, no CUDA, no problem!
@rajhlinux
Copy link

rajhlinux commented Sep 9, 2022

How do I compile it for FreeBSD 13.1 without GPU?

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.

5 participants