[caffe] Add new port #2570

vlj · 2018-01-14T00:16:05Z

This port is adding caffe-1.0 to vcpkg.
While caffe2 is newer it's not completly backward compatible which motivates this port. Additionnaly Caffe has an opencl branch (this is so far the only Deep Learning Framework with an opencl backend so far) which I'd like to bring to vcpkg through features.

Initially there is no CUDA/OpenCV/Python/Matlab support.

vlj · 2018-01-14T21:08:20Z

Added python support

jasjuang · 2018-01-15T22:18:20Z

Just wondering did you check out @willyd 's port BVLC/caffe#5674? His port works with OpenCV and CUDA but he never submitted a PR to vcpkg (not sure why). Can you look into it and update this PR?

willyd · 2018-01-16T13:26:41Z

@vjl This port has been on my TODO list for a while now. I have one that works but is different from yours in some ways:

Mine is based off the caffe/windows branch
Mine supports CUDA and OpenCV
Your supports Python. I wonder how this is possible, vcpkg overwrites the PATH variable so CMake can't find my Anaconda Python.
Mine supports Dynamic linking (Also supports static linking in theory, but I haven't tried to build it yet)

I haven't submitted a PR since I needed to make some changes to the caffe code base for the port to work and I still need to find some time to merge the changes in the caffe windows branch.

Would you like to collaborate on this?

vlj · 2018-01-17T20:18:21Z

I added CUDA and OpenCV using @willyd branch. I checked both and OpenCV works, but Cuda fails with the following :
"fatal error C1189: #error: -- unsupported Microsoft Visual Studio version! Only the versions 2012, 2013, 2015 and 2017 are supported!"

I have vanilla Python 3 installed and I didn't get issue at all. I suspect vcpkg uses it's own python, but relies on Python_packages_path envvar to find Numpy.

For me dynamic linking builds but doesn't work, caffe executable fails because protobuf is misleaded by a static symbol found twice. This is an instance of this bug : BVLC/caffe#1917

The branch is based on the 1.0 release with patch backported from the Windows branch.

jasjuang · 2018-01-17T20:26:16Z

@vlj I suspect you are using VS2017 15.5, unfortunately you need to downgrade it to VS2017 15.4 for CUDA to work. See https://devtalk.nvidia.com/default/topic/1027299/cuda-setup-and-installation/cuda-9-failed-to-support-the-latest-visual-studio-2017-version-15-5/

willyd · 2018-01-17T21:07:13Z

@vlj There is a detail writeup on a to workaround the CUDA issue here: BVLC/caffe#5654 (comment)

As @jasjuang mentionned the safest route is probably to downgrade until NVIDIA releases an update.

willyd · 2018-01-17T21:13:13Z

I have vanilla Python 3 installed and I didn't get issue at all. I suspect vcpkg uses it's own python, but relies on Python_packages_path envvar to find Numpy.

I guess CMake looks up the Python registry entries. This won't work with conda environments and virtual envs. Sad.

For me dynamic linking builds but doesn't work, caffe executable fails because protobuf is misleaded by a static symbol found twice. This is an instance of this bug : BVLC/caffe#1917

Yeah, I had to fiddle a lot with the build to get this to work. Changed caffeproto from STATIC to OBJECT and changed the protoc command line call in Protobuf.cmake. See this commit willyd/caffe@8aee77a

vlj · 2018-01-20T19:45:29Z

I updated the port so that it uses dynamic instead of static, using @willyd tips (declaring proto as OBJECT instead of STATIC). I also had to use the same tricks for pycaffe.

With these modifications I got Deepdream script working : https://www.alanzucconi.com/2016/05/25/generating-deep-dreams/
natively on Windows with dynamic linked caffe.

Unfortunatly the static version doesn't work, it looks like it strips static symbol when linking proto (hence the layers descriptions are not found when pycaffe is loaded and it's not possible to use any caffeproto file). I will import the export related code in static build in the Windows branch of caffe I guess.

willyd · 2018-01-20T23:51:51Z

@vjl This is great! Use the /wholearchive linker flag. This way you won't need to use the pragma trick that I implemented in the windows branch. I am going to drop this along VS 2013 support anyway.

vlj · 2018-01-21T17:38:11Z

Actually I had to revert back to using static linking to enable CUDA support ; OBJECT doesn't accept *.cu file.
I got Deepdream python script working however it gives me some strange result with the sky picture, as if it was random :

willyd · 2018-01-22T00:52:04Z

@vjl It was only necessary to use OBJECT for the caffeproto library. This library does not include any .cu files. You have to use the WINDOWS_EXPORT_ALL_SYMBOLS CMake trick though.

Can't comment on the DeepDream result I always wanted to try it but never got to it. I usually try MNIST or just a simple linear regression to see if everything is working fine.

vlj · 2018-01-22T20:22:29Z

I opened an issue here : BVLC/caffe#6192

ras0219-msft · 2018-01-23T15:06:28Z

LMK when this is ready to be merged :)

vlj · 2018-01-24T20:52:55Z

I'm a bit puzzled since the caffe prebuilt binaries from this link : https://github.com/BVLC/caffe/tree/windows
works properly with the deepdream script.

Unit tests mostly pass.
There's some patch I didn't import like the one moving Template from headers to cpp files (but I think using /WHOLEARCHIVE flag is equivalent) and some fix to MakeTempDirectory but I'm not sure it's used for anything but tests. There's also leveldb which is missing but it doesn't seem to be used to load caffemodel files.
I'm investigating

vlj · 2018-01-25T20:56:21Z

It looks like protobuf fails to load the caffemodel file, I don't know why. I modified the proto definition by adding an underscore to STRICT and PERMISSIVE, but removing them and manually changing the var definition in the autogenerated file doesn't seem to have any influence.

vlj · 2018-01-27T22:26:40Z

I fixed the issue, actually I missed a hunk (open needs the binary flag to read caffeproto files).
The branch should be ready to merge I think.

vlj · 2018-02-03T18:01:22Z

Feedback ?

jasjuang · 2018-02-04T05:43:10Z

Looks great to me! can't wait to see it to be merged

willyd · 2018-02-04T14:40:03Z

ports/caffe/0001-Use-vcpkg-dependencies.patch

+-list(APPEND Caffe_INCLUDE_DIRS PUBLIC ${HDF5_INCLUDE_DIRS})
+-list(APPEND Caffe_LINKER_LIBS PUBLIC ${HDF5_LIBRARIES} ${HDF5_HL_LIBRARIES})
+find_package(hdf5 COMPONENTS HL REQUIRED)
+list(APPEND Caffe_LINKER_LIBS PUBLIC hdf5-shared hdf5_hl-shared)


x64-windows-static would require to link to hdf5-static here.

willyd · 2018-02-04T14:42:59Z

ports/caffe/0002-Fix-permissive-strict.patch

+-    STRICT = 0;
+    STRICT_ = 0;
+     // PERMISSIVE requires only the count (num*channels*height*width) to match.
+-    PERMISSIVE = 1;


We should use the NO_STRICT compile definition instead.

willyd · 2018-02-04T14:43:13Z

ports/caffe/0002-Fix-permissive-strict.patch

+   repeated DimCheckMode blob_share_mode = 1002;
+   enum DimCheckMode {
+-    STRICT = 0;
+-    PERMISSIVE = 1;


willyd · 2018-02-04T17:18:19Z

ports/caffe/0004-Export-fix.patch

+ set_target_properties(pycaffe PROPERTIES PREFIX "" OUTPUT_NAME "_caffe")
+ target_include_directories(pycaffe PUBLIC ${PYTHON_INCLUDE_DIRS} ${NUMPY_INCLUDE_DIR})
+ target_link_libraries(pycaffe PUBLIC ${Caffe_LINK} ${PYTHON_LIBRARIES})
+set_target_properties(pycaffe PROPERTIES LINK_FLAGS "/WHOLEARCHIVE:caffe.lib")


Would a generator expression work here: /WHOLEARCHIVE:$<TARGET_FILE_NAME:caffe>?

willyd · 2018-02-04T17:21:58Z

ports/caffe/0004-Export-fix.patch

+ target_include_directories(gtest PUBLIC ${Caffe_SRC_DIR})
+ target_compile_definitions(gtest PUBLIC -DGTEST_USE_OWN_TR1_TUPLE)
+-
+set_target_properties(gtest PROPERTIES LINK_FLAGS_RELEASE "/WHOLEARCHIVE:caffe")


Should probably be in the LINK_INTERFACE_LIBRARIES of the caffe lib instead.

willyd · 2018-02-04T17:27:01Z

ports/caffe/portfile.cmake

+file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/debug/include)
+file(REMOVE_RECURSE ${CURRENT_PACKAGES_DIR}/debug/share)
+
+#file(COPY ${CURRENT_PACKAGES_DIR}/debug/lib/caffe-d.dll DESTINATION ${CURRENT_PACKAGES_DIR}/debug/bin)


You should use vcpkg_fixup_cmake_targets too!

willyd · 2018-02-04T17:27:26Z

@vlj Please see my comments on your patches.

cbezault · 2019-08-28T22:50:28Z

/azp run

azure-pipelines · 2019-08-28T22:50:37Z

Azure Pipelines successfully started running 1 pipeline(s).

NancyLi1013 · 2020-01-14T05:56:20Z

/azp run

NancyLi1013 · 2020-01-14T06:48:24Z

ports/caffe/CONTROL

@@ -0,0 +1,16 @@
+Source: caffe
+Version: 1.0
+Build-Depends: lmdb, gflags, glog, boost-thread, boost-filesystem, hdf5, protobuf, openblas


Could you please add Homepage here?

NancyLi1013 · 2020-01-14T06:50:16Z

ports/caffe/portfile.cmake

+
+if (VCPKG_TARGET_ARCHITECTURE STREQUAL x86)
+    message(FATAL_ERROR "Caffe cannot be built for the x86 architecture")
+endif()


if (VCPKG_TARGET_ARCHITECTURE STREQUAL x86) message(FATAL_ERROR "Caffe cannot be built for the x86 architecture") endif()

We can use vcpkg_fail_port_to_install(ON_ARCH "x86") instead.

NancyLi1013 · 2020-01-14T06:50:52Z

ports/caffe/portfile.cmake

+    message(FATAL_ERROR "Caffe cannot be built for the x86 architecture")
+endif()
+
+include(vcpkg_common_functions)


include(vcpkg_common_functions) is no longer needed.
Could you please remove this line?

NancyLi1013 · 2020-01-14T06:56:37Z

ports/caffe/portfile.cmake

+
+# install license
+file(COPY ${CURRENT_BUILDTREES_DIR}/src/caffe-1.0/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/caffe/LICENSE)
+file(RENAME ${CURRENT_PACKAGES_DIR}/share/caffe/LICENSE ${CURRENT_PACKAGES_DIR}/share/caffe/copyright)


Could you please update

file(COPY ${CURRENT_BUILDTREES_DIR}/src/caffe-1.0/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/caffe/LICENSE) file(RENAME ${CURRENT_PACKAGES_DIR}/share/caffe/LICENSE ${CURRENT_PACKAGES_DIR}/share/caffe/copyright)

as
file(INSTALL ${SOURTH_PATH}/LICENSE DESTINATION ${CURRENT_PACKAGES_DIR}/share/${PORT} RENAME copyright)?

NancyLi1013 · 2020-01-14T06:57:47Z

ports/caffe/portfile.cmake

+
+vcpkg_from_github(
+    OUT_SOURCE_PATH SOURCE_PATH
+    REPO BVLC/caffe


Is this a fork repo?

NancyLi1013 · 2020-01-14T06:59:00Z

ports/caffe/portfile.cmake

+vcpkg_from_github(
+    OUT_SOURCE_PATH SOURCE_PATH
+    REPO BVLC/caffe
+    REF 1.0


We prefer to use commit id instead of tag name in REF ?
Could you please update it?

NancyLi1013 · 2020-01-14T07:04:35Z

ports/caffe/portfile.cmake

+    ${CMAKE_CURRENT_LIST_DIR}/0006-Do-not-rename-caffe.bin.exe-to-avoid-confusing-ninja.patch
+    ${CMAKE_CURRENT_LIST_DIR}/0007-Fix-dependencies.patch
+    ${CMAKE_CURRENT_LIST_DIR}/0008-Make-proto-OBJECT-to-avoid-protobuf-loading-caffe-pr.patch
+    ${CMAKE_CURRENT_LIST_DIR}/0009-Fix-Cuda.patch


Can these patches be submitted upstream?

Besides this, vcpkg_apply_patches() is deprecated, if there are some patches needed by this port, please use vcpkg_from_github() instead.

dan-shaw · 2020-01-31T01:02:20Z

I’m going to close this PR because it’s been a long time since any update has been made. If this is something you’d like to see merged and work is being done, please reopen.

vlj force-pushed the caffe branch from 64eb14c to f1717d3 Compare January 14, 2018 18:43

vlj force-pushed the caffe branch from f1717d3 to 90d0b98 Compare January 14, 2018 21:44

vlj force-pushed the caffe branch from 90d0b98 to 3958a0b Compare January 16, 2018 22:19

vlj force-pushed the caffe branch 3 times, most recently from 7cf350e to 990c32c Compare January 20, 2018 17:19

vlj force-pushed the caffe branch from 990c32c to e5640a2 Compare January 20, 2018 23:59

vlj force-pushed the caffe branch from e5640a2 to be85141 Compare January 26, 2018 22:01

Add caffe port.

c9947b1

vlj force-pushed the caffe branch from be85141 to c9947b1 Compare January 27, 2018 22:25

willyd reviewed Feb 4, 2018

View reviewed changes

willyd mentioned this pull request Feb 21, 2018

[caffe] Add new port #2848

Closed

cbezault assigned Rastaban Aug 28, 2019

luncliff mentioned this pull request Sep 30, 2019

[libtorch] Add new port #8318

Closed

Rastaban assigned vicroms and unassigned Rastaban Oct 3, 2019

PhoebeHui assigned NancyLi1013 Jan 14, 2020

NancyLi1013 changed the title ~~Add caffe port.~~ [caffe] Add new port Jan 14, 2020

NancyLi1013 suggested changes Jan 14, 2020

View reviewed changes

NancyLi1013 assigned JackBoosY Jan 14, 2020

NancyLi1013 requested a review from JackBoosY January 14, 2020 07:07

JackBoosY added the waiting for response label Jan 19, 2020

dan-shaw closed this Jan 31, 2020

[caffe] Add new port #2570

[caffe] Add new port #2570

Conversation

vlj commented Jan 14, 2018

vlj commented Jan 14, 2018

jasjuang commented Jan 15, 2018

willyd commented Jan 16, 2018

vlj commented Jan 17, 2018

jasjuang commented Jan 17, 2018

willyd commented Jan 17, 2018

willyd commented Jan 17, 2018

vlj commented Jan 20, 2018

willyd commented Jan 20, 2018

vlj commented Jan 21, 2018

willyd commented Jan 22, 2018

vlj commented Jan 22, 2018

ras0219-msft commented Jan 23, 2018

vlj commented Jan 24, 2018

vlj commented Jan 25, 2018

vlj commented Jan 27, 2018

vlj commented Feb 3, 2018

jasjuang commented Feb 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

willyd commented Feb 4, 2018

cbezault commented Aug 28, 2019

azure-pipelines bot commented Aug 28, 2019

NancyLi1013 commented Jan 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dan-shaw commented Jan 31, 2020