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

make inference_lib_dist for fluid inference shared library #7977

Merged
merged 7 commits into from
Feb 7, 2018
Merged

make inference_lib_dist for fluid inference shared library #7977

merged 7 commits into from
Feb 7, 2018

Conversation

luotao1
Copy link
Contributor

@luotao1 luotao1 commented Jan 30, 2018

PaddleRoot
├── paddle
│   ├── framework
│   │   ├── attribute.h
│   │   ├── backward.h ...
│   │   └── var_type_inference.h
│   ├── inference
│   │   ├── inference.h
│   │   ├── libpaddle_fluid.a
│   │   └── libpaddle_fluid.so
│   ├── memory
│   │   ├── detail
│   │   ├── memcpy.h
│   │   └── memory.h
│   ├── platform
│   │   ├── assert.h
│   │   ├── call_once.h ...
│   │   └── variant.h
│   └── string
│       ├── piece.h
│       ├── printf.h
│       ├── tinyformat
│       └── to_string.h
└── third_party
    ├── eigen3
    │   ├── Eigen
    │   │   ├── Core
    │   │   └── src
    │   └── unsupported
    │       └── Eigen
    └── install
        ├── gflags
        │   ├── include
        │   └── lib
        ├── glog
        │   ├── include
        │   └── lib
        └── protobuf
            ├── include
            └── lib
  • the command for this fluid inference library is :
make -j 12
make inference_lib_dist 

@Xreki Xreki added the 预测 原名Inference,包含Capi预测问题等 label Jan 30, 2018
@luotao1 luotao1 requested a review from Xreki January 30, 2018 13:23
set(lib_dir "${CMAKE_INSTALL_PREFIX}/third_party/eigen3")
add_custom_target(eigen3_lib
COMMAND mkdir -p "${lib_dir}/Eigen" "${lib_dir}/unsupported"
COMMAND cp "${EIGEN_INCLUDE_DIR}/Eigen/Core" "${lib_dir}/Eigen"
Copy link
Contributor

Choose a reason for hiding this comment

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

It is better to use ${CMAKE_COMMAND} -E copy_directory, like here

@Xreki
Copy link
Contributor

Xreki commented Feb 2, 2018

@luotao1 线下讨论了一下,我们觉得当前这种修改方式不太美观,且破坏了CMakeList.txt的整体风格,故总结出以下修改方案:

  1. 【待确定】make inference_dist_lib命令名太长,且太具针对性。实际上,我们加入这些命令的目的,是部署C++的头文件和库,所以,@wangkuiyi 改成make library是否更合适?
  2. Paddle/cmake目录下新增一个library.cmake的文件,用于部署,从而不需要修改各个目录下的CMakeLists.txt
  3. 实现一个cmake function,封装add_custom_target功能,接口为copy(xxx FROM xxx xxx TO xxx),功能大体要求如下:
    • 如果目标目录不存在,则需要创建
    • 可以拷贝单个文件、多个文件、整个目录
    • 考虑是否需要支持通配符,这个根据具体需求而定

@luotao1
Copy link
Contributor Author

luotao1 commented Feb 6, 2018

Discussed with @wangkuiyi

  1. We will still use make inference_dist_lib command.
  2. We will use inference_lib.cmake to replace library.cmake.

Copy link
Contributor

@Xreki Xreki left a comment

Choose a reason for hiding this comment

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

LGTM
可以在copy函数里面加一些print信息,打印从哪里拷贝到哪里,类似install命令的输出一样。

@luotao1 luotao1 merged commit 6c3b78b into PaddlePaddle:develop Feb 7, 2018
@luotao1
Copy link
Contributor Author

luotao1 commented Feb 7, 2018

可以的,再下一个PR中为copy函数加上打印信息。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
预测 原名Inference,包含Capi预测问题等
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants