Add the automatic update of submodules in cmake. #951

Xreki · 2016-12-19T09:58:16Z

增加了submodules.cmake，其中可以：

cmake时通过-DWARPCTC_ROOT=...设置warpctc头文件ctc.h的路径
在WARPCTC_ROOT/include和${PROJECT_SOURCE_DIR}/warp-ctc/include目录下都没有找到头文件'ctc.h'时，执行命令git submodule update --init -- warp-ctc，自动下载submodule。

目前这个功能只能自动下载warp-ctc，看看这个功能是否需要吧。

qingqing01 · 2016-12-19T15:01:21Z

LGTM

gangliao · 2016-12-20T08:01:38Z

CMakeLists.txt

@@ -68,6 +68,7 @@ find_package(Git REQUIRED)
 # version.cmake will get the current PADDLE_VERSION
 include(version)
 add_definitions(-DPADDLE_VERSION=${PADDLE_VERSION})
+include(submodules)


名字叫submodules，以后的第三方模块意思是都写里面？

这个，其实我不确定，原来有考虑要不要写一个通用的。不过，也许以后也不会再有submodule了。

gangliao · 2016-12-20T08:05:20Z

cmake/submodules.cmake

+  endfunction(FindWarpCTC)
+
+  FindWarpCTC()
+  if(NOT WARPCTC_INCLUDE_DIR)


是不是可以不用FindWarpCTC()?

if(EXISTS ${PROJECT_SOURCE_DIR}/.gitmodules) execute_process( COMMAND ${GIT_EXECUTABLE} submodule update --init -- warp-ctc WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} OUTPUT_VARIABLE GIT_OUTPUT RESULT_VARIABLE GIT_RESULT) message(STATUS ${GIT_OUTPUT}) if(${GIT_RESULT}) ... else() fatal... endif() endif()

另外modules一次性 --recursive 有什么区别？

我其实对于这个GIT_RESULT做了挺多次实验，发现有时候，执行git submodule update --init -- warp-ctc并没有把warp-ctc拉下来，返回结果一直都是0。

https://github.com/uplusplus/pcl_android/blob/3114013adffcd42d736176a5b89532510e06566a/build/CMakeExternals/tmp/boost-fetch/boost-fetch-gitclone.cmake#L45

看起来0才是正确的意思

这里可否用 @gangliao 在 https://github.com/gangliao/CodeCoverageCMake/blob/master/cmake/third_party.cmake 这里使用的 ExternalProject_Add，而不是自己写代码去 git pull warp-ctc 呢？

reyoung

另外，wrapCTC应该放到thrid_party里面，而不是根目录里面。

reyoung · 2016-12-20T09:45:10Z

cmake/submodules.cmake

@@ -0,0 +1,28 @@
+# Automatically init submodules
+if(EXISTS ${PROJECT_SOURCE_DIR}/.gitmodules)


PROJECT_SOURCE_DIR => ${PROJ_ROOT}

reyoung · 2016-12-20T09:45:30Z

cmake/submodules.cmake

+      WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
+      OUTPUT_VARIABLE GIT_OUTPUT
+      RESULT_VARIABLE GIT_RESULT)
+    message(STATUS ${GIT_OUTPUT})


CHECK GIT_RESULT is ok

我其实对于这个GIT_RESULT做了挺多次实验，发现有时候，执行git submodule update --init -- warp-ctc并没有把warp-ctc拉下来，返回结果一直都是0。不知道是不是我哪里弄得不对。。。

Xreki · 2016-12-20T12:02:56Z

@reyoung 我想warp-ctc也是应该放在某个子目录下。但是感觉warp-ctc更像plugin而不是third_party，因为warp-ctc是一个可选的功能，当不用到这个功能，就不需要编译warp-ctc本身，而这不会影响paddle其他功能的使用。

wangkuiyi · 2016-12-20T22:12:48Z

paddle/cuda/include/hl_warpctc_wrap.h

@@ -15,8 +15,8 @@ limitations under the License. */
 #ifndef HL_WARPCTC_WRAP_H_
 #define HL_WARPCTC_WRAP_H_

+#include "ctc.h"


这里是不是还是原来的写法（`#include "warp-ctc/include/ctc.h"）更清晰，能体现出头文件到底在哪里。

增加了一个WARPCTC_ROOT的cmake选项，原意是如果用户机器上已经安装过warp-ctc，则可以直接使用已经安装好的这个版本。看是否需要支持这种情况吧。

wangkuiyi · 2016-12-20T22:15:04Z

cmake/submodules.cmake

+  endfunction(FindWarpCTC)
+
+  FindWarpCTC()
+  if(NOT WARPCTC_INCLUDE_DIR)


这里可否用 @gangliao 在 https://github.com/gangliao/CodeCoverageCMake/blob/master/cmake/third_party.cmake 这里使用的 ExternalProject_Add，而不是自己写代码去 git pull warp-ctc 呢？

Xreki · 2016-12-21T02:27:33Z

@wangkuiyi 我尝试过ExternalProject_Add这个功能，不过写的比较简单

ExternalProject_Add(warp-ctc
    PREFIX ${PROJECT_SOURCE_DIR}/warp-ctc
    GIT_REPOSITORY "https://github.com/baidu-research/warp-ctc.git")

也可以指定某次commit或者tag。
这样设置，会在make的时候自动下载然后编译warp-ctc。考虑到以下两点：

若使用ExternalProject_Add，那似乎也就没有必要添加submodule了。
因为有的用户也许根本不需要使用warp-ctc这个功能，所以我认为warp-ctc应该作为一个可选的组件。在编译paddle之前，强制编译warp-ctc，若warp-ctc编译失败，则会阻塞paddle的编译。

要不要在编译paddle的时候自动编译warp-ctc，这个问题也曾经和 @gangliao @luotao1 讨论过。
关于ExternalProject_Add的使用，我再查一下吧。

reyoung · 2016-12-21T03:00:34Z

我想warp-ctc也是应该放在某个子目录下。但是感觉warp-ctc更像plugin而不是third_party，因为warp-ctc是一个可选的功能，当不用到这个功能，就不需要编译warp-ctc本身，而这不会影响paddle其他功能的使用。

third_party是引第三方库的标准做法和标准目录结构。如果不是特别必要，还是大家咋弄咱咋弄比较好。

另外，单说plugin这个东西，和动态库以及运行时载入动态库的含义并不一致。关于plugin这个东西，一般会认为是具有统计的接口(interface)的一些不同动态库实现，这些动态库实现可以在运行时载入，卸载。为程序增加和删除功能。重点还是得接口一致，所以和这个ctc的实现不太一样。

可能的plugin的文章:

Xreki · 2016-12-23T09:57:11Z

@gangliao 将使用cmake的ExternalProject_Add功能统一实现。

* documents will be ready afterwards not ready now * Update index_en.rst * Update index_cn.rst * Update index_en.rst * Update index_cn.rst * Update index_en.rst

Tuple的init参数修改

Xreki added 3 commits December 19, 2016 09:30

Add the automatic update of submodules in cmake.

bc882ac

Merge branch 'develop' into cmake_for_warpctc

b32fa7b

Format files using clang-format 3.8.

5c6be6c

qingqing01 assigned gangliao Dec 19, 2016

gangliao reviewed Dec 20, 2016

View reviewed changes

reyoung reviewed Dec 20, 2016

View reviewed changes

wangkuiyi requested changes Dec 20, 2016

View reviewed changes

Xreki closed this Dec 23, 2016

Xreki deleted the cmake_for_warpctc branch October 29, 2019 00:41

wangxicoding pushed a commit to wangxicoding/Paddle that referenced this pull request Dec 9, 2021

Update data_preprocess.rst (PaddlePaddle#951)

02bb22e

Tuple的init参数修改

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the automatic update of submodules in cmake. #951

Add the automatic update of submodules in cmake. #951

Xreki commented Dec 19, 2016 •

edited

Loading

qingqing01 commented Dec 19, 2016

gangliao Dec 20, 2016

Xreki Dec 20, 2016

gangliao Dec 20, 2016

Xreki Dec 20, 2016

gangliao Dec 20, 2016 •

edited

Loading

wangkuiyi Dec 20, 2016

reyoung left a comment

reyoung Dec 20, 2016

Xreki Dec 20, 2016

reyoung Dec 20, 2016

Xreki Dec 20, 2016

Xreki commented Dec 20, 2016

wangkuiyi Dec 20, 2016

Xreki Dec 21, 2016

wangkuiyi Dec 20, 2016

Xreki commented Dec 21, 2016

reyoung commented Dec 21, 2016

Xreki commented Dec 23, 2016 •

edited

Loading

		@@ -0,0 +1,28 @@
		# Automatically init submodules
		if(EXISTS ${PROJECT_SOURCE_DIR}/.gitmodules)

Add the automatic update of submodules in cmake. #951

Add the automatic update of submodules in cmake. #951

Conversation

Xreki commented Dec 19, 2016 • edited Loading

qingqing01 commented Dec 19, 2016

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

gangliao Dec 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reyoung left a 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

Xreki commented Dec 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Xreki commented Dec 21, 2016

reyoung commented Dec 21, 2016

Xreki commented Dec 23, 2016 • edited Loading

Xreki commented Dec 19, 2016 •

edited

Loading

gangliao Dec 20, 2016 •

edited

Loading

Xreki commented Dec 23, 2016 •

edited

Loading