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

CMake build system - cleaned #573

Closed
wants to merge 2 commits into from
Closed

CMake build system - cleaned #573

wants to merge 2 commits into from

Conversation

akosiorek
Copy link
Contributor

CMake build system from #442. I squashed commits and cleaned the history for easier merging.

I also added LMDB support as in #571 (@kloudkl I hope you won't hold it against me)

Building tips:

mkdir build
cd build
CC=gcc-4.6 cmake ..
make
ctest -v

(Not sure if CC=gcc-4.6 is neccesary, but no other version works for me)

Options are specified at cmake invokation with -D flag as in cmake -DBUILD_PYTHON=ON ..

  • BUILD_PYTHON [default OFF]
  • BUILD_MATLAB [default OFF]
  • BUILD_EXAMPLES [default ON]
  • BLAS [atlas, open, mkl; default atlas]
  • BUILD_SHARED_LIBS [default OFF]
  • CUDA_TEST_DEVICE [default 0]

Remarks:

  • Tested only under Linux Mint 16


### Install #################################################################################

install(TARGETS caffe DESTINATION lib)
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to install also caffe_cu

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No we don't need to do that. It's only an intermediate library that gets compiled into caffe target. I would have used an object library, which was introduced in CMake 2.8.8 for exactly this purpouse, if it was supported by cuda_add_library() command

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. But actually when i install shared library in /usr/local/lib or /usr/lib caffe binary runtimes need also caffe_cu. So we need to find a solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Building caffe_cu explicitly as STATIC should solve the problem. Could you try replacing
cuda_add_library(caffe_cu ${CU_SOURCES}) with cuda_add_library(caffe_cu STATIC ${CU_SOURCES}) and try again? This way caffe_cu is always compiled as a static lib (it does not depend on BUILD_SHARED_LIBS variable) and is linked to caffe. In case it does not work I'll add caffe_cu to be installed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes making it static works. If it is not needed by developers.

@kloudkl
Copy link
Contributor

kloudkl commented Jul 1, 2014

To be honest, the FindLMDB cmake script belongs to crack-language hosted on Google code.


### Install #################################################################################

install(TARGETS ${name} DESTINATION tools)
Copy link
Contributor

Choose a reason for hiding this comment

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

We could user install destination "bin" instead of tools so that a system make install could put this binaries in /usr/bin or /usr/local/bin and are binaries are available in system path

@kloudkl
Copy link
Contributor

kloudkl commented Jul 1, 2014

Are you sure to have squashed all the changes?

-- Examples enabled
CMake Error at CMakeLists.txt:55 (add_subdirectory):
  The source directory

    /home/user/Codes/caffe-fork/examples

  does not contain a CMakeLists.txt file.


-- Configuring incomplete, errors occurred!

@akosiorek
Copy link
Contributor Author

Sorry about that. I forgot that the examples folder was in .gitginore and had to be forced into git.

@kloudkl kloudkl mentioned this pull request Jul 2, 2014
@shelhamer
Copy link
Member

@kloudkl @akosiorek @bhack

Re: #15 (comment), #561 (comment), #561 (comment)

This is not ready. While I respect the effort that has been put into it and the ideal of a self-configuring and cross-platform build, in practice it has to work. For merge, the CMake build system should strictly improve on the current Makefile + Makefile.config and work in all the same cases (and more).

To make the policy clear: the project's official build system is the Makefile. A parallel build system is infeasible to maintain. Until CMake can entirely replace the Makefile it cannot be merged. Issues and questions regarding the CMake build will not be supported by the Caffe team until it is official, which is to say merged to dev / released in master. Should any aspect of the official build change while the CMake build is still under development, the CMake build must track these changes -- this is not the responsibility of PRs into dev since the Makefile is presently the official build.

I hope that progress can be made on the CMake build to ready it for review by the Caffe core developers as a new official build. The BVLC:cmake branch was promoted to make this work more public so that it might be useful to those interested in it now and encourage finishing it.

In order to do this at least the following must be addressed:

@kloudkl
Copy link
Contributor

kloudkl commented Jul 2, 2014

@akosiorek, it seems that the PR target has to be changed (again) to the cmake branch so that I and others can contribute.

@jeffdonahue
Copy link
Contributor

Has this been rebased to the latest dev? It might be better for me to do a hard reset of the BVLC branch to this if so...not really sure. @shelhamer how have you managed keeping feature branches up to date in the past?

@shelhamer
Copy link
Member

@jeffdonahue here's what I've done:

  1. Promote the feature branch to BVLC:feature.
  2. Open a PR from it to dev to keep track of progress, review, and have
    public discussion.
  3. Contributors PR to BVLC:feature, and once merged the PR to dev
    automatically updates.
  4. From time-to-time rebase BVLC:feature on dev and force push.
  5. Repeat 2-3 as needed until finally ready for merge. Applause.

To diffuse responsibility, you should have contributors do the rebasing
themselves. They can push to their forks and comment to let you know. Then
you can hard reset BVLC:feature to their fork's branch -- you have to be in
the loop since you have push rights, but this keeps it minimal.

Admittedly, for the boost migration I did too much of step 3 myself so
learn from my mistake.

On Wed, Jul 2, 2014 at 11:09 AM, Jeff Donahue notifications@github.com
wrote:

Has this been rebased to the latest dev? It might be better for me to do a
hard reset of the BVLC branch to this if so...not really sure. @shelhamer
https://github.com/shelhamer how have you managed keeping feature
branches up to date in the past?


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

@akosiorek
Copy link
Contributor Author

@kloudkl @bhack Now when this PR was promoted to a feature branch in BVLC repo it would seem that it's life, as a PR, is over. Am I right? I can add you to my fork as collaborators or you can contribute to BVLC:cmake via separate PRs.

I'd like to work on CMake up to a point when it'll be an official build system for Caffe. Could we sketch a TO-DO list of what's left, besides @shelhamer's remarks?

@shelhamer
Copy link
Member

@akosiorek @kloudkl @bhack see #165 for a model of feature branch integration. cmake is not over as a PR. While you can collaborate on it through PRs to BVLC:cmake or together at your forks, the integration overall should be handled as a PR from BVLC:cmake to BVLC:dev like the MKL / non-MKL integration. This PR will last until the feature branch is done.

@jeffdonahue please open such a PR for them to sketch their TODOs and otherwise discuss completing and integrating the feature branch.

@bhack
Copy link
Contributor

bhack commented Jul 3, 2014

We need to get in sync with behavior of Makefile for shared and static? See my wrong feedback to the user in #593.
Distro maintainers put headers and static version of the libraries in dev version of package (I.e. libcaffe-dev in Debian an derivates style)

@jeffdonahue
Copy link
Contributor

Thanks @akosiorek! I've (force) pushed this to BVLC's cmake branch and opened #623 for official tracking of CMake development.

@jeffdonahue jeffdonahue closed this Jul 5, 2014
aoym pushed a commit to ideeinc/caffe that referenced this pull request May 30, 2017
Add support for cuDNN v6
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