-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Download, build, install External dependencies via cmake #1017
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
赞!是不是可以在这个PR里顺便创建一个 /testing 子目录,里面放上一个简单的程序 third_party_dependencies_test.cc,同时用了这几个external libraries。这个程序编译如果没有错,则说明cmake能成功使用这些external libraries。
@wangkuiyi ok |
|
||
ExternalProject_Add( | ||
warpctc | ||
GIT_REPOSITORY "https://github.com/gangliao/warp-ctc.git" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warp-ctc官方目前支持力度不是很大,完全融入Paddle, 其实需要做一些定制化修改,可以考虑帮助他们改进,或者自己维护一个版本。
"${PROTOBUF_INSTALL_DIR}/lib/libprotoc.lib" CACHE FILEPATH "protobuf libraries." FORCE) | ||
SET(PROTOBUF_PROTOC_EXECUTABLE "${PROTOBUF_INSTALL_DIR}/bin/protoc.exe" CACHE FILEPATH "protobuf executable." FORCE) | ||
ELSE(WIN32) | ||
SET(PROTOBUF_LIBRARIES |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
发现linux下面是lib64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
option(WITH_DOC "Compile PaddlePaddle with documentation" OFF) | ||
option(WITH_SWIG_PY "Compile PaddlePaddle with py PaddlePaddle prediction api" ${SWIG_FOUND}) | ||
option(WITH_SWIG_PY "Compile PaddlePaddle with py PaddlePaddle prediction api" ON) | ||
option(ON_TRAVIS "Running test on travis-ci or not." OFF) | ||
option(ON_COVERALLS "Generating code coverage data on coveralls or not." OFF) | ||
option(COVERALLS_UPLOAD "Uploading the generated coveralls json." ON) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMakeLists 干净很多了
find_package(Git REQUIRED) | ||
find_package(Threads REQUIRED) | ||
|
||
include(simd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
runtime avx搞定之后的话,simd也可以去掉了
"${WARPCTC_INSTALL_DIR}/lib/warpctc.dll" CACHE FILEPATH "Warp-ctc Library" FORCE) | ||
ELSE(WIN32) | ||
SET(WARPCTC_LIBRARIES | ||
"${WARPCTC_INSTALL_DIR}/lib/libwarpctc.dylib" CACHE FILEPATH "Warp-ctc Library" FORCE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dylib 还需要添加linux so的情况
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
@@ -81,18 +81,6 @@ function(link_paddle_exe TARGET_NAME) | |||
set(METRIC_LIBS "") | |||
endif() | |||
|
|||
if(PADDLE_WITH_INTERNAL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@backyes yanfei 说可以去掉internal部分了
@@ -88,6 +88,8 @@ else() | |||
${CUDA_CXX_SOURCES}) | |||
endif() | |||
|
|||
add_dependencies(paddle_cuda ${external_project_dependencies}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
添加external_project_dependencies依赖,保证外部依赖先完成编译安装
CHECK_EQ(1, inputs.size()); | ||
CHECK_EQ(2, outputs.size()); | ||
CHECK_EQ(0, inouts.size()); | ||
CHECK_EQ(1, static_cast<int>(inputs.size())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
引入新的glog, 编译发现paddle还是存在一些类型不一致问题,顺手解决
PREFIX ${PROTOBUF_SOURCES_DIR} | ||
DEPENDS zlib | ||
GIT_REPOSITORY "https://github.com/google/protobuf.git" | ||
# GIT_TAG "v3.1.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
protobuf GIT_TAG目前没有添加,原因是支持最新Mac Serria系统的代码还没有release出来。
UPDATE_COMMAND "" | ||
) | ||
|
||
SET(WARPCTC_INCLUDE_DIR "${WARP_INSTALL_DIR}/include" CACHE PATH "Warp-ctc Directory" FORCE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WARP_INSTALL_DIR 写错了。。WARPCTC_INSTALL_DIR
# See the License for the specific language governing permissions and | ||
# limitations under the License | ||
|
||
cmake_minimum_required(VERSION 3.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cmake最好提升到3.0, 因为外部依赖库普遍最低要求3.0
@@ -96,11 +96,6 @@ TEST(checkGradient, multi) { | |||
TEST(checkGradient, hsigmoid) { checkGradientTest(configFile2, false, false); } | |||
|
|||
TEST(checkGradient, chunk) { | |||
#if defined(__APPLE__) || defined(__OSX__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这部分直接放在cmake里面吧
@@ -17,9 +17,10 @@ add_test(NAME test_Compare | |||
################# test_Trainer ########################### | |||
add_unittest_without_exec(test_Trainer | |||
test_Trainer.cpp) | |||
set(diy_dll_dir ${CMAKE_CURRENT_BINARY_DIR}/../../gserver/tests) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
没什么用,删了
cd googletest-release-1.8.0/ | ||
cmake . | ||
make install | ||
brew install openblas md5sha1sum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
openblas还在考察
CHECK_EQ(3, inputs.size()); | ||
CHECK_EQ(1, outputs.size()); | ||
CHECK_EQ(0, inouts.size()); | ||
CHECK_EQ(3, static_cast<int>(inputs.size())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
看起来新版的glog很严格
${PROTOBUF_LIBRARY} | ||
${LIBGLOG_LIBRARY} | ||
${GFLAGS_LIBRARIES} | ||
${EXTERNAL_LIBS} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
有了EXTERNAL_LIBS,其他都可以去掉了
#if this module is not called, RDMA_INC_DIR RDMA_LIBS will be null, so top module always refer this variable | ||
message(FATAL_ERROR, "RDMA libraries are not found, try to set RDMA_ROOT or check all related libraries.") | ||
endif() | ||
else(WITH_RDMA) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果WITH_RDMA=OFF, 就设置为空
@wangkuiyi @reyoung 开始review吧 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
请先不要merge。
我自己mac下build似乎有些问题。我先看一下。
|
@reyoung 之后会增加一个PR 来find_package(SWIG),毕竟Swig只是用来生成paddle python wrapper, 系统有swig的话,就不需要下载,编译swig了 |
#1083 这个PR会帮助改进目前的paddle swig生成方式。 |
#1001
目前测试Mac, Centos, ubuntu已经全部通过。
由于PR比较复杂,我这里梳理一下,帮助review
已解决:
glog, gflags, protobuf, gtest, python, numpy, wheel, pip, swig, python-protobuf, zlib, setuptools, six, cython 自动下载, 编译,安装 。python部分最复杂。
warpctc自动下载,编译,安装。原版warpctc存在一些bug,比如没有openmp的开关, 没有gpu的开关,make install写的有问题。已经给他们提交PR,并且merge了。https://github.com/baidu-research/warp-ctc/commits/master
后期会继续给他们贡献一个static库版本。
通过观察Travis CI编译和测试时间,发现第三方依赖全部下载,编译,安装所需的时间与之前没有明显区别。
未解决:
目前唯一没有解决的依赖是代数库 openblas. 原因主要是目前paddle有几个函数依赖于lapack,openblas部分代码需要默认使用gfortran编译,在linux系统上面,除了链接libopenblas.a, 最终还需要链接libgfortran.so, 这部分我再考证一下。
本PR带来的好处是:
长远好处:
Paddle的第三方依赖全部来自CMake自动下载,编译,安装,与系统本身相对隔离,对今后发布二进制有好处。
CMake 第三方依赖库的自动下载,编译,安装,为不同操作系统的移植工作,提供了极大的便利。
Travis ci linux最后安装的时候有问题, 可能是pip或者python版本的问题,比较奇怪的是,我fork的版本,一样的代码,travis ci确实没有这个问题的。https://travis-ci.org/gangliao/Paddle @reyoung