Skip to content
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

add shared warpctc lib in whl #3503

Merged
merged 3 commits into from
Aug 16, 2017
Merged

add shared warpctc lib in whl #3503

merged 3 commits into from
Aug 16, 2017

Conversation

luotao1
Copy link
Contributor

@luotao1 luotao1 commented Aug 15, 2017

Inspired by #3461 , add shared warpctc lib in whl.

  • Thus, after pip install *.whl, we can run warpctc layer exactly.
  • Before this PR, we should export LD_LIBRARY_PATH=$PADDLE_INSTALL_DIR/Paddle/third_party/install/warpctc/lib:$LD_LIBRARY_PATH to run warpctc layer.

@luotao1 luotao1 requested review from Xreki and tensor-tang August 15, 2017 10:44
@tensor-tang
Copy link
Contributor

I just found that:

paddle/platform/dynload/dynamic_loader.cc:34:DEFINE_string(warpctc_dir, "", "Specify path for loading libwarpctc.so.");

and

paddle/utils/DynamicLoader.cpp:31:DEFINE_string(warpctc_dir, "", "Specify path for loading libwarpctc.so.");

Maybe we can remove it now?

And should we add some unit test for loading warpctc just like import *?

@luotao1
Copy link
Contributor Author

luotao1 commented Aug 16, 2017

@tensor-tang Discuss with @Xreki , we remain these related codes currently, as we may build libwarpctc.a later.
@gangliao Is your /~https://github.com/gangliao/warp-ctc has a plan to build libwarpctc.a later?

Copy link
Contributor

@tensor-tang tensor-tang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Xreki
Copy link
Contributor

Xreki commented Aug 17, 2017

仔细分析了下,目前warp-ctc最好还是保持原状吧,即,继续使用dlopen的实现(不要改成直接链接动态库,也不要改成使用静态库),理由如下:

  • 使用c-api一般情况都是用于inference,而inference时是不需要warpctccost layer的。目前的CMake编译系统,在编译时不能很好地拆分成traininference两种情况。保持现状的好处是,用户在使用Paddlec-api动态库或者静态库上线、或者使用到Android app,都可以忽略warpctc这个库的存在。

另外,在对Paddle执行make install时,是否也需要将mkldnn等相关动态库installCMAKE_PREFIX_INSTALL目录下呢,因为如果编译c-api库时使用到了这些库,在使用c-api预测时也必须显式链接这些库。

@tensor-tang
Copy link
Contributor

tensor-tang commented Aug 17, 2017

另外,在对Paddle执行make install时,是否也需要将mkldnn等相关动态库install到CMAKE_PREFIX_INSTALL目录下呢

这一点,以前实际就是安装到CMAKE_PREFIX_INSTALL的,就是当时的#3162, 因为考虑到没有root权限的,会安装到home/user下。目录上不干净。
现在会统一放到build/third party/install/mkldnn里面,这样目录上会整齐很多。

因为如果编译c-api库时使用到了这些库,在使用c-api预测时也必须显式链接这些库。

如果用户在使用c-api的时候,不删除build/third party/install/mkldnn肯定是没有问题的,这一点pass。
如果想要make install 到/usr/local/opt/paddle/lib,也是需要用户将加入LD_LIBRARY_PATH之后才可以使用。
或者就是得让安装third party的时候,就已经安装到build/third party/install/mkldnn,然后把这个目录加入rpath能搞定,可是这个方法就是之前的方法,会遇到无法安装的权限问题,被迫安装到了home。

不管怎么样,安装到哪个目录从我这里是code上都是能实现的,主要是符合paddle的设计需求就行。

都可以忽略warpctc这个库的存在。

另外关于WarpCTC,我觉得是不是可以也给一个WITH_WARPCTC
不然现在的窘境就是,其实每次third party是会一直需要下载并编译WarpCTC, 但是实际上,使用时是不一定需要的。
由于没有这个WITH_WARPCTC,所以又会一直把他的lib放入whl,这样才比较尴尬了。
所以如果有个开关,就可以像现在mklml和mkldnn这样,只在需要时才放入whl。

@Xreki
Copy link
Contributor

Xreki commented Aug 18, 2017

这一点,以前实际就是安装到CMAKE_PREFIX_INSTALL的,就是当时的#3162, 因为考虑到没有root权限的,会安装到home/user下。目录上不干净。

make install会将Paddle的CMake中指定的需要安装的文件,安装到用户指定的CMAKE_PREFIX_INSTALL目录下,既然用户指定了CMAKE_PREFIX_INSTALLCMAKE_PREFIX_INSTALL目录不一定是home/user,它有可能是/home/user/install/paddle之类的,是用户授权的。另外,强调下,我这里说到的是make install,而不是pip install

现在会统一放到build/third party/install/mkldnn里面,这样目录上会整齐很多。

你可能误解我的意思了。mkldnn肯定是首先要安装到build/third party/install/mkldnn目录下的,这个是发生在对Paddle执行make的时候。而我说的,其实是希望把third_party下安装的所以lib都拷贝到CMAKE_INSTALL_PREFIX目录下,这个是发生在Paddle的make install阶段。现在很多人编译安装Paddle已经不使用make install操作了,#3524 中还讨论是否要移除make install操作。我认为这个操作是需要保留的,有了这个操作,至少用户能明确地知道他所编译的Paddle库.a.so安装到了哪里。

如果用户在使用c-api的时候,不删除build/third party/install/mkldnn肯定是没有问题的,这一点pass。

用户希望将c-apilibpaddle_capi_shared.so或者libpaddle_capi_whole.a用到他们的预测程序里面去,那么也可能需要显式链接third_party中依赖的各库。所以,有考虑将third_party中的库也一起installCMAKE_PREFIX_INSTALL目录下,这样用户能比较明确地知道需要链接哪些库。当然,这个不只是相关mkldnn,我只是突然想到,在这里提起了。

关于WarpCTC,我觉得是不是可以也给一个WITH_WARPCTC

关于warp-ctc,设计成dlopen方式实现的初衷是,不希望发布版本的时候,需要发布一个-with-warpctc-without-warpctc两个版本。最初是以sub-module方式引进来的,用户需要用到warp-ctc功能,则需自行编译warp-ctc库。什么样的方式好,这我也不好说。

@tensor-tang
Copy link
Contributor

tensor-tang commented Aug 18, 2017

CMAKE_PREFIX_INSTALL目录不一定是home/user,它有可能是/home/user/install/paddle之类的,是用户授权的。

解释下哈,这里可能是我之前没说清楚,最开始是在安装third party 的 mkldnn的时候是直接安装到/usr/local/lib下的,所以会存在有可能权限不够的情况。所以后来改了下,安装到了CMAKE_PREFIX_INSTALL,就是你在#3162中的问题介绍中看到的/home/user/opt/paddle/thirdparty (这一点,与先安装到build/thirdparty/install,再copy到CMAKE_PREFIX_INSTALL/paddle/thirdparty是一个效果)。但是其实这样也并不好,所以统一改到目录build/third_party/install下,才是最终看到的现在这样(这一点我很同意啊)。

另外,强调下,我这里说到的是make install,而不是pip install。

就像你说的,我知道你说的是paddle 的make install,而我之前想说的是,假如我们现在把第三方lib安装到了CMAKE_PREFIX_INSTALL之后的情况。比如最终目录是/home/user/opt/paddle/thirdparty ,那么为了让我们的这些动态库(现在假设的是有动态库的情况)可以正确的链接对:

  1. 就需要用户把这个目录加入LD_LIBRARY_PATH。这个是需要用户干预。
  2. 或者在编译paddle的bin文件时,就需要把这个目录加入rpath。我担心的是到时候会不会有点不好做(当然也不一定,看具体情况而定)

同时,就像你在#3524 中说的:"Furthermore, I think maybe all the depended third_party libraries should be copied to CMAKE_PREFIX_INSTALL, because they may need to link these libraries when linking c-api library of Paddle. It is another topic."

  1. make install 我也建议是需要保留的。
  2. 把所有third party的lib在make install的时候 copy到用户指定的地方,我也认为是另一个故事,我之前只是在考虑copy完了之后,会不会有链接问题(当然这个担心有没有必要就另说哈,因为毕竟编译时用的目录和执行时用的目录不一样了)。

以上仅仅是一些个人观点和concern哈。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants