Skip to content

Comments

Patch the windows branch with CMake support#4043

Closed
recheliu wants to merge 7 commits intoBVLC:windowsfrom
recheliu:windows
Closed

Patch the windows branch with CMake support#4043
recheliu wants to merge 7 commits intoBVLC:windowsfrom
recheliu:windows

Conversation

@recheliu
Copy link

This pull request will patch Caffe so its source code and CMake file can work on both Linux and Windows. We have tested the same files on both Windows and Linux as seen below.

This patch does not address the building of 3rd party libraries though.

The followings are the tests we conducted for this pull request:

Build via CMake on Windows. CPU-only:

  • Unit tests: PASSED.
  • Mnist: PASSED.
  • Cifar10/quick: PASSED.

Build via CMake on Windows. CPU & GPU:

  • Unit tests: PASSED.
  • Mnist: PASSED.
  • Cifar10/quick: PASSED.

Build via the original windows/Caffe.sln:

  • Unit tests (Cannot finished, but the original windows branch has the same issue).
  • Mnist: PASSED.
  • Cifar 10(quick): PASSED.

Build via CMake on Linux. CPU & GPU:

  • Mnist: PASSED.
  • Cifar10 (quick): PASSED.

Note that the LMDB for mnist/cifar10 databases were generated on Linux.

The following is our test environment;

[Windows]
Windows 7 x64 with Visual Studio 2013.
CUDA: 7.0 for CMake and 7.5 for the original .sln file.
boost: 1.55
CUDNN: 3.0
BLAS: OpenBLAS.
Python: WinPython 2.7.10.3.

[Linux]
Ubuntu 14.04.
CUDA: 7.0.
CUDNN: 3.0
BLAS: Atlas.
Python: 2.7.

…cmake.

Modified   CMakeLists.txt
Modified   cmake/Targets.cmake
Modified   cmake/Templates/caffe_config.h.in
Modified   examples/cpp_classification/classification.cpp
Modified   examples/mnist/convert_mnist_data.cpp
Modified   include/caffe/layer.hpp
Modified   include/caffe/solver.hpp
Modified   include/caffe/test/test_gradient_check_util.hpp
Modified   include/caffe/util/db.hpp
Modified   include/caffe/util/io.hpp
Modified   python/CMakeLists.txt
Modified   src/caffe/common.cpp
Modified   src/caffe/layer_factory.cpp
Modified   src/caffe/layers/contrastive_loss_layer.cpp
Modified   src/caffe/layers/data_layer.cpp
Modified   src/caffe/layers/scale_layer.cpp
Modified   src/caffe/parallel.cpp
Modified   src/caffe/solver.cpp
Modified   src/caffe/test/CMakeLists.txt
Modified   src/caffe/test/test_contrastive_loss_layer.cpp
Modified   src/caffe/util/hdf5.cpp
Modified   src/caffe/util/signal_handler.cpp
Modified   tools/CMakeLists.txt
1. [MOD] Automatically patched. The diff was created & patched via the following command (64d9e1acbefe1660efa22d9bd9630b877d93a0ae is the commit in our internal repo):
$ git diff -w 4b8fa07 64d9e1acbefe1660efa22d9bd9630b877d93a0ae > CaffeOnWindows.patch
# Copy CaffeOnWindows.patch to the current source folder.
$ patch -p1 < CaffeOnWindows.patch

Modified   cmake/Dependencies.cmake
Modified   cmake/External/gflags.cmake
Modified   cmake/Modules/FindLevelDB.cmake
Modified   include/caffe/common.hpp
1. [MOD] Manually fixed the parts that cannot be patched.

Add        cmake/Msvc.cmake
1. [1ST] Check in the cmake rule for Visual Studio. The rule will scan for registered classes and add code to force them to be linked.

Add        include/caffe/msvc/caffe_msvc.hpp
1. [1ST] Check in the misc setting for compatibility on Windows.

Add        include/caffe/msvc/caffe_msvc_io.hpp
1. [1ST] Check in the io routines that mimic the postix one with Windows API.

Add        include/caffe/msvc/caffe_msvc_registry.hpp
1. [1ST] Check in the code that define the rule to force the linking of registered classes.

Add        src/caffe/msvc/caffe_classes.cpp
1. [1ST] Check in classes that might be not registered if certain preprocessor is disabled.
…cmake.

Modified   cmake/Dependencies.cmake
Modified   cmake/External/gflags.cmake
Modified   cmake/Modules/FindLevelDB.cmake
Modified   include/caffe/common.hpp
1. [CLEAN] Remove local test labels.
…cmake.

Modified   cmake/Dependencies.cmake
Modified   cmake/External/gflags.cmake
1. [MOD] Correct the indents.
Modified   CMakeLists.txt
Modified   src/caffe/test/CMakeLists.txt
1. [MOD] Move the definition of CMAKE_BUILD from unit test to the root CMakeLists.txt.

Modified   include/caffe/msvc/caffe_msvc_registry.hpp
1. [MOD] Only include the CMake-generated caffe_classes.hpp when using CMake.
Modified   CMakeLists.txt
Modified   include/caffe/msvc/caffe_msvc_registry.hpp
Modified   src/caffe/test/CMakeLists.txt
1. [CLEAN] Remove local test labels.
Modified   include/caffe/util/io.hpp
1. [MOD] Manually rollback io.hpp.
@willyd
Copy link
Contributor

willyd commented Apr 29, 2016

@recheliu I have submitted a similar PR in #3990. You might be interested in this.

@shelhamer shelhamer mentioned this pull request Apr 29, 2016
@recheliu
Copy link
Author

Hi @willyd,

Thanks for your reference. I compared your PR with mine. While our fixes to CMake are pretty similar, the main differences are listed below:

  1. Mine can only build static libraries.
  2. Mine does not need Ninja.
  3. Mine does not handling the building of 3rd party libraries.

On the other hand, I saw that you almost finished your PR. The Travis CI failures are mainly related to styles. AppVeyor failure is that the file solver_factory.cpp is not compiled. If you can fixed them, I think that we can just use your PR.

@willyd
Copy link
Contributor

willyd commented Apr 29, 2016

@recheliu

the main differences are listed below:

Mine can only build static libraries.
Mine does not need Ninja.
Mine does not handling the building of 3rd party libraries.

I would say that another major difference is that the forced symbol linking is handled differently too. Your PR parses the sources files, while mine lists the symbols from the generated caffe.lib.

I don't actually require Ninja, but I haven't tested the Visual Studio generator so it might not work but it should not be too hard to support it as well.

On the other hand, I saw that you almost finished your PR.

Note quite finished, it still needs more testing and proper CI support but where getting close you are right.

AppVeyor failure is that the file solver_factory.cpp is not compiled. If you can fixed them, I think that we can just use your PR.

Just updated the PR with a fix for this.

Any contribution is welcome.

@recheliu recheliu closed this May 3, 2016
@recheliu recheliu deleted the windows branch October 6, 2016 01:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants