-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
refine operators/math/CMakeLists.txt #8682
Conversation
luotao1
commented
Mar 1, 2018
•
edited
Loading
edited
- fix refine operators/math/CMakeLists.txt #8611
- separate im2col from math_function
# But it handle split GPU/CPU code and link some common library. | ||
set(cc_srcs) | ||
set(cu_srcs) | ||
set(math_common_deps device_context framework_proto) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好像只有math_function
依赖framework_proto
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 原来都是依赖
math_function
,从而依赖framework_proto
。 - 如果现在去掉依赖
framework_proto
,会出现https://paddleci.ngrok.io/viewLog.html?buildId=29829&buildTypeId=Paddle_PrCi&tab=buildLog&_focus=1362
[18:22:58] In file included from /paddle/paddle/fluid/framework/tensor_util.h:16:0,
[18:22:58] from /paddle/paddle/fluid/operators/math/im2col.h:18,
[18:22:58] from /paddle/paddle/fluid/operators/math/im2col.cc:15:
[18:22:58] /paddle/paddle/fluid/framework/data_type.h:17:49: fatal error: paddle/fluid/framework/framework.pb.h: No such file or directory
[18:22:58] compilation terminated.
[18:22:58] paddle/fluid/operators/math/CMakeFiles/im2col.dir/build.make:62: recipe for target 'paddle/fluid/operators/math/CMakeFiles/im2col.dir/im2col.cc.o' failed
[18:22:58] CMakeFiles/Makefile2:18030: recipe for target 'paddle/fluid/operators/math/CMakeFiles/im2col.dir/all' failed
[18:22:58] make[2]: *** [paddle/fluid/operators/math/CMakeFiles/im2col.dir/im2col.cc.o] Error 1
[18:22:58] make[1]: *** [paddle/fluid/operators/math/CMakeFiles/im2col.dir/all] Error 2
[18:22:58] make[1]: *** Waiting for unfinished jobs....
[18:22:58] [ 36%] Building CXX object CMakeFiles/libprotobuf.dir/paddle/build/third_party/protobuf/src/extern_protobuf/src/google/protobuf/struct.pb.cc.o
[18:22:58] [ 36%] Building CXX object CMakeFiles/libprotobuf.dir/paddle/build/third_party/protobuf/src/extern_protobuf/src/google/protobuf/source_context.pb.cc.o
[18:22:58] In file included from /paddle/paddle/fluid/framework/tensor_util.h:16:0,
[18:22:58] from /paddle/paddle/fluid/operators/math/vol2col.h:18,
[18:22:58] from /paddle/paddle/fluid/operators/math/vol2col.cc:15:
[18:22:58] /paddle/paddle/fluid/framework/data_type.h:17:49: fatal error: paddle/fluid/framework/framework.pb.h: No such file or directory
[18:22:58] compilation terminated.
[18:22:58] make[2]: *** [paddle/fluid/operators/math/CMakeFiles/vol2col.dir/vol2col.cc.o] Error 1
[18:22:58] make[1]: *** [paddle/fluid/operators/math/CMakeFiles/vol2col.dir/all] Error 2
cc_test(sequence_padding_test SRCS sequence_padding_test.cc DEPS sequence_padding) | ||
if(WITH_GPU) | ||
nv_test(math_function_gpu_test SRCS math_function_test.cu) | ||
nv_test(selected_rows_functor_gpu_test SRCS selected_rows_functor_test.cu DEPS selected_rows_functor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
话说我一直觉得这两个单测其实可以合并到math_function_test.cc
和selected_rows_functor_test.cc
里面,因为单测里面并没有CUDA kernel代码,并且测试的内容应该和CPU单测保持一致。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已经新建一个issue:#8828
set(cc_srcs) | ||
set(cu_srcs) | ||
set(math_common_deps device_context framework_proto) | ||
set(multiValueArgs SRCS DEPS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里去掉SRCS
?或者有SRCS
时改怎么处理呢?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可以完全自动匹配,所以去掉SRCS
if (WITH_GPU) | ||
nv_library(${TARGET} SRCS ${cc_srcs} ${cu_srcs} DEPS ${math_library_DEPS} ${math_common_deps}) | ||
else() | ||
cc_library(${TARGET} SRCS ${cc_srcs} DEPS ${math_library_DEPS} ${math_common_deps}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里加个${cc_srcs}是否为空
的判断,depthwise_conv
应该就不需要特殊处理。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DONE
math_library(unpooling) | ||
math_library(cos_sim_functor) | ||
math_library(lstm_compute DEPS activation_functions) | ||
math_library(gru_compute DEPS activation_functions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
原来sequence_pooling
, context_project
, sequence2batch
, gru_compute
对math_function
是有依赖的,其中:
sequnce_pooling
和gru_compute
确实调用了math_function
中的函数,恰巧这几个Functor都没有单测,不然不加math_function
的依赖应该会出错。context_project
和sequence2batch
包含了math_function.h
,但似乎没有函数调用,include
语句应该可以移除。context_project
依赖了im2col
,应该也是因为没有单测,所以才没报错。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DONE。
除了context_project
需要依赖math_function
,因为调用了axpy
。
axpy<DeviceContext, T>(context, w_sub.numel(), static_cast<T>(1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM