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

compile and install the static library of fluid inference #7827

Merged
merged 18 commits into from
Mar 7, 2018
Merged

compile and install the static library of fluid inference #7827

merged 18 commits into from
Mar 7, 2018

Conversation

luotao1
Copy link
Contributor

@luotao1 luotao1 commented Jan 24, 2018

related #7231

  • The structure of the installed shared library is as follows. It uses default cmake option + -DWITH_PYTHON=OFF
PaddleRoot/
├── bin
│   └── paddle
├── include
│   └── paddle
│       ├── framework
│       ├── inference
│       ├── memory
│       ├── platform
│       └── string
├── lib
│   ├── libpaddle_fluid.a
│   └── libpaddle_fluid.so
├── opt
│   └── paddle
│       └── bin
└── third_party
    ├── eigen3
    │   ├── Eigen
    │   └── unsupported
    ├── gflags
    │   ├── include
    │   └── lib
    ├── glog
    │   ├── include
    │   └── lib
    ├── openblas
    │   ├── include
    │   └── lib
    └── protobuf
        ├── include
        └── lib
du -sh *
12K     bin
700K    include
188M    lib
200M    opt
92M     third_party
128M    libpaddle_fluid.a
61M     libpaddle_fluid.so

@Xreki Xreki added the 预测 原名Inference,包含Capi预测问题等 label Jan 24, 2018
Copy link
Collaborator

@wangkuiyi wangkuiyi left a comment

Choose a reason for hiding this comment

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

Please refer to my comment #7231 (comment) 8 days ago.

It seems that this PR and the issue aforementioned are related to the binary release of PaddlePaddle.

However, to what I understand is that our binary release would be in the form of pip packages, but not as binary libraries (/bin, /include, /lib directories).

Also, even if we are going to add binary libraries as an additional form of binary release, we are not supposed to put third-party binaries in our directory hierarchy. The right way is to build only ours and pack a .deb file, which describes on what other .deb files it depends.

My suggestion is that we discard this PR and possibly revert another one (I vaguely remember, had been merged), and rethink about (1) if we are going to do this, and (2) what's the right way to doing it.

@luotao1
Copy link
Contributor Author

luotao1 commented Jan 25, 2018

Please refer to my comment #7231 (comment) 8 days ago

Discussed with @Xreki, we have already answered in #7572 (comment), but sorry to forget @ you.

It seems that this PR and the issue aforementioned are related to the binary release of PaddlePaddle.

The binary directory /bin and /opt is left for C-API since WITH_FLUID option (#6346) has not been finished now. After finishing #6346, the final fluid inference library directory likes as follows, and it is not a binary library:

PaddleRoot/
├── include
│   └── paddle
│       ├── framework
│       ├── inference
│       ├── memory
│       ├── platform
│       └── string
├── lib
│   ├── libpaddle_fluid.a
│   └── libpaddle_fluid.so
└── third_party
    ├── eigen3
    │   ├── Eigen
    │   └── unsupported
    ├── gflags
    │   ├── include
    │   └── lib
    ├── glog
    │   ├── include
    │   └── lib
    ├── openblass
    │   ├── include
    │   └── lib
    └── protobuf
        ├── include
        └── lib

However, to what I understand is that our binary release would be in the form of pip packages

As both IOS and Android don't support pip packages since they don't support python, the fluid inference library should not be in the form of this way.

Also, even if we are going to add binary libraries as an additional form of binary release, we are not supposed to put third-party binaries in our directory hierarchy.

Our third-party libraries only have include/ and lib/, and don't contain any binaries.

There are two reasons why we put third-party libraries in our directory:

  • For some platforms (such as IOS) and usages, users must link static library. Thus, they must link the corresponding third-party libraries. If we don't provide them, users must download and compile them manually, which is not convenient and sometimes hard for users. For example, like openblas.cmake, the compile option is different from IOS, Android, RPI, Apple and Linux.
    IF(CMAKE_CROSSCOMPILING)
    SET(OPTIONAL_ARGS HOSTCC=${HOST_C_COMPILER})
    GET_FILENAME_COMPONENT(CROSS_SUFFIX ${CMAKE_C_COMPILER} DIRECTORY)
    SET(CROSS_SUFFIX ${CROSS_SUFFIX}/)
    IF(ANDROID)
    IF(ANDROID_ABI MATCHES "^armeabi(-v7a)?$")
    # use softfp
    SET(OPTIONAL_ARGS ${OPTIONAL_ARGS} TARGET=ARMV7 ARM_SOFTFP_ABI=1 USE_THREAD=0)
    ELSEIF(ANDROID_ABI STREQUAL "arm64-v8a")
    SET(OPTIONAL_ARGS ${OPTIONAL_ARGS} TARGET=ARMV8 BINARY=64 USE_THREAD=0)
    ENDIF()
    ELSEIF(IOS)
    IF(CMAKE_OSX_ARCHITECTURES MATCHES "arm64")
    SET(OPENBLAS_CC "${OPENBLAS_CC} ${CMAKE_C_FLAGS} -isysroot ${CMAKE_OSX_SYSROOT}")
    SET(OPENBLAS_CC "${OPENBLAS_CC} -arch arm64")
    SET(OPTIONAL_ARGS ${OPTIONAL_ARGS} TARGET=ARMV8 BINARY=64 USE_THREAD=0 CROSS_SUFFIX=${CROSS_SUFFIX})
    ELSE()
    MESSAGE(FATAL_ERROR "OpenBLAS only support arm64 architectures on iOS. "
    "You can set IOS_USE_VECLIB_FOR_BLAS=ON or USE_EIGEN_FOR_BLAS=ON to use other blas library instead.")
    ENDIF()
    ELSEIF(RPI)
    # use hardfp
    SET(OPTIONAL_ARGS ${OPTIONAL_ARGS} TARGET=ARMV7 USE_THREAD=0)
    ENDIF()
    ELSE()
    IF(APPLE)
    SET(OPENBLAS_CC "${CMAKE_C_COMPILER} -isysroot ${CMAKE_OSX_SYSROOT}")
    ENDIF()
    SET(OPTIONAL_ARGS "")
    IF(CMAKE_SYSTEM_PROCESSOR MATCHES "^x86(_64)?$")
    SET(OPTIONAL_ARGS DYNAMIC_ARCH=1 NUM_THREADS=64)
    ENDIF()
    ENDIF()
  • It is easy for users to know how many third-party libraries depended by paddle, and it is easy for users to link these libraries themselves.

The right way is to build only ours and pack a .deb file, which describes on what other .deb files it depends.

.deb can only be installed on Ubuntu, but it can't be installed on Mac, IOS and Andriod. Thus, .deb may not be a good choice.

@wangkuiyi
Copy link
Collaborator

I agree that we publish our inference library, but with two points different from the current status of this PR:

  1. make inference_lib_dist instead of make install
  2. the directory hierarchy in the generated tgz file should be the same as that in ./build.

@luotao1
Copy link
Contributor Author

luotao1 commented Jan 29, 2018

@wangkuiyi

the directory hierarchy in the generated tgz file should be the same as that in ./build

If the same as that in ./build, the final directory hierarchy would like as follows:

PaddleRoot
├── paddle
│   ├── framework
│   │   ├── attribute.h
│   │   ├── backward.h ...
│   │   └── var_type_inference.h
│   ├── inference
│   │   ├── inference.h
│   │   └── 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

@luotao1 luotao1 closed this in #7977 Feb 7, 2018
@luotao1 luotao1 reopened this Feb 8, 2018
@@ -82,6 +82,7 @@ IF(NOT ${CBLAS_FOUND})
CONFIGURE_COMMAND ""
)
SET(CBLAS_PROVIDER openblas)
FILE(REMOVE_RECURSE ${CBLAS_INSTALL_DIR}/lib/cmake ${CBLAS_INSTALL_DIR}/lib/pkgconfig)
Copy link
Contributor

Choose a reason for hiding this comment

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

FILE命令应该是在执行cmake命令的时候执行的。extern_openblas是在make阶段执行的,也就是说,必须要执行完extern_openblas这个任务之后,才会生成${CBLAS_INSTALL_DIR}/lib/cmake${CBLAS_INSTALL_DIR}/lib/pkgconfig这两个目录。所以这条语句应该达不到删除的目的。应该使用execute_process语句删除,并且该语句需要依赖extern_openblas这个任务。

# Merge all modules into a single static library
cc_library(paddle_fluid DEPS paddle_fluid_api ${FLUID_CORE_MODULES} ${GLOB_OP_LIB})
cc_library(paddle_fluid DEPS paddle_fluid_api
Copy link
Contributor

Choose a reason for hiding this comment

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

memory模块一样,我们在每个模块最后加一个cc_library,将该模块里面所有的target都合并成一个静态库?

cc_library(paddle_memory
DEPS
memory
memcpy
meta_data
meta_cache
memory_block
buddy_allocator
system_allocator)

这样,在inference里面,我们只需要依赖paddle_memroy, paddle_framework, paddle_operators, paddle_string, paddle_platform。不过这样还是不能自动添加。

@luotao1 luotao1 closed this Feb 10, 2018
@luotao1 luotao1 reopened this Feb 10, 2018
@CLAassistant
Copy link

CLAassistant commented Feb 10, 2018

CLA assistant check
All committers have signed the CLA.

DSTS ${dst_dir} ${dst_dir}/lib
)
ENDIF(NOT PROTOBUF_FOUND)

IF(NOT WITH_MKL)
Copy link
Contributor

Choose a reason for hiding this comment

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

这里应该是if(NOT CBLAS_FOUND)cblas.cmake里面会去找系统里的一些blas库,只有CBLAS_FOUNDOFF时才会自动去下载、编译和安装openblas。原则上,不是Paddle自动下载、编译的库,我们不要拷贝吧。mklml在加入的时候,也在cblas.cmake里面设置了CBLAS_FOUND,这种情况另外再考虑吧。

另外,大小写统一一下?:smile:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

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 and have tested. It is glad to see all fluid C++ modules can be collected automatically and merged into a single static library. Great work!

@wangkuiyi wangkuiyi merged commit 6f50dee into PaddlePaddle:develop Mar 7, 2018
@luotao1 luotao1 deleted the fluid_infer branch May 22, 2018 05:46
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.

4 participants