Skip to content

Conversation

@Superjomn
Copy link
Contributor

@Superjomn Superjomn commented Jun 3, 2018

Code of Anakin's paddle interface is a little mess, I changed some and will give a PR to its repo latter.

  • TODO add an offline valid test.
  • TODO wget the Anakin library from somewhere in CI phrase or just run it manually offline?
@Superjomn Superjomn requested review from luotao1 and panyx0718 June 4, 2018 00:47
endif(APPLE)

set(ANAKIN_INCLUDE "" CACHE STRING "root of Anakin header files")
set(ANAKIN_LIBRARY "" CACHE STRING "path of Anakin library")
Copy link
Contributor

Choose a reason for hiding this comment

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

可以仿照TENSORRT和CUDNN一样,合并成一个ANAKIN_ROOT么,然后自动找include和lib。

set(TENSORRT_ROOT "/usr" CACHE PATH "TENSORRT ROOT")
find_path(TENSORRT_INCLUDE_DIR NvInfer.h
PATHS ${TENSORRT_ROOT} ${TENSORRT_ROOT}/include
$ENV{TENSORRT_ROOT} $ENV{TENSORRT_ROOT}/include
NO_DEFAULT_PATH
)
find_library(TENSORRT_LIBRARY NAMES libnvinfer.so libnvinfer.a
PATHS ${TENSORRT_ROOT} ${TENSORRT_ROOT}/lib
$ENV{TENSORRT_ROOT} $ENV{TENSORRT_ROOT}/lib
NO_DEFAULT_PATH
DOC "Path to TensorRT library.")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

暂时不行。。 阿拉金的库比较乱

  • 接口没有封装,要依赖整个 repo 的头文件
  • library 需要手动拷,和完整 repo 的头文件没有放在一起
  • 他们的 paddle_api.h 是个 test,没有包含在发布的头文件里

所以,现在必须显式下载 library,和完整的 github repo,分别设定路径,才能编译

后面会让他们改,这个可能是临时方案。


if(APPLE)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=pessimizing-move")
endif(APPLE)
Copy link
Contributor

Choose a reason for hiding this comment

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

可以单独一个anakin.cmake,paddle中所有第三方库加入的时候,都单独做了一个cmake,这样会更加清晰。
20-42行可以放进anakin.cmake中。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

后面可以看看,暂时他们的代码也有挺多问题的,现在给的接口也非常初步。。

Copy link
Contributor

Choose a reason for hiding this comment

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

单独的anakin.cmake里面也可以显示设置include和lib

Copy link
Contributor Author

Choose a reason for hiding this comment

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

可以等稍微成熟些再改地正规些,我感觉需要再跟他们沟通下。

目前的状态太初步了,他们的头文件我自己也改了很多地方才能用,后面应该他们要有大的修正才正式用起来。

set(inference_deps paddle_inference_api paddle_fluid_api)

# if anakin is set enable anakin api implementation
if (NOT ANAKIN_INCLUDE STREQUAL "")
Copy link
Contributor

Choose a reason for hiding this comment

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

if(ANAKIN_INCLUDE_DIR AND ANAKIN_LIBRARY) set(ANAKIN_FOUND ON) else() set(ANAKIN_FOUND OFF) endif() 

需要一个ANAKIN_FOUND的开关

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个可以有

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=switch")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=return-type")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=non-virtual-dtor")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=cpp")
Copy link
Contributor

Choose a reason for hiding this comment

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

28-35行可以合并成:

if(ANAKIN_FOUND) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=comment -Wno-error=reorder -Wno-error=format ...") 
Copy link
Contributor Author

Choose a reason for hiding this comment

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

我试试

inference_api_test(test_paddle_inference_api_impl
ARGS test_word2vec test_image_classification)

if (NOT ANAKIN_INCLUDE STREQUAL "")
Copy link
Contributor

Choose a reason for hiding this comment

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

if(ANAKIN_FOUND)

const std::vector<PaddleTensor> &inputs,
std::vector<PaddleTensor> *output_data) {
for (const auto &input : inputs) {
CHECK(input.dtype == PaddleDType::FLOAT32);
Copy link
Contributor

Choose a reason for hiding this comment

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

should return false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get


PaddleInferenceAnakinPredictor::PaddleInferenceAnakinPredictor(
const AnakinConfig &config) {
CHECK(Init(config));
Copy link
Contributor

Choose a reason for hiding this comment

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

Do this in CreateXXX and return false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, Anakin engine's creation is just a void function.

// TODO(Superjomn) Tell anakin to support return code.
engine_.Execute();

CHECK(!output_data->empty())
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe not need to worry about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found some issue about the usage of the output_data. The current implementation for the native engine is that the output_data's memory is maintained by the engine, but the original intention is to force the users to maintain the memory outside and the engine just copies the data.

So that, it can support

  • fetch any target, just set the PaddleTensor's names of output_data -- given a big model, fetch any variable
  • engines like Anakin's output variables are located at GPU, need a memory to copied, and users can do some memory optimization such as pre-allocate a big buffer.
auto *tensor = engine_.GetOutputInGPU(output.name);
output.shape = tensor->shape();
// Copy data from GPU -> CPU
CHECK_EQ(cudaMemcpy(output.data.data,
Copy link
Contributor

Choose a reason for hiding this comment

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

return false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get

# if anakin is set enable anakin api implementation
if (NOT ANAKIN_INCLUDE STREQUAL "")
# Anakin's code style doesn't follow google c style.
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=comment")
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this affect other codes outside anakin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, if Anakin is turn on. @luotao1 any solution?

Copy link
Contributor

Choose a reason for hiding this comment

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

请问是我们使用anakin头文件的时候,会出现各种错误么?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这几个 warning 都会编译失败,代码问题比较多,现在是会嵌套 include 他们所有的头文件,会有这类 warning.

Copy link
Contributor

@luotao1 luotao1 Jun 4, 2018

Choose a reason for hiding this comment

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

有一个办法,是可以不改cmake_cxx_flags。

  • 参考 https://eigen.tuxfamily.org/dox/DisableStupidWarnings_8h_source.html
    新增一个DisableStupidWarnings.h的头文件,将所有的warning以 #pragma GCC diagnostic ignored "-Wignored-attributes" 形式给去掉,加载anakin的头文件时,同步加载这个头文件。或者直接写进anakin的头文件中。
  • 我在使用ICC编译器时,会出现Intel编译器一堆warning的问题,亲测有效。
Copy link
Contributor

@luotao1 luotao1 left a comment

Choose a reason for hiding this comment

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

ci里面没有检查使用anakin的代码正确性,是不是需要在dockerfile里面先把anakin给装上?


if(APPLE)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-error=pessimizing-move")
endif(APPLE)
Copy link
Contributor

Choose a reason for hiding this comment

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

单独的anakin.cmake里面也可以显示设置include和lib

@Superjomn Superjomn merged commit 418c41d into PaddlePaddle:develop Jun 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants