Improved CMake scripts #1667

Closed
wants to merge 19 commits into
from

Projects

None yet
@Nerei
Nerei commented Jan 2, 2015

@shelhamer @jeffdonahue @baeuml @kloudkl @akosiorek @Yangqing @BlGene

Hello all,

hope I referenced everyone who participated cmake development (at least I didn't find others)

Following discussion here #1586, as I promised, I prepared slightly improved caffe cmake scripts. The improvement was developed using Ubuntu 14.04 and tested on Yosemite (with libstdc++). I believe Windows support now is as difficult as compiling all dependencies. But I prefer to postpone testing on Windows until current very linux-ish build scripts and behaviour are slightly adapted for cross-platform use and some dependencies are made optional.

Description of changes and new features added

Added OpenCV like formatted configuration log
https://travis-ci.org/BVLC/caffe/jobs/45700248

Added CaffeConfig.cmake generation for build/install cases.
This allows you to connect Caffe to your application using CMake's find_package(Caffe). For more detailed description see below.

BUILD_SHARED_LIB=ON (default) or OFF
build caffe as shared library. In CMake it not good practice to build both shared and static simultaneously. That’s why the switch.

CPU_ONLY = OFF(default) or ON
Forces excluding CUDA support. Also Caffe will compile in CPU_ONLY mode if CUDA Toolkit is not installed or found by cmake. Before build please read chic configuration log dumped by cmake to control this case.

USE_CUDNN = ON (default)
If enabled and cudnn is found build with it, otherwise build without.

CUDA_ARCH_NAME=Auto(default), All, Fermi, Kepler, Maxwell, Manual
specifies target GPU architecture, Selecting concrete value reduces CUDA code compilation time (for instance compilation for sm_20 and sm_30 is twice longer than just for one of them). In case of Auto, cmake will make an attempt to detect GPUS installed in your computer and compile only for them. In case of manual, new CUDA_ARH_BIN/PTX cmake variables are created where space separated list of architectures should be set. Example, CUDA_ARH_BIN=”20 21(20) 50”

BUILD_docs = ON (default)

  • If doxygen installed and found enables doc target. Use make docs to build and make jekyll to run web server. html docs are built in <source folder>/doxygen, and next symlink is created in <source folder>/docs/doxygen. Functionality from scripts/gather_examples.sh is now implemented in cmake, but copy_notebook.py is still required.
  • Source folder for generation is used because .Doxyfile contains relative paths and I prefer not to modify it now, but think generation in binary folder is better

BUILD_python = ON (default)
Build python interface if all required dependencies found, otherwise excluded from build automatically

BUILD_matlab = OFF(default)
Enables building matlab interfaces. Currently it supports both Octave and Matlab. For Octave set Octave_compiler to mkoctfile if not found automatically. For Matlab specify Matlab_DIR or Matlab_mex and Matlab_mexext if again not found automatically. If both installed and found, to select which one to use, set Matlab_build_mex_using=Matlab(default) or Octave. Note matlab wrappers can only be built if BUILD_SHARED_LIB=On. On macos both doesn’t compile.

Proto-files
Now protobuf files ARE NOT copied to <caffe_root>/include/caffe/proto anymore. Instead they are generated to <build_dir>/include/caffe/proto. Know one may include old headers, but this is interest rates to payback of technical debt appeared due to incorrect original cmake scripts design. Also removed them from .gitignore

test.testbin

  • Now NO cmake_test_defines.hpp and sample_data_list.txt are configured by cmake to source directory and NO -DCMAKE_BUILD definition added and all *.in templates were removed. This is because make runtest command is executed in source directory, and embedding absolute paths to test cpp-files is not required! Consider configure_file() to source folder as antipattern. However, one may return such embedding by uncommenting couple lines in srcs/test/CMakeLists.txt.
  • All garbage targets (one per each test file) were removed because they flood IDEs while compilation time reduction is controversial. I replaced them with option BUILD_only_tests that allows quickly include only selected tests. Example: cmake -DBUILD_only_tests=="common,net,blob,im2col_kernel"

Yosemite support
I was able to compile with CUDA support using the Caffe instruction with libstdc++ and patching opencv as here opencv/opencv@32f6e1a. Accelerate.framework support added. Matlab interface was failed to compile.

Temporary changes

  • make symlink creates symlink [caffe_root]/build -> cmake_build_directory
  • Now all examples are built without .bin suffix and next symlink with .bin suffix created nearby. So that tutorials could work. Once naming standardized, should remove this.

Including Caffe in your CMake project via find_package()

git clone git@github.com:BVLC/caffe.git. 
cd caffe && mkdir cmake_build && cd cmake_build
cmake .. -DBUILD_SHARED_LIB=ON

Verify that cmake found everything and in proper locations. After can run make -j 12 right now or better do this:

cmake . -DCMAKE_BUILD_TYPE=Debug     # switch to debug
make -j 12 && make install           # installs by default to build_dir/install
cmake . -DCMAKE_BUILD_TYPE=Release   # switch to release
make -j 12 && make install           # doesn’t overwrite debug install
make symlink

After the operations complete, caffe tutorials should work from caffe root directory. Let’s now see how to connect caffe to a C++ application that uses Caffe API with cmake. Prepare the following script:

cmake_minimum_required(VERSION 2.8.8)

find_package(Caffe)
include_directories(${Caffe_INCLUDE_DIRS})
add_definitions(${Caffe_DEFINITIONS})    # ex. -DCPU_ONLY

add_executable(caffeinated_application main.cpp)
target_link_libraries(caffeinated_application ${Caffe_LIBRARIES})

Run CMake to configure this application and generate build scripts or IDE project. It will automatically find Caffe in its build directory and pick up all necessarily dependencies (includes, libraries, definitions) and application will compile without any additional actions. Caffe dependencies will also have been included. If you have several Caffe builds or for some reason Cmake wasn’t able find Caffe automatically, you may specify Caffe_DIR=<path-to-caffe-build-dir> in Cmake and this guarantees that everything will work.

Specified Caffe_DIR to build directory leads to always using a build configuration (say, Release or Debug) Caffe compiled last time for. If you set Caffe_DIR=<caffe-install-dir>/share/Caffe where both configurations have been installed, proper debug or release caffe binaries will be selected depending on for which configuration you compile your caffeinated_application.

Enjoy!!

@Nerei
Nerei commented Jan 3, 2015

Nerei@60e8f7a#commitcomment-9141630 According to this @longjon 's comment, I reverted last commit where I had reduced NumPy requirements from 1.7.1. to 1.6.1. But now python interface compilation is not tested by Travis CI. Need to update numpy there.

@longjon
Contributor
longjon commented Jan 3, 2015

Re: numpy versions, Travis should use a recent version after #1473. (If we want to build pycaffe with these CMake updates before that, the Travis build script could be modified to force CMake to go ahead with an older version, either by some CMake switch that I don't know, or by actually updating the CMake script before the build.)

@Nerei
Nerei commented Jan 4, 2015

I noticed that some tasty python features are being implemented (Once they're done, I'm ready to prepare new PR with its support)

@shelhamer
Member

@Nerei thanks for the effort to improve the CMake build and align it with the features of the Makefile. Nice to have the pieces in place to include Caffe in other projects too.

Regarding the shared or static library build, I'd rather the shared library be the default since it is used more commonly (right?).

@bhack
Contributor
bhack commented Jan 5, 2015

👍 for shared default

@Nerei
Nerei commented Jan 5, 2015

Absolutely agree that shared is better! Just it was for similarity with previous vesion.

@bhack
Contributor
bhack commented Jan 11, 2015

@Nerei I'm guessing what is the support status for windows generators with this refactoring. Working VS generators could help people at #15

@bhack bhack referenced this pull request Jan 11, 2015
Open

Caffe on Windows #15

@shelhamer
Member

Great work @Nerei! Is this ready for review or are there further changes planned for this PR?

(Of course more can be done in follow ups, such as CMake support for the new Python features that are on track for merge.)

@Nerei Nerei closed this Jan 11, 2015
@Nerei Nerei reopened this Jan 11, 2015
@Nerei
Nerei commented Jan 11, 2015

To: @bhack CC: @shelhamer @Yangqing

Absolutely, agree that can use windows generators with cmake. But now I don't think that windows is currently supported by caffe without modifications.

If you compile caffe as dynamic library, Attribute __declspec(dllexport) using macro like here should be added to the beginning of EACH class and function in caffe, otherwise the symbols won't be visible from outside of dll-library. In case of static compilation, current layer registration scheme (introduced in @Yangqing's commit 2f88dab) doesn't work because unreferenced static constructors that register layers are just excluded by linker. To fix this, caffe uses workaround in Makefile.

For Windows, one may try -DCMAKE_CXX_FLAGS=/OPT:NOREF option, But I'd rather rewrite the code since anyway it is not very cross-platform, produces difficulties with different compilers, with Matlab/Octave interface and even with clang for ubuntu (--force-load only for shared libraries not for executables). Even if I make it work for some set of compilers now, it would be difficult to support and test these scripts in future for different configurations.

@Nerei
Nerei commented Jan 11, 2015

@shelhamer Thanks! I think it's ready for review. At least future changes, like windows support or improvements, I would cover in separate PR.

I think better to fetch the branch and review cmake-files locally, it should be easy to read. I wanted to write them as very readable, all difficulties are separated as macros, most of which I just copy-pasted from my projects and opencv. My aim was to create some well done basis/style and standardize it, so that it would be clear where/how to write changes in future.

@Nerei
Nerei commented Jan 11, 2015

Regarding windows, I might be worth creating a repository with cmakeified dependencies so that one can compile and use use the dependencies for caffe compilation "in couple clicks". I can help with this since I am primary a windows user.

@bhack
Contributor
bhack commented Jan 11, 2015

@Nerei I'm not a window user but in past for other projects we have used MXE on Linux to cross compile dependencies. It integrates also well with Cmake.

@Nerei
Nerei commented Jan 11, 2015

@bhack Interesting. But cmake scripts are required to be written anyway. And this cross-compile tool uses mingw (gcc port to windows), and its output can't be used from Visual Studio, only from mingw.

@bhack
Contributor
bhack commented Jan 11, 2015

@Nerei VS generators are a feature of cmake. FindScripts need to be written or copied by other BSD project for correct multiplatform support (ones that are not already distributed by Cmake). Mxe is only for building easly I.e. widows dependencies from Linux or easily compiling caffe for windows target in Linux with almost the actual infrastructure.

@Nerei
Nerei commented Jan 11, 2015

Yes. you're completely right, you can compile caffe.exe from linux for windows and use the executable. there. But not cafe.lib to link into your Visual Studio application. The same with dependencies.

@bhack
Contributor
bhack commented Jan 11, 2015

Static library are directly supported in VS. Also dynamic ones are supported with some additional steps

@bhack
Contributor
bhack commented Jan 11, 2015

You can see also VLC example

@Nerei
Nerei commented Jan 11, 2015

For sure you're right. And my 9-th year experience of cross-platform OpenCV support doesn't help me to understand what can and what can't be done with mingw. At least I still think C API can and C++ can't.

@bhack
Contributor
bhack commented Jan 11, 2015

@Nerei so your are from Itseez. Nice to know 😉

@bhack
Contributor
bhack commented Jan 11, 2015

I'm sure that with all the opencv dependencies could be quite impractical.

@shelhamer
Member

@Nerei why does the CMake build crash on

[  2%] Building NVCC (Device) object src/caffe/CMakeFiles/cuda_compile.dir/layers/cuda_compile_generated_absval_layer.cu.o
/usr/local/include/opencv2/core/mat.hpp(117): error: type name is not allowed
/usr/local/include/opencv2/core/mat.hpp(117): error: identifier "_Atomic" is undefined
/usr/local/include/opencv2/core/mat.hpp(117): error: expected an expression
/usr/local/include/opencv2/core/mat.hpp(117): error: identifier "__c11_atomic_fetch_add" is undefined

while there is no issue with the Makefile build? I know you cited @bhack's patch but I installed on OS X 10.10 with OpenCV 2.4.9 with no issue with #1740.

p.s. the auto arch detection is nice for GPU code.

@shelhamer
Member

The Python dependencies should be aligned so that libraries and numpy matching the interpreter are chosen and not this:

-- Python:
--   Interpreter       :   /home/shelhamer/anaconda/bin/python2.7 (ver. 2.7.8)
--   Libraries         :   /usr/lib/x86_64-linux-gnu/libpython2.7.so (ver 2.7.6)

A good rule might be to pick the Python from the user's PATH and determine the libs from there. Often users will have multiple Pythons so picking matters.

@Nerei
Nerei commented Jan 17, 2015

@shelhamer Good question :-) The answer is -DOSX wasn't added in cmake, while added in Makefile. It controlled OpenCV inclusion, but now it seems someone removed OpenCV from headers. At least after rebase, I can't reproduce the problem even if -DOSX is not defined.

FYI, It's better to use #if defined __APPLE__ rather than declare own define in build scripts.

@bhack
Contributor
bhack commented Jan 17, 2015

@Nerei Was it removed with this merge #1236?

@Nerei
Nerei commented Jan 17, 2015

@shelhamer Regarding the python issue, CMake always takes PythonInterp from PATH first, and after, if not found, searches in standard folders. While PythonLibs are always searched in some set of standard hint directories.

CMake doesn't know about your non-standard python installation. General rule (cmake standard) here is to specify all required paths manually in command line:

cmake <build_dir> -DPYTHON_EXECUTABLE=<..> -DPYTHON_LIBRARY=<..> -DPYTHON_INCLUDE_DIR=<..>

or in cmake-gui (cd build_dir && cmake-gui .) So this is not a bug, but well known feature, although agree, it may confuse beginners.

If several Python versions installed, current scripts by default search for 2.7, but user is free to specify 3.0 paths in the same way as for anaconda above. Thanks to cmake variable caching, this have to be done only once.

Perhaps, some kind of automation and checks can be added (say, if ANACONDA_PATH defined by user take everything from it instead of standard python search) but I would put it out of scope of this PR since well done solution require deep plunging into python search specifics on different platforms. Current solution is a bit dumb but universal. CMake summary log should help to catch issues.

@shelhamer
Member

Thanks for the Python explanation in #1667 (comment). I agree it's fine as it is but there'll need to be a note in the installation instructions.

@longjon
Contributor
longjon commented Jan 19, 2015

@Nerei (or others), as I agree with the comments at #1738, but am still not very familiar with the CMake build, I wonder if you could enlighten me about a couple things:

  • Does the CMake build do proper automatic dependency generation for the C++ source? for the CUDA source?
  • Why does the CMake build involve substantially more code than the pure make build?
@Nerei
Nerei commented Jan 19, 2015

@longjon

  1. Yes.
  2. If I did it in fewer code lines than in makefile, nobody would like lt. It would be as primitive as pure make build. Why do modern cars involve substantially more components/details/features than the pure cars from 1950-60s? In fact, both generations are suitable for traveling.

Update: And, if exclude counting utility code, it uses much less lines of code to describe caffe build rules.

@longjon
Contributor
longjon commented Jan 19, 2015

@Nerei re: 2, what I'd like to know is, what additional functionality are we buying for the price of the extra code?

At a quick glance, it looks like there is significant automatic detection of library locations and versions (along with a more formal structure for performing such detection, which tends to blow things up more). So, for example, FindNumPy.cmake is 58 lines that automatically determine numpy version and header location, whereas the pure make system just forces you to write down the header location. My main concern with automatic detection is being able to specify what I really want when it goes wrong, without digging through a bunch of special logic. It looks like the relevant variables are pretty clearly spelled out, and presumably CMake makes this sufficiently easy.

The fact that the build rules are succinct is a good sign. I think I'll try this branch in my day-to-day and let you know how it goes.

@longjon
Contributor
longjon commented Jan 29, 2015

Okay, I finally tried this out. This was my experience.

Run cmake, it quits immediately.

  • I'm told 2.8.8 is required, but I'm on Precise, which only has 2.8.7. Okay, let's hope that's not true.

Edit CMakeLists.txt to soften the requirement, run cmake, it can't find my atlas. Well, I'm using MKL.

  • Since we're trying to autoconfigure all the things here, ideally the build would try all the BLAS options before choking.

But, okay, easy enough to specify, let's pop open CMakeCache.txt.

  • Whoa, over seven hundred lines, that's ten times bigger than Makefile.config, there's no way I can just read all the options.

Well, my / key works fine, let's find the BLAS option and set it to MKL. Success! Time for make -j30.

  • The build is nice and readable, even colored, but progress indicators are dumped all over the place (i.e., not one per line like make -j1), making it rather jarring to read. Surely CMake users don't just... deal with this?

Uh oh, undefined references to BLAS functions. But... the summary said it was using my MKL. Pop open CMakeCache.txt to see where it thinks my MKL is. Search MKL. There's nothing except the one line where I set it. Search for BLAS. Nothing useful. Well, I guess it didn't actually find my MKL, unless it's hidden somewhere in this file... Pull up FindMKL.cmake. The default path is exactly where my MKL is. Pull up the caller, Dependencies.cmake. Yes, it definitely has a line to search for MKL. Bang head for a few minutes... what's this on line https://github.com/BVLC/caffe/pull/1667/files#diff-dcf5891602b4162c36c2125c806639c5R81? If BLAS equals MLK!

  • Okay, typos happen. But why did the build tell me my BLAS was fine and set to MKL when it actually failed to find anything?

Typo fixed, everything should be good now. Oops, could not find MKL. At least this is an honest failure. Back to CMakeCache, it can't find... FFT? SCALAPACK?

  • Why is this irrelevant stuff included?

Back to FindMKL.cmake, hack that stuff out.

  • But what's this, the search paths are hardcoded to mkl/ia32!

That's not going to work for me, I guess I'll hardcode them to intel64. Run cmake, it can't find the interface library, hack that out, run, build... undefined references still. Maybe I hacked out something I needed... back to FindMKL.cmake, read carefully. Oh, what I want is MKL_SDL=ON.

  • Why didn't this appear in my CMakeCache.txt... or somewhere else obvious... or be set by default?

Okay, try once more. This time, it doesn't link against curand. Check CMakeCache.txt, no references to curand, even though cublas etc. are found fine.

  • Once again, why did they build proceed okay even though a required library was missing?

Check Modules/FindCUDA.cmake...

  • Oh wait, it's just Cuda.cmake. Why the inconsistency?

That file seems to be reading some kind of automatically generated variable. Google around a bit, no hints. I give up, I'll just set it manually.

  • What happened? Is this a CMake bug? The library is right there, alongside my cublas...

And, phew, it builds. Okay, but I really wanted a debug build, so let me cmake -DCMAKE_BUILD_TYPE=Debug .., and build. Works fine, but all the binaries have this funny -d suffix.

  • That's going to breaks all my scripts, what's the deal?

Maybe I need to run make install to get the proper version... nope. Let's try a clean build. Run make clean... wait, the release binaries are still here.

  • Why does make clean leave binaries around after switching build type?

Okay, save my CMakeCache.txt, rm -rf *, redo the build. Still the funny -d suffix.

I have no more time to play with build systems, I'm going back to Makefile. I'd love to have a single, beautiful, clean and readable, automagic build that works for everybody, but this is exactly what I feared: the build goes wrong, and then I have to dig. My impression is that this is a big improvement over the previous CMake build, and we should probably merge it, though I'll leave that up to others, but I can't yet support making this the one true build.

@bhack
Contributor
bhack commented Jan 29, 2015

@longjon I don't understand you conclusion. It is strictly a progressive task to create build scripts that can work on "any system". If we don't switch to this one we cannot collect enough feedbacks from users to let's Cmake be robust (other then specific problem you got with your system) if people continue to build caffe with makefile. Also the official Makefile wasn't created perfect in Caffe so that builded the first time on every system and was improved through time also fixing issues opened on github. I can help to support the Cmake build but we need to switch to that if we want collect enough feedbacks from different users systems.

@shelhamer
Member

I'd love to have a single, beautiful, clean and readable, automagic build that works for everybody, but this is exactly what I feared: the build goes wrong, and then I have to dig. My impression is that this is a big improvement over the previous CMake build, and we should probably merge it, though I'll leave that up to others, but I can't yet support making this the one true build.

This is exactly right. As an improvement to the current CMake build this is a step forward, but still short of an official build. This should likely be merged for those it does help, but at the same time there are many who can't have their work hampered by a byzantine build, so the official Makefile is still needed in that role.

@bhack it isn't strictly progressive if it introduces compilation issues by its complexity or trips up skilled developers, scientists, and entrepreneurs who have taken care to know and configure their systems right. Feedback can still be collected from the parts of the community that decide to try the CMake build to improve it, but this has yet to eclipse the official Makefile.

@Nerei thanks again for making this dramatic improvement over the state of the previous CMake build -- this can be merged for the next release -- but there is follow-up work to be done before this is the one true build.

@bhack
Contributor
bhack commented Jan 29, 2015

@shelhamer If cmake build doesn't works for core developers environments how we can assure users that we are not diverging Makefile and Cmake on future commits that will impact Makefiles? Probably Cmake will be broken because core members don't edit it and never use it regularly.

@Nerei
Nerei commented Feb 1, 2015

@longjon Well, thanks for your feedback

I don't think it is good to soften requirements and then blame people. CMake 2.8.7 doesn't search for curand. However, let me lavishly contribute 2.8.7 support just by adding single code line.

In fact, I never tested mkl and openblas as well. I relied on previous cmake contributor assuming he had tested it carefully. Well, today, I got a trial of the latest mkl for linux and reworked the script a bit to support intel64 and make it more convenient. AFAIU, proper multi-mkl-version multi-platform multi-mkl-configuration MKL search script require much more work and testing. But current version is enough to specify just single library and single include similar to Makefile.config

But, okay, easy enough to specify, let's pop open CMakeCache.txt.

  • Whoa, over seven hundred lines, that's ten times bigger than Makefile.config, there's no way I can just read all the options.

LOOOOOL, you will have to pay for my doctor. I can't laugh so much. Well, my recommendation for you is: cd cmake_build && cmake-gui . (note dot as parameter). Enjoy! Are you console/ssh fan? Then do: cd cmake_build && ccmake . The tools will give your everything you need to configure and catch all issue conviniently.

I added debug suffix intentionally to allow debug and release install to the same folder. It's usually a good practice and convenient for library users. Nonetheless, symlinks are creates without the suffix. Consider the following variants: 1) removing the suffix in cmake 2) making it optional/configurable in cmake 3) consider updating your scripts for debug suffix support.

Cuda.cmake and Modules/FindCuda.cmake are highly consistent and the naming is very well done (and tested in many projects). Former is not a classical find_package() script. Former is adapter/extender for the latter.

@Nerei
Nerei commented Feb 1, 2015

@shelhamer

Actually, I've seen such situations many many times. For example, the time when people did migrate from svn to git.

There was so many complains from developers. Git is difficult. Svn is simple and beautiful. Git requre additional knowlege. Git hampers their work by requiring adidional actions (add-commit-push instead of just single commit). People have to learn long manuals about git commands and parameters. So many complains and heavy opposition. But eventually all of them migrated to git. And have been working with it for serveral years and don't mind their life without git now.

The same with cmake. Eventually you will migrate to it. Your team is just lacking a guy who know cmake well. I'm here not to war and argue, but to help and share my experience.

@akosiorek
Contributor

I relied on previous cmake contributor that he had tested it carefully.

In the discussion around my pull request we decided not to support MKL back then. OpenBLAS has worked like a charm on a couple of platforms we used, though.

Anyway, great work @Nerei. I learned so much just by reading your scripts.

@bhack
Contributor
bhack commented Feb 1, 2015

@Nerei Here nobody is making a war. The general problem IMHO is still a policy one. Also if you are an experienced opencv core member and infrastructure maintainers with several years of experience I think that BVLC core members don't want to maintain portion of code that cannot do them self. This is a general problem also for other PRs. Without an enlarged modules/components maintainers system every contribution that they doesn't want to maintain probably will not merged.

@bhack
Contributor
bhack commented Feb 1, 2015

I want also renew my support on Cmake to @Nerei as I've done a little bit with the old cmake scripts but only if we can find a way to improve its robustness by having a large user base (this mean also to don't merge future commits that impact Makefile before someone have analyzed Cmake build system impact to avoid to have Cmake often broken and so with a very low user base and platforms coverage).

@Nerei
Nerei commented Feb 1, 2015

@akosiorek Thank you ;-)

@shelhamer
Member

@Nerei I'll do a last pass on this to include it in the next release soon after the ICML deadline 02/06. This is progress, and will help those it helps.

Could you check this against #1740 with regards to 10.10 and the Accelerate / vecLib differences? Furthermore CUDA 7 is compatible with libc++ so the OS X install will be dramatically simplified -- please add a check for the CUDA 7 case that turns off the libstdc++ work around for OS X -- if I have a chance to add the check to the Makefile before then I'll point out the patch.

Thanks for the build effort! One usually hears only complaints about builds as with most infrastructure, but it's an important task.

@Nerei
Nerei commented Feb 1, 2015

@shelhamer Sure. In couple days as soon as get access to os x.

@Nerei
Nerei commented Feb 6, 2015

@shelhamer

I tested on Yosemite with CUDA 7.0 and libc++. It works like a charm! Without any additional manipulations with homebrew scripts to enable libstdc++ for dependencies. Also homebrew installs the latest opencv 2.4.10.1 which already includes patch opencv/opencv@32f6e1a and removes the nvcc problems on macos with undefined intrinsic (but caffe supports older opencv too).

Regarding #1740, I originally opened the PR with already gtest forced to use own tuple and with vecLib/Accelerate support added. Now I added the following OSX/Cuda 7.0 logic:

  1. cmake detects osx version
  2. if osx version < 10.9 then do nothing (libstdc++ is default already)
  3. if osx version >= 10.9 then
    • creates USE_libstdcpp option in cmake
    • the option is ON by default if CUDA version < 7.0 otherwise OFF by default
    • the option controls adding -stdlib=libstdc++ flag

Cuda version has only impact on default value. User can always overwrite it via -DUSE_libstdcpp=ON/OFF command line parameter or in cmake-gui. So user can always select which library to use.

Another good news that Octave interface compiled without any problems with Cuda 7.0 and libc++. I don't have Matlab here to test.

(rebased onto the latest origin/dev)

@shelhamer
Member

@Nerei nice update -- I switched to CUDA 7 + libc++ myself last night with a small Makefile patch 33a56e0. It's nice to leave the brew editing behind.

I will do a last pass and test build on my machine and a compute server to merge for our next release.

@shelhamer
Member

@Nerei here are a few observations from my attempt at using this build.

  • it seems this is looking for OpenCV OpenCL but I'd rather not have that dependency since we don't yet ourselves support OpenCL:
CMake Error at /usr/local/share/OpenCV/OpenCVModules-release.cmake:109 (set_property):
  set_property could not find TARGET opencv_ocl.  Perhaps it has not yet been
  created.
Call Stack (most recent call first):
  /usr/local/share/OpenCV/OpenCVModules.cmake:104 (include)
  /usr/local/share/OpenCV/OpenCVConfig.cmake:49 (include)
  cmake/Dependencies.cmake:60 (find_package)
  CMakeLists.txt:24 (include)
  • There seems to be an issue with OpenCV version detection as I have 2.4.10 installed but CMake is erring out at not finding 2.4.9:
CMake Error at /usr/local/share/OpenCV/OpenCVModules.cmake:114 (message):
  The imported target "opencv_core" references the file

     "/usr/local/lib/libopencv_core.2.4.9.dylib"

  but this file does not exist.  Possible reasons include:

This is on OS X 10.10 with homebrew dependencies.

  • On Ubuntu 14.04, the build fails to link and complains of undefined OpenCV symbols. I think this is because it is finding /usr/lib/opencv instead of /usr/lib/opencv2 from my brief inspection of the make VERBOSE=1.
../lib/libcaffe.so: undefined reference to `cv::imread(cv::String const&, int)'
../lib/libcaffe.so: undefined reference to `cv::String::allocate(unsigned long)'
../lib/libcaffe.so: undefined reference to `cv::String::deallocate()'
  • Even when I explicitly set the BLAS option to MKL the USE_MKL flag is not set for compilation which is needed to select our vector math implementation in device_alternate.hpp.
@Nerei
Nerei commented Feb 7, 2015

@shelhamer Regarding MKL, added it. Good catch, I didn't know! For all other comments nothing required from me since that are opencv/cmake features or configuration errors. Let's examine all them.

it seems this is looking for OpenCV OpenCL but I'd rather not have that dependency since we don't yet ourselves support OpenCL:

Caffe cmake invokes OpenCV search here using the following lines:

# ---[ OpenCV
find_package(OpenCV QUIET COMPONENTS core highgui imgproc imgcodecs)
if(NOT OpenCV_FOUND) # if not OpenCV 3.x, then imgcodecs are not found
  find_package(OpenCV REQUIRED COMPONENTS core highgui imgproc)
endif()

As you can see, it requests minimal number of components and doesn't call for OpenCL support. This is the most proper way to find OpenCV library. But OpenCL dependency may still exists indirectly, for example if opencv_core depends on it. Internal OpenCV kitchen it not what you can control in caffe cmake. You have to use older OpenCV or manually recompile OpenCV in a different configuration without OpenCL support.

There seems to be an issue with OpenCV version detection as I have 2.4.10 installed but CMake is erring out at not finding 2.4.9:

This is not an issue at all. You searched for opencv 2.4.9 before and cmake cached old libs and each time generates Makefile with old libs. After version switch you have to just invalidate Cmake cache. This can be done in of the following ways:

  • rm CMakeCache.txt (disadvantage: loosing all other settings)
  • open cmake-gui, type OpenCV in search filter, select and remove all opencv variables (perhaps except OpenCV_DIR which should point to proper OpenCV build or installation)
  • in command line cmake <build_dir> -UOpenCV* -DOpenCV_DIR=<path> (-U means unset, note * symbol)

Automatic invalidation is difficult and not implemented in OpenCVConfig.cmake script (belongs to OpenCV package not to Caffe). Every CMake user even with little experience knows about such possible issues (with any library), so automatic invalidation is not very required.

On Ubuntu 14.04, the build fails to link and complains of undefined OpenCV symbols. I think this is because it is finding /usr/lib/opencv instead of /usr/lib/opencv2 from my brief inspection of the make VERBOSE=1.

Again configuration error. Perhaps you have several OpenCV's installed. Difficult to diagnose remotely. You can understand which opencv is used by analyzing cmake configuration summary log I added. Try to invalidate OpenCV variables in cache and specify proper OpenCV_DIR as above. For apt-get-installed opencv on 14.04, specify OpenCV_DIR=/usr/share/OpenCV. Use cmake-gui for convenience.

@Nerei
Nerei commented Feb 7, 2015

@shelhamer Don't hesitate to ask for more help and questions :-)

@shelhamer
Member

@Nerei thanks for pointing out the OpenCV search is minimal, as it should be. I fixed the configuration fails by rooting out a stale CMake configuration from an old brew package -- I did try invalidating the CMake cache but it was the old configuration causing the problem. Once that was out of the way my OpenCV with OpenCL disabled was detected and the build completed. So yeah, not the Caffe CMake build's problem.

The last concerns for merge are documentation and alignment with our examples and docs:

  1. For documentation we can pull in your pull request description for the new install docs -- although this will still be the unofficial build option for now.
  2. build/examples and build/tools are used in examples and scripts for instance
  3. The ipython notebooks examples do relative imports of the module so that it works out of the box. While configuring PYTHONPATH is obviously the right way, convenience is important for examples and tutorials.

2 & 3 are collected in the install target. Perhaps this target could symlink the expected files in the source dir? Or the Makefile build dir and CMake install dir could be aligned. Thoughts @longjon @jeffdonahue ?

@shelhamer shelhamer referenced this pull request Feb 9, 2015
Merged

Next: release candidater #1849

@Nerei
Nerei commented Feb 10, 2015

@shelhamer Unsure that I clearly understand you. Currently behavior is completely similar to Makefile. I maintain different structures for build and install trees.

For build tree, all you need is to invoke make symlink command and which creates symbolic link <caffe_root>/build -> <cmake_build_dir>. Build tree repeats Makefile's tree structure, so all examples should work in box with build tree after creating the symlink.

As for install tree, I also copied Makefile's behavior. All binaries are accumulated in <instal_dir>/bin directory. Originally install dir assumes copying files for further deploying without connection with sources, so a bit strange to create symlinks.

We may implement completely self-contained install tree with examples, notebooks, documentation and helper scripts. But this will be definitely superset of Makefiles features. And before doing this I would discuss with all you possible design of this stuff and convenience for everyone. And some things that perhaps should be done, like removing extra '.bin' suffix (since It doesn't make sense on Windows) or re-implementing all bash scripts in python (since it more cross platform and unified) and so on.

Anatoly Baks... and others added some commits Feb 6, 2015
@Nerei
Nerei commented Feb 10, 2015

FYI, the PR has merge conflicts with #1703 in .proto file although I never modified it and even after I rebased onto the latest origin/dev.

@shelhamer
Member

I merged this with squashed commits and a slight doc note in c5a6a8c. Thanks @Nerei!

More thorough documentation will follow shortly based on your PR comment.

@shelhamer shelhamer closed this Feb 17, 2015
@Nerei
Nerei commented Feb 17, 2015

@shelhamer Thank you! Please feel free to ask me to review future cmake pull requests!

@shelhamer
Member

@Nerei thank you, that is a helpful and welcome offer.

@boaz001 boaz001 commented on the diff Mar 24, 2015
cmake/Modules/FindAtlas.cmake
-set(LOOKED_FOR
+find_library(Atlas_CBLAS_LIBRARY NAMES ptcblas_r ptcblas cblas_r cblas PATHS ${Atlas_LIB_SEARCH_PATHS})
+find_library(Atlas_BLAS_LIBRARY NAMES atlas_r atlas PATHS ${Atlas_LIB_SEARCH_PATHS})
+find_library(Atlas_LAPACK_LIBRARY NAMES alapack_r alapack lapack_atlas PATHS ${Atlas_LIB_SEARCH_PATHS})
@boaz001
boaz001 Mar 24, 2015

CMake reports: Could NOT find Atlas (missing: Atlas_LAPACK_LIBRARY). Because on this line it searches for a library named 'alapack' while on my system it is called 'lapack' (/usr/lib64/atlas/liblapack.so). Atlas version installed: 3.8.4-2.el6. Is this a typo in the FindAtlas.cmake file?

@Nerei
Nerei Mar 24, 2015

Actually I don't know whether it is a typo or not. I copied these lines from a file from previous version of caffe cmake scrips (from here)

I don't know if on a OS it is not called as alapack. So the best solution is just to write the following:

+find_library(Atlas_LAPACK_LIBRARY NAMES alapack_r 
           alapack lapack_atlas lapack PATHS ${Atlas_LIB_SEARCH_PATHS})

i.e. just add lapack to the libraries search list.

@boaz001 Would you mind you create such PR? Or I can do this.

@baimin1
baimin1 commented Jun 19, 2015

Could NOT find HDF5 (missing: HDF5_LIBRARIES), but i have install hdf5-1.8.14, and set the path
as:
PATH=$PATH:/usr/local/hdf5
PATH=$PATH:/data/home/tools_for_caffe/hdf5-1.8.14
and source, but still Could NOT find HDF5, anyone can help me?

@akosiorek
Contributor

@baimin1 CMake uses Find*.cmake scripts for finding particular packages, in this case it's the FindHDF5.cmake script delivered with your CMake installation. It's best to look in this script which variables it does use to determine location of a given package. E.g. in my installation (3.22) it uses HDF5_ROOT variable, either enviornmental or specified with configuration as cmake -DHDF5_ROOT ....

@flowirin
flowirin commented Jul 9, 2015

c:>git clone git@github.com:BVLC/caffe.git.
Cloning into 'caffe.git.'...
Warning: Permanently added the RSA host key for IP address '192.30.252.129' to the list of known hosts.
Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

i'm new to this. how do i fix this error?

@Nerei
Nerei commented Jul 9, 2015

Add your public ssh key to your github account

@achristensen56

Hi,
I'm trying to install Caffe with cmake on a macbookpro 2015 without a dedicated GPU. When I do:

cmake . -DBUILD_SHARED_LIB=ON -DCPU_ONLY=ON

I get the following output:

` cmake . -DBUILD_SHARED_LIB=ON -DCPU_ONLY=ON
-- Boost version: 1.57.0
-- Found the following Boost libraries:
-- system
-- thread
-- filesystem
-- Found gflags (include: /usr/local/include, library: /usr/local/lib/libgflags.dylib)
-- Found glog (include: /usr/local/include, library: /usr/local/lib/libglog.dylib)
-- Found PROTOBUF Compiler: /usr/local/bin/protoc
-- Found lmdb (include: /usr/local/include, library: /usr/local/lib/liblmdb.dylib)
-- Found LevelDB (include: /usr/local/include, library: /usr/local/lib/libleveldb.dylib)
-- Found Snappy (include: /usr/local/include, library: /usr/local/lib/libsnappy.dylib)
-- -- CUDA is disabled. Building without it...
-- OpenCV found (/usr/local/share/OpenCV)
-- Found standalone vecLib.framework
-- NumPy ver. 1.10.1 found (include: /Users/ameliachristensen/anaconda2/lib/python2.7/site-packages/numpy/core/include)
-- Boost version: 1.57.0
-- Found the following Boost libraries:
-- python

-- Could NOT find Doxygen (missing: DOXYGEN_EXECUTABLE)

-- ******************* Caffe Configuration Summary *******************
-- General:
-- Version : 1.0.0-rc3
-- Git : unknown
-- System : Darwin
-- C++ compiler : /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
-- Release CXX flags : -O3 -DNDEBUG -fPIC -Wall -Wno-sign-compare -Wno-uninitialized
-- Debug CXX flags : -g -fPIC -Wall -Wno-sign-compare -Wno-uninitialized

-- Build type : Release

-- BUILD_SHARED_LIBS : ON
-- BUILD_python : ON
-- BUILD_matlab : OFF
-- BUILD_docs : ON
-- CPU_ONLY : ON
-- USE_OPENCV : ON
-- USE_LEVELDB : ON
-- USE_LMDB : ON

-- ALLOW_LMDB_NOLOCK : OFF

-- Dependencies:
-- BLAS : Yes (vecLib)
-- Boost : Yes (ver. 1.57)
-- glog : Yes
-- gflags : Yes
-- protobuf : Yes (ver. 2.6.1)
-- lmdb : Yes (ver. 0.9.14)
-- LevelDB : Yes (ver. 1.18)
-- Snappy : Yes (ver. 1.1.3)
-- OpenCV : Yes (ver. 2.4.12)

-- CUDA : No

-- Python:
-- Interpreter : /Users/ameliachristensen/anaconda2/bin/python2.7 (ver. 2.7.11)
-- Libraries : /Users/ameliachristensen/anaconda2/lib/libpython2.7.dylib (ver 2.7.11)

-- NumPy : /Users/ameliachristensen/anaconda2/lib/python2.7/site-packages/numpy/core/include (ver 1.10.1)

-- Documentaion:
-- Doxygen : No

-- config_file :

-- Install:

-- Install path : /Users/ameliachristensen/Documents/caffe-master/install

-- Configuring done
-- Generating done
CMake Warning:
Manually-specified variables were not used by the project:

BUILD_SHARED_LIB

-- Build files have been written to: /Users/ameliachristensen/Documents/caffe-master`

however when I do:

make -j 4 && make install

The install fails on not finding the file 'cblas.h'... any hints?

@Nerei
Nerei commented Feb 15, 2016

Without debugging no idea why install requires cblas.h

p.s. BUILD_SHARED_LIBS (should end with s)

@EasonD3
EasonD3 commented Apr 20, 2016

Thanks for the tutorial. I'm able to import Caffe using CMake in my own project. But I still cannot figure out how the system finds Caffe through find_package(Caffe) without setting Caffe_DIR in the CMakeLists.txt in my own project.

I've searched a lot about that but haven't found any answer yet.. Any comments will be very much appreciated!

@Nerei
Nerei commented Apr 20, 2016

You have to set Caffe_DIR. Just pass it once as CMake parameter. Future cmake runs don't require passing it again sine it will be cached.

cmake -DCaffe_DIR=<...>  ...
@EasonD3
EasonD3 commented Apr 20, 2016

@Nerei, for my case, I didn't set Caffe_DIR (I just did cmake ..). After CMake compilation, Caffe can be successfully found in my own project via find_package(Caffe). I tried to look into the compilation related files, but haven't found any clue about how the system can find Caffe.

@InfiniteLifeH
InfiniteLifeH commented May 23, 2016 edited

Hello.
Trying to use Caffe C++ API in my project with Visual Studio, Windows. I have build Caffe sucsesfully according to WIndows installation guide.
Can someone advise how to include Caffe to my project? I'm new to Cmake, so I tried to do following, may be something naive - created CMakeLists.txt with

> cmake_minimum_required(VERSION 2.8.8)
> 
> find_package(Caffe)
> include_directories(${Caffe_INCLUDE_DIRS})
> add_definitions(${Caffe_DEFINITIONS})    # ex. -DCPU_ONLY
> 
> add_executable(caffeinated_application main.cpp)
> target_link_libraries(caffeinated_application ${Caffe_LIBRARIES})

and have set Caffe_DIR to root path to Caffe project. But after every Configure try, Cmake says that it's not able to find Caffe package, and value of Caffe_DIR gets replaces by "NOT_FOUND"

@Nerei
Nerei commented May 23, 2016

Please ask on caffe forums. Actually you should set Caffe_DIR to caffe build directory, not source one.

@nineilpitt

Hi all,

I have installed caffe and have tested with a mnist script. That one is working, also with the gpu :)

Now, I am trying to work with the matlab interface. I am testing with the basic demo from CAFFE_FOLDER/matlab/demo

However, I am getting the error:

[libprotobuf FATAL google/protobuf/stubs/common.cc:61] This program requires version 3.0.0 of the Protocol Buffer runtime library, but the installed version is 2.5.0. Please update your library. If you compiled the program yourself, make sure that your headers are from the same version of Protocol Buffers as your link-time library. (Version verification failed in "google/protobuf/any.pb.cc".)

I have verified that I have the proto version 3.00 in my path with 'protoc --version' in console and also in matlab with unix('protoc --version') and I am getting version 3.00. However, still has the previous error.

Any suggestions ?

Thanks.

@moumitaTora

I am continuously getting this error while trying to install Caffe using CMake:
/usr/lib/gcc/x86_64-linux-gnu/4.8/../../../x86_64-linux-gnu/libglog.so: undefined reference to `google::FlagRegisterer::FlagRegisterer(char const_, char const_, char const_, char const_, void_, void_)'

Please help!

@ssamira
ssamira commented Jul 4, 2016

Hello,

I am trying to use Caffe API in my project in visual studio but I have the same issue as @InfiniteLifeH has, @Nerei I set Caffe_DIR to build and still have the problem. I am getting this warning in Cmake :

CMake Warning at (find_package):
By not providing "FindCaffe.cmake" in CMAKE_MODULE_PATH this project has
asked CMake to find a package configuration file provided by "Caffe", but
CMake did not find one.

Could not find a package configuration file provided by "Caffe" with any of
the following names:

CaffeConfig.cmake
caffe-config.cmake

Add the installation prefix of "Caffe" to CMAKE_PREFIX_PATH or set
"Caffe_DIR" to a directory containing one of the above files. If "Caffe"
provides a separate development package or SDK, be sure it has been
installed.
Any suggestions?

Thanks,

@psy770
psy770 commented Jul 15, 2016

hi, can anyone help me for this error?

[----------] 2 tests from BatchReindexLayerTest/1, where TypeParam = caffe::CPUDevice
[ RUN ] BatchReindexLayerTest/1.TestForward
[ OK ] BatchReindexLayerTest/1.TestForward (0 ms)
[ RUN ] BatchReindexLayerTest/1.TestGradient
[ OK ] BatchReindexLayerTest/1.TestGradient (87 ms)
[----------] 2 tests from BatchReindexLayerTest/1 (87 ms total)

[----------] 5 tests from ImageDataLayerTest/1, where TypeParam = caffe::CPUDevice
[ RUN ] ImageDataLayerTest/1.TestReshape
[ OK ] ImageDataLayerTest/1.TestReshape (46 ms)
[ RUN ] ImageDataLayerTest/1.TestShuffle
[ OK ] ImageDataLayerTest/1.TestShuffle (106 ms)
[ RUN ] ImageDataLayerTest/1.TestRead
[ OK ] ImageDataLayerTest/1.TestRead (103 ms)
[ RUN ] ImageDataLayerTest/1.TestResize
[ OK ] ImageDataLayerTest/1.TestResize (122 ms)
[ RUN ] ImageDataLayerTest/1.TestSpace
[ OK ] ImageDataLayerTest/1.TestSpace (63 ms)
[----------] 5 tests from ImageDataLayerTest/1 (440 ms total)

[----------] Global test environment tear-down
[==========] 1096 tests from 150 test cases ran. (68513 ms total)
[ PASSED ] 1095 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] NeuronLayerTest/0.TestPReLUForward, where TypeParam = caffe::CPUDevice

1 FAILED TEST
make[3]: *** [src/caffe/test/CMakeFiles/runtest] Error 1
make[2]: *** [src/caffe/test/CMakeFiles/runtest.dir/all] Error 2
make[1]: *** [src/caffe/test/CMakeFiles/runtest.dir/rule] Error 2
make: *** [runtest] Error 2

@riteshpradhan
riteshpradhan commented Jul 28, 2016 edited

make symlink
make: *** No rule to make targetsymlink'. Stop.`

it's not working.

@EmpireofQin

i run into this problem:

[ 88%] Building CXX object src/caffe/CMakeFiles/caffe.dir/layer.cpp.o
Linking CXX shared library ../../lib/libcaffe-d.so
/usr/bin/ld: /usr/local/lib/libcblas.a(cblas_sgemv.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC
/usr/local/lib/libcblas.a: error adding symbols: Bad value
collect2: error: ld returned 1 exit status
make[2]: *** [lib/libcaffe-d.so.1.0.0-rc3] Error 1
make[1]: *** [src/caffe/CMakeFiles/caffe.dir/all] Error 2
make: *** [all] Error 2

any idea? thanks a lot ^_^

@flowirin
flowirin commented Oct 5, 2016

try compiling with -fPIC ?

flow ir in

On 5 October 2016 at 14:09, EmpireofQin notifications@github.com wrote:

i run into this problem:

[ 88%] Building CXX object src/caffe/CMakeFiles/caffe.dir/layer.cpp.o
Linking CXX shared library ../../lib/libcaffe-d.so
/usr/bin/ld: /usr/local/lib/libcblas.a(cblas_sgemv.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC
/usr/local/lib/libcblas.a: error adding symbols: Bad value
collect2: error: ld returned 1 exit status
make[2]: *** [lib/libcaffe-d.so.1.0.0-rc3] Error 1
make[1]: *** [src/caffe/CMakeFiles/caffe.dir/all] Error 2
make: *** [all] Error 2

any idea? thanks a lot ^_^


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1667 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AMnuraAjwwK-tenDParQaW62SaK6K9SDks5qwvjOgaJpZM4DNzQk
.

@EmpireofQin

I have tried this, but dosen't work

@flowirin
flowirin commented Oct 7, 2016

what is the error?

flow ir in

On 7 October 2016 at 14:21, EmpireofQin notifications@github.com wrote:

I have tried this, but dosen't work


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1667 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AMnurS7qJtoWVIRYoxcLhnEwHTZbCMnMks5qxZ6KgaJpZM4DNzQk
.

@EmpireofQin
EmpireofQin commented Oct 7, 2016 edited

I have solved this problem. I tried to edit CMakeCache.txt in the build file and changed

//Path to a library.
Atlas_CBLAS_LIBRARY:FILEPATH=<path to libcblas.a>

into

//Path to a library.
Atlas_CBLAS_LIBRARY:FILEPATH=/usr/lib/libcblas.so //<path to libcblas.so in my machine>

and it works.
i suppose it is because i have tried to install atlas things to many times LOL
thanks a lot @flowirin

@YinHT2016

@riteshpradh
hi,I met the same question as yours,could you share the solution?thanks a lot.
my email is 714789880@qq.com

@YinHT2016

@longjon
hi,there is a question I met:
make symlink
make: *** No rule to make targetsymlink'. Stop.`
I am lack of experience on the CMake and Linux ,could you tell me what shoud I do ?
Thanks a lot.

@EmpireofQin
EmpireofQin commented Oct 31, 2016 edited

Hi, I have error like this:

CMake Error at CMakeLists.txt:83 (add_dependencies):
  The dependency target "pycaffe" of target "pytest" does not exist.

I can still do next steps to finish compiling. What should I do to solve this error?
Hoping to get help, thanks a lot! ^_^

@viscropst

Who can tell me how to use intel mkl to compile caffe with cmake????

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment