From d027f47d7d93b1bdbf7b91090f362fdd879c7120 Mon Sep 17 00:00:00 2001 From: Yu Yang Date: Tue, 11 Jul 2017 20:22:18 +0800 Subject: [PATCH 01/11] Default scope function `Paddle` manages Scope as programming language's scope. It just a thread-local stack of Scope. Top of that stack is current scope, the bottom of that stack is all scopes' parent. Invoking `create_var/get_var` can `create/get` variable in current scope. Invoking `enter_local_scope/leave_local_scope` can create or destroy local scope. A `scoped_function` will take a `function` as input. That function will be invoked in a new local scope. --- .../v2/framework/default_scope_funcs.py | 83 +++++++++++++++++++ .../paddle/v2/framework/tests/CMakeLists.txt | 3 +- .../tests/test_default_scope_funcs.py | 33 ++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 python/paddle/v2/framework/default_scope_funcs.py create mode 100644 python/paddle/v2/framework/tests/test_default_scope_funcs.py diff --git a/python/paddle/v2/framework/default_scope_funcs.py b/python/paddle/v2/framework/default_scope_funcs.py new file mode 100644 index 0000000000..4e772326c9 --- /dev/null +++ b/python/paddle/v2/framework/default_scope_funcs.py @@ -0,0 +1,83 @@ +""" +Default scope function. + +`Paddle` manages Scope as programming language's scope. It just a +thread-local stack of Scope. Top of that stack is current scope, the bottom +of that stack is all scopes' parent. + +Invoking `create_var/get_var` can `create/get` variable in current scope. +Invoking `enter_local_scope/leave_local_scope` can create or destroy local +scope. + +A `scoped_function` will take a `function` as input. That function will be +invoked in a new local scope. +""" + +import paddle.v2.framework.core +import threading + +__tl_scope__ = threading.local() + +__all__ = [ + 'get_cur_scope', 'enter_local_scope', 'leave_local_scope', 'create_var', + 'get_var', 'scoped_function' +] + + +def get_cur_scope(): + """ + Get current scope. + :rtype: paddle.v2.framework.core.Scope + """ + cur_scope_stack = getattr(__tl_scope__, 'cur_scope', None) + if cur_scope_stack is None: + __tl_scope__.cur_scope = list() + if len(__tl_scope__.cur_scope) == 0: + __tl_scope__.cur_scope.append(paddle.v2.framework.core.Scope(None)) + return __tl_scope__.cur_scope[-1] + + +def enter_local_scope(): + """ + Enter a new local scope + """ + cur_scope = get_cur_scope() + new_scope = paddle.v2.framework.core.Scope(cur_scope) + __tl_scope__.cur_scope.append(new_scope) + + +def leave_local_scope(): + """ + Leave local scope + """ + __tl_scope__.cur_scope.pop() + + +def create_var(name): + """ + create variable in current scope. + """ + return get_cur_scope().create_var(name) + + +def get_var(name): + """ + get variable in current scope. + """ + return get_cur_scope().get_var(name) + + +def scoped_function(func): + """ + invoke `func` in new scope. + + :param func: a callable function that will be run in new scope. + :type func: callable + """ + enter_local_scope() + try: + func() + except: + raise + finally: + leave_local_scope() diff --git a/python/paddle/v2/framework/tests/CMakeLists.txt b/python/paddle/v2/framework/tests/CMakeLists.txt index d809917af1..7023e82b5f 100644 --- a/python/paddle/v2/framework/tests/CMakeLists.txt +++ b/python/paddle/v2/framework/tests/CMakeLists.txt @@ -1 +1,2 @@ -add_python_test(test_framework test_protobuf.py test_scope.py) +add_python_test(test_framework test_protobuf.py test_scope.py + test_default_scope_funcs.py) diff --git a/python/paddle/v2/framework/tests/test_default_scope_funcs.py b/python/paddle/v2/framework/tests/test_default_scope_funcs.py new file mode 100644 index 0000000000..81033deb15 --- /dev/null +++ b/python/paddle/v2/framework/tests/test_default_scope_funcs.py @@ -0,0 +1,33 @@ +from paddle.v2.framework.default_scope_funcs import * +import unittest + + +class TestDefaultScopeFuncs(unittest.TestCase): + def test_cur_scope(self): + self.assertIsNotNone(get_cur_scope()) + + def test_none_variable(self): + self.assertIsNone(get_var("test")) + + def test_create_var_get_var(self): + var_a = create_var("var_a") + self.assertIsNotNone(var_a) + self.assertIsNotNone(get_cur_scope().get_var('var_a')) + enter_local_scope() + self.assertIsNotNone(get_cur_scope().get_var('var_a')) + leave_local_scope() + + def test_var_get_int(self): + def __new_scope__(): + i = create_var("var_i") + self.assertFalse(i.is_int()) + i.set_int(10) + self.assertTrue(i.is_int()) + self.assertEqual(10, i.get_int()) + + for _ in xrange(10): + scoped_function(__new_scope__) + + +if __name__ == '__main__': + unittest.main() From 51690f104597da7b519d7209de4f2a76175a689c Mon Sep 17 00:00:00 2001 From: hedaoyuan Date: Wed, 12 Jul 2017 13:33:55 +0800 Subject: [PATCH 02/11] Compile for armv8. --- cmake/cross_compiling/android.cmake | 7 +++++++ cmake/external/openblas.cmake | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cmake/cross_compiling/android.cmake b/cmake/cross_compiling/android.cmake index 9724c16122..dcfbc5d012 100644 --- a/cmake/cross_compiling/android.cmake +++ b/cmake/cross_compiling/android.cmake @@ -106,6 +106,9 @@ IF("${CMAKE_VERSION}" VERSION_LESS "3.7.0") SET(CMAKE_SYSTEM_PROCESSOR armv7-a) ENDIF() ENDIF() + IF(ANDROID_ABI STREQUAL "arm64-v8a") + SET(ANDROID_TOOLCHAIN_NAME aarch64-linux-android) + ENDIF() SET(ANDROID_TOOLCHAIN_PREFIX "${ANDROID_TOOLCHAIN_ROOT}/bin/${ANDROID_TOOLCHAIN_NAME}-") ENDIF() @@ -162,6 +165,10 @@ IF("${CMAKE_VERSION}" VERSION_LESS "3.7.0") ENDIF() ENDIF() + IF(ANDROID_ABI STREQUAL "arm64-v8a") + LIST(APPEND ANDROID_COMPILER_FLAGS -march=armv8-a) + ENDIF() + STRING(REPLACE ";" " " ANDROID_COMPILER_FLAGS "${ANDROID_COMPILER_FLAGS}") STRING(REPLACE ";" " " ANDROID_LINKER_FLAGS "${ANDROID_LINKER_FLAGS}") diff --git a/cmake/external/openblas.cmake b/cmake/external/openblas.cmake index 5b9d9844ed..60a1041936 100644 --- a/cmake/external/openblas.cmake +++ b/cmake/external/openblas.cmake @@ -32,7 +32,12 @@ IF(NOT ${CBLAS_FOUND}) # arm_soft_fp_abi branch of OpenBLAS to support softfp # https://github.com/xianyi/OpenBLAS/tree/arm_soft_fp_abi SET(OPENBLAS_COMMIT "b5c96fcfcdc82945502a2303116a64d89985daf5") - SET(OPTIONAL_ARGS HOSTCC=${HOST_C_COMPILER} TARGET=ARMV7 ARM_SOFTFP_ABI=1 USE_THREAD=0) + IF(ANDROID_ABI MATCHES "^armeabi(-v7a)?$") + SET(TARGET "ARMV7") + ELSEIF(ANDROID_ABI STREQUAL "arm64-v8a") + SET(TARGET "ARMV8") + ENDIF() + SET(OPTIONAL_ARGS HOSTCC=${HOST_C_COMPILER} TARGET=${TARGET} ARM_SOFTFP_ABI=1 USE_THREAD=0) ELSEIF(RPI) # use hardfp SET(OPENBLAS_COMMIT "v0.2.19") From ea3a1df18bad2dd7e3ab12d776d7a3e9734ad9c6 Mon Sep 17 00:00:00 2001 From: hedaoyuan Date: Wed, 12 Jul 2017 21:15:34 +0800 Subject: [PATCH 03/11] Fix some compilation errors in some Android environments. --- CMakeLists.txt | 8 ++++++-- cmake/generic.cmake | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2c713db3e3..a3ab046289 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -28,7 +28,9 @@ if(NOT CMAKE_CROSSCOMPILING) endif(NOT CMAKE_CROSSCOMPILING) find_package(Git REQUIRED) find_package(Threads REQUIRED) -find_package(Boost QUIET) +if(NOT ANDROID) + find_package(Boost QUIET) +endif() include(simd) @@ -147,7 +149,9 @@ if(WITH_GOLANG) endif(WITH_GOLANG) add_subdirectory(paddle) -add_subdirectory(python) +if(WITH_PYTHON) + add_subdirectory(python) +endif() if(WITH_DOC) add_subdirectory(doc) endif() diff --git a/cmake/generic.cmake b/cmake/generic.cmake index 83e3d155d0..6e648cb53a 100644 --- a/cmake/generic.cmake +++ b/cmake/generic.cmake @@ -90,7 +90,7 @@ # including binary directory for generated headers. include_directories(${CMAKE_CURRENT_BINARY_DIR}) -if(NOT APPLE) +if(NOT APPLE AND NOT ANDROID) find_package(Threads REQUIRED) link_libraries(${CMAKE_THREAD_LIBS_INIT}) set(CMAKE_CXX_LINK_EXECUTABLE "${CMAKE_CXX_LINK_EXECUTABLE} -ldl") From 4e918377d0a37bd4216062ee4d9c19778bd284bd Mon Sep 17 00:00:00 2001 From: qijun Date: Thu, 13 Jul 2017 11:10:03 +0800 Subject: [PATCH 04/11] fix bug in dynload --- paddle/framework/CMakeLists.txt | 2 +- paddle/platform/dynload/cublas.h | 28 ++++++++++++++-------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/paddle/framework/CMakeLists.txt b/paddle/framework/CMakeLists.txt index b8642ca22a..de31952e79 100644 --- a/paddle/framework/CMakeLists.txt +++ b/paddle/framework/CMakeLists.txt @@ -11,7 +11,7 @@ proto_library(op_proto SRCS op_proto.proto DEPS attr_type) cc_test(op_proto_test SRCS op_proto_test.cc DEPS op_proto protobuf) proto_library(op_desc SRCS op_desc.proto DEPS attr_type) cc_test(op_desc_test SRCS op_desc_test.cc DEPS op_desc protobuf) -cc_library(operator SRCS operator.cc DEPS op_desc protobuf) +cc_library(operator SRCS operator.cc DEPS op_desc protobuf device_context) cc_test(operator_test SRCS operator_test.cc DEPS operator op_registry place) cc_library(op_registry SRCS op_registry.cc DEPS op_proto op_desc) cc_test(op_registry_test SRCS op_registry_test.cc DEPS op_registry operator) diff --git a/paddle/platform/dynload/cublas.h b/paddle/platform/dynload/cublas.h index 47c7a8ec21..c44b7240a8 100644 --- a/paddle/platform/dynload/cublas.h +++ b/paddle/platform/dynload/cublas.h @@ -67,20 +67,20 @@ extern void *cublas_dso_handle; __macro(cublasSgemm); \ __macro(cublasDgemm); \ __macro(cublasSgeam); \ - __macro(cublasDgeam); - -DECLARE_DYNAMIC_LOAD_CUBLAS_V2_WRAP(cublasCreate); -DECLARE_DYNAMIC_LOAD_CUBLAS_V2_WRAP(cublasDestroy); -DECLARE_DYNAMIC_LOAD_CUBLAS_V2_WRAP(cublasSetStream); -DECLARE_DYNAMIC_LOAD_CUBLAS_V2_WRAP(cublasSetPointerMode); -DECLARE_DYNAMIC_LOAD_CUBLAS_V2_WRAP(cublasGetPointerMode); -DECLARE_DYNAMIC_LOAD_CUBLAS_WRAP(cublasSgemmBatched); -DECLARE_DYNAMIC_LOAD_CUBLAS_WRAP(cublasDgemmBatched); -DECLARE_DYNAMIC_LOAD_CUBLAS_WRAP(cublasCgemmBatched); -DECLARE_DYNAMIC_LOAD_CUBLAS_WRAP(cublasZgemmBatched); -DECLARE_DYNAMIC_LOAD_CUBLAS_WRAP(cublasSgetrfBatched); -DECLARE_DYNAMIC_LOAD_CUBLAS_WRAP(cublasSgetriBatched); -DECLARE_DYNAMIC_LOAD_CUBLAS_WRAP(cublasDgetrfBatched); + __macro(cublasDgeam); \ + __macro(cublasCreate_v2); \ + __macro(cublasDestroy_v2); \ + __macro(cublasSetStream_v2); \ + __macro(cublasSetPointerMode_v2); \ + __macro(cublasGetPointerMode_v2); \ + __macro(cublasSgemmBatched); \ + __macro(cublasDgemmBatched); \ + __macro(cublasCgemmBatched); \ + __macro(cublasZgemmBatched); \ + __macro(cublasSgetrfBatched); \ + __macro(cublasSgetriBatched); \ + __macro(cublasDgetrfBatched); \ + __macro(cublasDgetriBatched) CUBLAS_BLAS_ROUTINE_EACH(DECLARE_DYNAMIC_LOAD_CUBLAS_WRAP); From 46c704ecf0eb259187681b4d2f184e6b1e0ba2ca Mon Sep 17 00:00:00 2001 From: dongzhihong Date: Thu, 13 Jul 2017 11:39:08 +0800 Subject: [PATCH 05/11] "fix init error" --- go/cmd/pserver/pserver.go | 2 +- go/pserver/service.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/cmd/pserver/pserver.go b/go/cmd/pserver/pserver.go index 48351ab6d0..b331b8126c 100644 --- a/go/cmd/pserver/pserver.go +++ b/go/cmd/pserver/pserver.go @@ -34,7 +34,7 @@ func main() { var idx int - var cp *pserver.Checkpoint + var cp pserver.Checkpoint var e *pserver.EtcdClient if *index >= 0 { idx = *index diff --git a/go/pserver/service.go b/go/pserver/service.go index 65db6970a7..a0319000ee 100644 --- a/go/pserver/service.go +++ b/go/pserver/service.go @@ -119,7 +119,7 @@ func NewCheckpointFromFile(cpPath string, idx int, e *EtcdClient) (*Checkpoint, // NewService creates a new service, will bypass etcd registration if no // endpoints specified. It will recovery from checkpoint file if a exists a specified checkpoint. -func NewService(idx int, interval time.Duration, path string, client *EtcdClient, cp *Checkpoint) (*Service, error) { +func NewService(idx int, interval time.Duration, path string, client *EtcdClient, cp Checkpoint) (*Service, error) { s := &Service{ idx: idx, checkpointInterval: interval, @@ -130,7 +130,7 @@ func NewService(idx int, interval time.Duration, path string, client *EtcdClient s.initialized = make(chan struct{}) if cp != nil { - for _, item := range *cp { + for _, item := range cp { p := ParameterWithConfig{ Param: item.Param, Config: item.Config, From 7cfcda5f4f184ee5958139d023cc72474ec2e81a Mon Sep 17 00:00:00 2001 From: dongzhihong Date: Thu, 13 Jul 2017 12:46:11 +0800 Subject: [PATCH 06/11] "fix checkpoint pointer" --- go/pserver/service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go/pserver/service.go b/go/pserver/service.go index a0319000ee..fec2ec61dc 100644 --- a/go/pserver/service.go +++ b/go/pserver/service.go @@ -83,7 +83,7 @@ type parameterCheckpoint struct { } // NewCheckpointFromFile loads parameters and state from checkpoint file -func NewCheckpointFromFile(cpPath string, idx int, e *EtcdClient) (*Checkpoint, error) { +func NewCheckpointFromFile(cpPath string, idx int, e *EtcdClient) (Checkpoint, error) { v, err := e.GetKey(PsPath+string(idx), 3*time.Second) if err != nil { return nil, err @@ -110,7 +110,7 @@ func NewCheckpointFromFile(cpPath string, idx int, e *EtcdClient) (*Checkpoint, } dec := gob.NewDecoder(bytes.NewReader(content)) - cp := &Checkpoint{} + cp := Checkpoint{} if err = dec.Decode(cp); err != nil { return nil, err } From a7c7a82626df16a4a9244499afd63f96d10adf94 Mon Sep 17 00:00:00 2001 From: hedaoyuan Date: Thu, 13 Jul 2017 13:03:38 +0800 Subject: [PATCH 07/11] Fix a small mistake. --- cmake/generic.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/generic.cmake b/cmake/generic.cmake index 2c1335d205..fcbdc64c63 100644 --- a/cmake/generic.cmake +++ b/cmake/generic.cmake @@ -94,7 +94,7 @@ if(NOT APPLE AND NOT ANDROID) find_package(Threads REQUIRED) link_libraries(${CMAKE_THREAD_LIBS_INIT}) set(CMAKE_CXX_LINK_EXECUTABLE "${CMAKE_CXX_LINK_EXECUTABLE} -ldl -lrt") -endif(NOT APPLE) +endif(NOT APPLE AND NOT ANDROID) function(merge_static_libs TARGET_NAME) set(libs ${ARGN}) From 728665d709811162ac1e2e136e44f88d6e68cb7f Mon Sep 17 00:00:00 2001 From: Qiao Longfei Date: Thu, 13 Jul 2017 15:57:22 +0800 Subject: [PATCH 08/11] Add Init to OperatorBase (#2838) --- paddle/framework/op_registry.h | 1 + paddle/framework/operator.h | 4 ++++ paddle/framework/operator_test.cc | 5 +++++ 3 files changed, 10 insertions(+) diff --git a/paddle/framework/op_registry.h b/paddle/framework/op_registry.h index 248c7a1a3b..e46da822c6 100644 --- a/paddle/framework/op_registry.h +++ b/paddle/framework/op_registry.h @@ -119,6 +119,7 @@ class OpRegistry { op->attrs_[attr.name()] = AttrTypeHelper::GetAttrValue(attr); } op_checkers().at(op_type).Check(op->attrs_); + op->Init(); return op; } diff --git a/paddle/framework/operator.h b/paddle/framework/operator.h index 0ce422e007..4336115670 100644 --- a/paddle/framework/operator.h +++ b/paddle/framework/operator.h @@ -49,6 +49,10 @@ class OperatorBase { std::string DebugString() const; + /// Init will be called after CreateOperator, you can put some initialization + /// logic here. + virtual void Init() {} + /// InferShape infer the size of Variables used by this Operator with /// information inside scope virtual void InferShape(const std::shared_ptr& scope) const = 0; diff --git a/paddle/framework/operator_test.cc b/paddle/framework/operator_test.cc index be8c4be2d4..01b87bb50e 100644 --- a/paddle/framework/operator_test.cc +++ b/paddle/framework/operator_test.cc @@ -21,14 +21,19 @@ namespace framework { class OperatorTest : public OperatorBase { public: + void Init() override { x = 1; } void InferShape(const std::shared_ptr& scope) const override {} void Run(const std::shared_ptr& scope, const platform::DeviceContext& dev_ctx) const override { float scale = GetAttr("scale"); ASSERT_NEAR(scale, 3.14, 1e-5); ASSERT_EQ(scope->GetVariable(inputs_[0]), nullptr); + ASSERT_EQ(x, 1); ASSERT_NE(scope->GetVariable(outputs_[0]), nullptr); } + + public: + float x = 0; }; class OperatorTestProtoAndCheckerMaker : public OpProtoAndCheckerMaker { From a0aaafe9de7008db91f32e50d36ee7d623bf1fa4 Mon Sep 17 00:00:00 2001 From: Yu Yang Date: Thu, 13 Jul 2017 17:29:33 +0800 Subject: [PATCH 09/11] Add a sample op, `add_op` * Refine register methods, make Op can get rid of whole-archieve * `USE_OP` before a op is used. * Add unittest for add_op. --- cmake/external/glog.cmake | 3 +- paddle/CMakeLists.txt | 1 + paddle/framework/CMakeLists.txt | 4 +- paddle/framework/op_registry.h | 78 ++++++++++++++++++++++++---- paddle/framework/op_registry_test.cc | 22 +++----- paddle/framework/operator.h | 53 +++++++++++++++---- paddle/framework/operator_test.cc | 44 +++++----------- paddle/op/CMakeLists.txt | 6 +++ paddle/op/add_op.cc | 44 ++++++++++++++++ paddle/op/add_op.cu | 5 ++ paddle/op/add_op.h | 17 ++++++ paddle/op/add_op_test.cc | 9 ++++ 12 files changed, 216 insertions(+), 70 deletions(-) create mode 100644 paddle/op/CMakeLists.txt create mode 100644 paddle/op/add_op.cc create mode 100644 paddle/op/add_op.cu create mode 100644 paddle/op/add_op.h create mode 100644 paddle/op/add_op_test.cc diff --git a/cmake/external/glog.cmake b/cmake/external/glog.cmake index bd401faa6e..8a594a825a 100644 --- a/cmake/external/glog.cmake +++ b/cmake/external/glog.cmake @@ -52,6 +52,7 @@ ExternalProject_Add( ADD_LIBRARY(glog STATIC IMPORTED GLOBAL) SET_PROPERTY(TARGET glog PROPERTY IMPORTED_LOCATION ${GLOG_LIBRARIES}) -ADD_DEPENDENCIES(glog extern_glog) +ADD_DEPENDENCIES(glog extern_glog gflags) +LINK_LIBRARIES(glog gflags) LIST(APPEND external_project_dependencies glog) diff --git a/paddle/CMakeLists.txt b/paddle/CMakeLists.txt index 0b5e9a2599..61d0aac602 100644 --- a/paddle/CMakeLists.txt +++ b/paddle/CMakeLists.txt @@ -14,6 +14,7 @@ if(Boost_FOUND) add_subdirectory(memory) add_subdirectory(platform) add_subdirectory(framework) + add_subdirectory(op) # because `operator` is a reserved word for CPP, so short to `op` add_subdirectory(pybind) endif() diff --git a/paddle/framework/CMakeLists.txt b/paddle/framework/CMakeLists.txt index de31952e79..8415ce67e9 100644 --- a/paddle/framework/CMakeLists.txt +++ b/paddle/framework/CMakeLists.txt @@ -11,8 +11,8 @@ proto_library(op_proto SRCS op_proto.proto DEPS attr_type) cc_test(op_proto_test SRCS op_proto_test.cc DEPS op_proto protobuf) proto_library(op_desc SRCS op_desc.proto DEPS attr_type) cc_test(op_desc_test SRCS op_desc_test.cc DEPS op_desc protobuf) -cc_library(operator SRCS operator.cc DEPS op_desc protobuf device_context) -cc_test(operator_test SRCS operator_test.cc DEPS operator op_registry place) +cc_library(operator SRCS operator.cc DEPS op_desc device_context) +cc_test(operator_test SRCS operator_test.cc DEPS operator op_registry) cc_library(op_registry SRCS op_registry.cc DEPS op_proto op_desc) cc_test(op_registry_test SRCS op_registry_test.cc DEPS op_registry operator) py_proto_compile(framework_py_proto SRCS attr_type.proto op_proto.proto op_desc.proto) diff --git a/paddle/framework/op_registry.h b/paddle/framework/op_registry.h index e46da822c6..e9e150224e 100644 --- a/paddle/framework/op_registry.h +++ b/paddle/framework/op_registry.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "paddle/framework/attr_checker.h" #include "paddle/framework/op_desc.pb.h" #include "paddle/framework/op_proto.pb.h" @@ -101,8 +102,11 @@ class OpRegistry { OpProto& op_proto = protos()[op_type]; OpAttrChecker& op_checker = op_checkers()[op_type]; ProtoMakerType(&op_proto, &op_checker); - PADDLE_ENFORCE(op_proto.IsInitialized(), - "Fail to initialize %s's OpProto !", op_type); + *op_proto.mutable_type() = op_type; + PADDLE_ENFORCE( + op_proto.IsInitialized(), + "Fail to initialize %s's OpProto, because %s is not initialized", + op_type, op_proto.InitializationErrorString()); } static OperatorBase* CreateOp(const OpDesc& op_desc) { @@ -143,18 +147,72 @@ class OpRegistry { template class OpRegisterHelper { public: - OpRegisterHelper(std::string op_type) { + OpRegisterHelper(const char* op_type) { OpRegistry::RegisterOp(op_type); } }; -#define REGISTER_OP(type, op_class, op_maker_class) \ - class op_class##Register { \ - private: \ - const static OpRegisterHelper reg; \ - }; \ - const OpRegisterHelper op_class##Register::reg( \ - #type) +#define STATIC_ASSERT_GLOBAL_NAMESPACE(uniq_name, msg) \ + struct __test_global_namespace_##uniq_name##__ {}; \ + static_assert(std::is_same<::__test_global_namespace_##uniq_name##__, \ + __test_global_namespace_##uniq_name##__>::value, \ + msg) + +#define REGISTER_OP(__op_type, __op_class, __op_maker_class) \ + STATIC_ASSERT_GLOBAL_NAMESPACE(__reg_op__##__op_type, \ + "REGISTER_OP must be in global namespace"); \ + static ::paddle::framework::OpRegisterHelper<__op_class, __op_maker_class> \ + __op_register_##__op_type##__(#__op_type); \ + int __op_register_##__op_type##_handle__() { return 0; } + +#define REGISTER_OP_KERNEL(type, GPU_OR_CPU, PlaceType, KernelType) \ + STATIC_ASSERT_GLOBAL_NAMESPACE( \ + __reg_op_kernel_##type##_##GPU_OR_CPU##__, \ + "REGISTER_OP_KERNEL must be in global namespace"); \ + struct __op_kernel_register__##type##__ { \ + __op_kernel_register__##type##__() { \ + ::paddle::framework::OperatorWithKernel::OpKernelKey key; \ + key.place_ = PlaceType(); \ + ::paddle::framework::OperatorWithKernel::AllOpKernels()[#type][key] \ + .reset(new KernelType()); \ + } \ + }; \ + static __op_kernel_register__##type##__ __reg_kernel_##type##__; \ + int __op_kernel_register_##type##_handle_##GPU_OR_CPU##__() { return 0; } + +#define REGISTER_OP_GPU_KERNEL(type, KernelType) \ + REGISTER_OP_KERNEL(type, GPU, ::paddle::platform::GPUPlace, KernelType) + +#define REGISTER_OP_CPU_KERNEL(type, KernelType) \ + REGISTER_OP_KERNEL(type, CPU, ::paddle::platform::CPUPlace, KernelType) + +#define USE_OP_WITHOUT_KERNEL(op_type) \ + STATIC_ASSERT_GLOBAL_NAMESPACE( \ + __use_op_without_kernel_##op_type, \ + "USE_OP_WITHOUT_KERNEL must be in global namespace"); \ + extern int __op_register_##op_type##_handle__(); \ + static int __use_op_ptr_##op_type##_without_kernel__ \ + __attribute__((unused)) = __op_register_##op_type##_handle__() + +#define USE_OP_KERNEL(op_type, CPU_OR_GPU) \ + STATIC_ASSERT_GLOBAL_NAMESPACE(__use_op_kernel_##op_type##_##CPU_OR_GPU##__, \ + "USE_OP_KERNEL must be in global namespace"); \ + extern int __op_kernel_register_##op_type##_handle_##CPU_OR_GPU##__(); \ + static int __use_op_ptr_##op_type##_##CPU_OR_GPU##_kernel__ \ + __attribute__((unused)) = \ + __op_kernel_register_##op_type##_handle_##CPU_OR_GPU##__() + +#ifdef PADDLE_ONLY_CPU +#define USE_OP(op_type) \ + USE_OP_WITHOUT_KERNEL(op_type); \ + USE_OP_KERNEL(op_type, CPU); + +#else +#define USE_OP(op_type) \ + USE_OP_WITHOUT_KERNEL(op_type); \ + USE_OP_KERNEL(op_type, CPU); \ + USE_OP_KERNEL(op_type, GPU) +#endif } // namespace framework } // namespace paddle diff --git a/paddle/framework/op_registry_test.cc b/paddle/framework/op_registry_test.cc index f5162fb870..b3460838f9 100644 --- a/paddle/framework/op_registry_test.cc +++ b/paddle/framework/op_registry_test.cc @@ -1,8 +1,6 @@ #include "paddle/framework/op_registry.h" #include -using namespace paddle::framework; - namespace paddle { namespace framework { class CosineOp : public OperatorBase { @@ -26,8 +24,6 @@ class CosineOpProtoAndCheckerMaker : public OpProtoAndCheckerMaker { } }; -REGISTER_OP(cos_sim, CosineOp, CosineOpProtoAndCheckerMaker); - class MyTestOp : public OperatorBase { public: void InferShape(const std::shared_ptr& scope) const override {} @@ -52,11 +48,14 @@ class MyTestOpProtoAndCheckerMaker : public OpProtoAndCheckerMaker { AddComment("This is my_test op"); } }; - -REGISTER_OP(my_test_op, MyTestOp, MyTestOpProtoAndCheckerMaker); } // namespace framework } // namespace paddle +REGISTER_OP(cos_sim, paddle::framework::CosineOp, + paddle::framework::CosineOpProtoAndCheckerMaker); +REGISTER_OP(my_test_op, paddle::framework::MyTestOp, + paddle::framework::MyTestOpProtoAndCheckerMaker); + TEST(OpRegistry, CreateOp) { paddle::framework::OpDesc op_desc; op_desc.set_type("cos_sim"); @@ -71,7 +70,7 @@ TEST(OpRegistry, CreateOp) { paddle::framework::OperatorBase* op = paddle::framework::OpRegistry::CreateOp(op_desc); - auto scope = std::make_shared(); + auto scope = std::make_shared(); paddle::platform::CPUDeviceContext dev_ctx; op->Run(scope, dev_ctx); float scale_get = op->GetAttr("scale"); @@ -114,7 +113,7 @@ TEST(OpRegistry, DefaultValue) { paddle::framework::OperatorBase* op = paddle::framework::OpRegistry::CreateOp(op_desc); - auto scope = std::make_shared(); + auto scope = std::make_shared(); paddle::platform::CPUDeviceContext dev_ctx; op->Run(scope, dev_ctx); ASSERT_EQ(op->GetAttr("scale"), 1.0); @@ -169,13 +168,8 @@ TEST(OpRegistry, CustomChecker) { paddle::framework::OperatorBase* op = paddle::framework::OpRegistry::CreateOp(op_desc); paddle::platform::CPUDeviceContext dev_ctx; - auto scope = std::make_shared(); + auto scope = std::make_shared(); op->Run(scope, dev_ctx); int test_attr = op->GetAttr("test_attr"); ASSERT_EQ(test_attr, 4); } - -int main(int argc, char** argv) { - testing::InitGoogleTest(&argc, argv); - return RUN_ALL_TESTS(); -} \ No newline at end of file diff --git a/paddle/framework/operator.h b/paddle/framework/operator.h index 4336115670..d3c55e0ceb 100644 --- a/paddle/framework/operator.h +++ b/paddle/framework/operator.h @@ -17,6 +17,7 @@ limitations under the License. */ #include #include #include +#include #include #include #include @@ -103,6 +104,19 @@ class OpKernel { virtual ~OpKernel() {} }; +template +struct VarToTensor {}; + +template <> +struct VarToTensor { + Tensor* operator()(Variable* var) { return var->GetMutable(); } +}; + +template <> +struct VarToTensor { + const Tensor* operator()(Variable* var) { return &var->Get(); } +}; + class OperatorWithKernel : public OperatorBase { public: struct OpKernelKey { @@ -136,19 +150,36 @@ class OperatorWithKernel : public OperatorBase { AllOpKernels() { static std::unordered_map g_all_op_kernels; return g_all_op_kernels; + } + void InferShape(const std::shared_ptr& scope) const final { + std::vector ins; + VarNamesToTensors(scope, inputs_, &ins); + std::vector outs; + VarNamesToTensors(scope, outputs_, &outs); + InferShape(ins, outs); }; + + private: + template + void VarNamesToTensors(const std::shared_ptr& scope, + const std::vector& var_names, + std::vector* container) const { + container->reserve(var_names.size()); + VarToTensor convert; + for (auto& name : var_names) { + auto var = scope->GetVariable(name); + if (var != nullptr) { + container->push_back(convert(var)); + } else { + container->push_back(nullptr); + } + } + } + + protected: + virtual void InferShape(const std::vector& inputs, + const std::vector& outputs) const = 0; }; } // namespace framework } // namespace paddle - -#define REGISTER_OP_KERNEL(type, PlaceType, KernelType) \ - struct __op_kernel_register__##type##__ { \ - __op_kernel_register__##type##__() { \ - ::paddle::framework::OperatorWithKernel::OpKernelKey key; \ - key.place_ = PlaceType(); \ - ::paddle::framework::OperatorWithKernel::AllOpKernels()[#type][key] \ - .reset(new KernelType()); \ - } \ - }; \ - static __op_kernel_register__##type##__ __reg_kernel_##type##__ diff --git a/paddle/framework/operator_test.cc b/paddle/framework/operator_test.cc index 01b87bb50e..a033ee1661 100644 --- a/paddle/framework/operator_test.cc +++ b/paddle/framework/operator_test.cc @@ -50,30 +50,6 @@ class OperatorTestProtoAndCheckerMaker : public OpProtoAndCheckerMaker { } }; -REGISTER_OP(test_operator, OperatorTest, OperatorTestProtoAndCheckerMaker); - -TEST(OperatorBase, all) { - OpDesc op_desc; - op_desc.set_type("test_operator"); - *op_desc.mutable_inputs()->Add() = "IN1"; - *op_desc.mutable_outputs()->Add() = "OUT1"; - auto attr = op_desc.mutable_attrs()->Add(); - attr->set_name("scale"); - attr->set_type(paddle::framework::AttrType::FLOAT); - float scale = 3.14; - attr->set_f(scale); - - platform::CPUDeviceContext device_context; - auto scope = std::make_shared(); - - OperatorBase* op = paddle::framework::OpRegistry::CreateOp(op_desc); - ASSERT_EQ(op->GetAttr("scale"), scale); - scope->CreateVariable("OUT1"); - op->Run(scope, device_context); - std::cout << op->DebugString() << std::endl; - delete op; -} - class OpKernelTestProtoAndCheckerMaker : public OpProtoAndCheckerMaker { public: OpKernelTestProtoAndCheckerMaker(OpProto* proto, OpAttrChecker* op_checker) @@ -83,14 +59,14 @@ class OpKernelTestProtoAndCheckerMaker : public OpProtoAndCheckerMaker { AddAttr("scale", "scale of cosine op") .SetDefault(1.0) .LargerThan(0.0); - AddType("test_operator"); AddComment("This is test op"); } }; class OpWithKernelTest : public OperatorWithKernel { - public: - void InferShape(const std::shared_ptr& scope) const override {} + protected: + void InferShape(const std::vector& inputs, + const std::vector& outputs) const override {} }; class CPUKernelTest : public OpKernel { @@ -103,10 +79,16 @@ class CPUKernelTest : public OpKernel { } }; -REGISTER_OP(op_with_kernel, OpWithKernelTest, OpKernelTestProtoAndCheckerMaker); -REGISTER_OP_KERNEL(op_with_kernel, platform::CPUPlace, CPUKernelTest); +} // namespace framework +} // namespace paddle + +REGISTER_OP(op_with_kernel, paddle::framework::OpWithKernelTest, + paddle::framework::OpKernelTestProtoAndCheckerMaker); +REGISTER_OP_CPU_KERNEL(op_with_kernel, paddle::framework::CPUKernelTest); TEST(OpKernel, all) { + using namespace paddle::framework; + OpDesc op_desc; op_desc.set_type("op_with_kernel"); *op_desc.mutable_inputs()->Add() = "IN1"; @@ -116,7 +98,7 @@ TEST(OpKernel, all) { attr->set_type(paddle::framework::AttrType::FLOAT); attr->set_f(3.14); - platform::CPUDeviceContext cpu_device_context; + paddle::platform::CPUDeviceContext cpu_device_context; auto scope = std::make_shared(); OperatorBase* op = paddle::framework::OpRegistry::CreateOp(op_desc); @@ -124,5 +106,3 @@ TEST(OpKernel, all) { delete op; } -} // namespace framework -} // namespace paddle \ No newline at end of file diff --git a/paddle/op/CMakeLists.txt b/paddle/op/CMakeLists.txt new file mode 100644 index 0000000000..40bb326512 --- /dev/null +++ b/paddle/op/CMakeLists.txt @@ -0,0 +1,6 @@ +if(WITH_GPU) + nv_library(add_op SRCS add_op.cc add_op.cu DEPS operator op_registry glog ddim) +else() + cc_library(add_op SRCS add_op.cc DEPS operator op_registry glog ddim) +endif() +cc_test(add_op_test SRCS add_op_test.cc DEPS add_op) diff --git a/paddle/op/add_op.cc b/paddle/op/add_op.cc new file mode 100644 index 0000000000..71fbe30289 --- /dev/null +++ b/paddle/op/add_op.cc @@ -0,0 +1,44 @@ +#include +#include +#include + +namespace paddle { +namespace op { + +class AddOp : public framework::OperatorWithKernel { +protected: + void InferShape( + const std::vector &inputs, + const std::vector &outputs) const override { + PADDLE_ENFORCE(inputs.size() == 2, "Input size of AddOp must be two"); + PADDLE_ENFORCE(outputs.size() == 1, "Output size of AddOp must be one"); + PADDLE_ENFORCE( + inputs[0] != nullptr && inputs[1] != nullptr && outputs[0] != nullptr, + "Inputs/Outputs of AddOp must all be set"); + PADDLE_ENFORCE(inputs[0]->dims() == inputs[1]->dims(), + "Two input of Add Op's dimension must be same."); + // Need set dims in Tensor + // outputs[0]->set_dims(inputs[0]->dims()) + } +}; + +class AddOpMaker : public framework::OpProtoAndCheckerMaker { +public: + AddOpMaker(framework::OpProto *proto, framework::OpAttrChecker *op_checker) + : framework::OpProtoAndCheckerMaker(proto, op_checker) { + AddInput("X", "The first input of add op"); + AddInput("Y", "The second input of add op"); + AddOutput("Out", "The output of add op"); + AddComment(R"DOC( +Two Element Add Operator. + +The equation is: Out = X + Y +)DOC"); + } +}; +} // namespace op +} // namespace paddle + +REGISTER_OP(add_two, paddle::op::AddOp, paddle::op::AddOpMaker); +REGISTER_OP_CPU_KERNEL(add_two, + ::paddle::op::AddKernel<::paddle::platform::CPUPlace>); \ No newline at end of file diff --git a/paddle/op/add_op.cu b/paddle/op/add_op.cu new file mode 100644 index 0000000000..d3d73d868b --- /dev/null +++ b/paddle/op/add_op.cu @@ -0,0 +1,5 @@ +#include +#include + +REGISTER_OP_GPU_KERNEL(add_two, + paddle::op::AddKernel); \ No newline at end of file diff --git a/paddle/op/add_op.h b/paddle/op/add_op.h new file mode 100644 index 0000000000..3a5a4fb00e --- /dev/null +++ b/paddle/op/add_op.h @@ -0,0 +1,17 @@ +#pragma once +#include +#include + +namespace paddle { +namespace op { + +template +class AddKernel : public framework::OpKernel { +public: + void Compute(const KernelContext &context) const override { + LOG(INFO) << "Add kernel in " << typeid(Place).name(); + } +}; + +} // namespace op +} // namespace paddle diff --git a/paddle/op/add_op_test.cc b/paddle/op/add_op_test.cc new file mode 100644 index 0000000000..f554ac1bef --- /dev/null +++ b/paddle/op/add_op_test.cc @@ -0,0 +1,9 @@ +#include +#define private public +#include +USE_OP(add_two); +TEST(AddOp, GetOpProto) { + auto& protos = paddle::framework::OpRegistry::protos(); + auto it = protos.find("add_two"); + ASSERT_NE(it, protos.end()); +} \ No newline at end of file From 79b70c2d697b6538eb55a0d6f092886630fbb13b Mon Sep 17 00:00:00 2001 From: Yu Yang Date: Thu, 13 Jul 2017 20:14:03 +0800 Subject: [PATCH 10/11] Follow comments * Convert `op` --> `operators` * Remove AddType in OpProtoMaker, because type is part of registry. * Rename CPU_OR_GPU --> DEVICE_TYPE in registry macro. --- paddle/CMakeLists.txt | 2 +- paddle/framework/op_registry.h | 17 ++++++++--------- paddle/framework/op_registry_test.cc | 2 -- paddle/framework/operator_test.cc | 14 -------------- paddle/{op => operators}/CMakeLists.txt | 0 paddle/{op => operators}/add_op.cc | 10 +++++----- paddle/{op => operators}/add_op.cu | 0 paddle/{op => operators}/add_op.h | 2 +- paddle/{op => operators}/add_op_test.cc | 0 9 files changed, 15 insertions(+), 32 deletions(-) rename paddle/{op => operators}/CMakeLists.txt (100%) rename paddle/{op => operators}/add_op.cc (84%) rename paddle/{op => operators}/add_op.cu (100%) rename paddle/{op => operators}/add_op.h (94%) rename paddle/{op => operators}/add_op_test.cc (100%) diff --git a/paddle/CMakeLists.txt b/paddle/CMakeLists.txt index 61d0aac602..4b06966fba 100644 --- a/paddle/CMakeLists.txt +++ b/paddle/CMakeLists.txt @@ -14,7 +14,7 @@ if(Boost_FOUND) add_subdirectory(memory) add_subdirectory(platform) add_subdirectory(framework) - add_subdirectory(op) # because `operator` is a reserved word for CPP, so short to `op` + add_subdirectory(operators) add_subdirectory(pybind) endif() diff --git a/paddle/framework/op_registry.h b/paddle/framework/op_registry.h index e9e150224e..61dfcb7049 100644 --- a/paddle/framework/op_registry.h +++ b/paddle/framework/op_registry.h @@ -82,8 +82,6 @@ class OpProtoAndCheckerMaker { return op_checker_->AddAttrChecker(name); } - void AddType(const std::string& op_type) { proto_->set_type(op_type); } - void AddComment(const std::string& comment) { *(proto_->mutable_comment()) = comment; } @@ -194,13 +192,14 @@ class OpRegisterHelper { static int __use_op_ptr_##op_type##_without_kernel__ \ __attribute__((unused)) = __op_register_##op_type##_handle__() -#define USE_OP_KERNEL(op_type, CPU_OR_GPU) \ - STATIC_ASSERT_GLOBAL_NAMESPACE(__use_op_kernel_##op_type##_##CPU_OR_GPU##__, \ - "USE_OP_KERNEL must be in global namespace"); \ - extern int __op_kernel_register_##op_type##_handle_##CPU_OR_GPU##__(); \ - static int __use_op_ptr_##op_type##_##CPU_OR_GPU##_kernel__ \ - __attribute__((unused)) = \ - __op_kernel_register_##op_type##_handle_##CPU_OR_GPU##__() +#define USE_OP_KERNEL(op_type, DEVICE_TYPE) \ + STATIC_ASSERT_GLOBAL_NAMESPACE( \ + __use_op_kernel_##op_type##_##DEVICE_TYPE##__, \ + "USE_OP_KERNEL must be in global namespace"); \ + extern int __op_kernel_register_##op_type##_handle_##DEVICE_TYPE##__(); \ + static int __use_op_ptr_##op_type##_##DEVICE_TYPE##_kernel__ \ + __attribute__((unused)) = \ + __op_kernel_register_##op_type##_handle_##DEVICE_TYPE##__() #ifdef PADDLE_ONLY_CPU #define USE_OP(op_type) \ diff --git a/paddle/framework/op_registry_test.cc b/paddle/framework/op_registry_test.cc index b3460838f9..9bcc0407ad 100644 --- a/paddle/framework/op_registry_test.cc +++ b/paddle/framework/op_registry_test.cc @@ -19,7 +19,6 @@ class CosineOpProtoAndCheckerMaker : public OpProtoAndCheckerMaker { AddAttr("scale", "scale of cosine op") .SetDefault(1.0) .LargerThan(0.0); - AddType("cos"); AddComment("This is cos op"); } }; @@ -44,7 +43,6 @@ class MyTestOpProtoAndCheckerMaker : public OpProtoAndCheckerMaker { }; AddAttr("test_attr", "a simple test attribute") .AddCustomChecker(my_checker); - AddType("my_test_op"); AddComment("This is my_test op"); } }; diff --git a/paddle/framework/operator_test.cc b/paddle/framework/operator_test.cc index a033ee1661..204b601a4a 100644 --- a/paddle/framework/operator_test.cc +++ b/paddle/framework/operator_test.cc @@ -36,20 +36,6 @@ class OperatorTest : public OperatorBase { float x = 0; }; -class OperatorTestProtoAndCheckerMaker : public OpProtoAndCheckerMaker { - public: - OperatorTestProtoAndCheckerMaker(OpProto* proto, OpAttrChecker* op_checker) - : OpProtoAndCheckerMaker(proto, op_checker) { - AddInput("input", "input of test op"); - AddOutput("output", "output of test op"); - AddAttr("scale", "scale of cosine op") - .SetDefault(1.0) - .LargerThan(0.0); - AddType("test_operator"); - AddComment("This is test op"); - } -}; - class OpKernelTestProtoAndCheckerMaker : public OpProtoAndCheckerMaker { public: OpKernelTestProtoAndCheckerMaker(OpProto* proto, OpAttrChecker* op_checker) diff --git a/paddle/op/CMakeLists.txt b/paddle/operators/CMakeLists.txt similarity index 100% rename from paddle/op/CMakeLists.txt rename to paddle/operators/CMakeLists.txt diff --git a/paddle/op/add_op.cc b/paddle/operators/add_op.cc similarity index 84% rename from paddle/op/add_op.cc rename to paddle/operators/add_op.cc index 71fbe30289..2766f0bf25 100644 --- a/paddle/op/add_op.cc +++ b/paddle/operators/add_op.cc @@ -1,9 +1,9 @@ #include #include -#include +#include namespace paddle { -namespace op { +namespace operators { class AddOp : public framework::OperatorWithKernel { protected: @@ -39,6 +39,6 @@ The equation is: Out = X + Y } // namespace op } // namespace paddle -REGISTER_OP(add_two, paddle::op::AddOp, paddle::op::AddOpMaker); -REGISTER_OP_CPU_KERNEL(add_two, - ::paddle::op::AddKernel<::paddle::platform::CPUPlace>); \ No newline at end of file +REGISTER_OP(add_two, paddle::operators::AddOp, paddle::operators::AddOpMaker); +REGISTER_OP_CPU_KERNEL( + add_two, ::paddle::operators::AddKernel<::paddle::platform::CPUPlace>); \ No newline at end of file diff --git a/paddle/op/add_op.cu b/paddle/operators/add_op.cu similarity index 100% rename from paddle/op/add_op.cu rename to paddle/operators/add_op.cu diff --git a/paddle/op/add_op.h b/paddle/operators/add_op.h similarity index 94% rename from paddle/op/add_op.h rename to paddle/operators/add_op.h index 3a5a4fb00e..17d459dbc8 100644 --- a/paddle/op/add_op.h +++ b/paddle/operators/add_op.h @@ -3,7 +3,7 @@ #include namespace paddle { -namespace op { +namespace operators { template class AddKernel : public framework::OpKernel { diff --git a/paddle/op/add_op_test.cc b/paddle/operators/add_op_test.cc similarity index 100% rename from paddle/op/add_op_test.cc rename to paddle/operators/add_op_test.cc From e588730147069e967e4c9108d24d663a5a5b55f6 Mon Sep 17 00:00:00 2001 From: qiaolongfei Date: Fri, 14 Jul 2017 09:56:27 +0800 Subject: [PATCH 11/11] change op to operators --- paddle/operators/add_op.cu | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/paddle/operators/add_op.cu b/paddle/operators/add_op.cu index d3d73d868b..5979345fff 100644 --- a/paddle/operators/add_op.cu +++ b/paddle/operators/add_op.cu @@ -1,5 +1,5 @@ -#include +#include #include REGISTER_OP_GPU_KERNEL(add_two, - paddle::op::AddKernel); \ No newline at end of file + paddle::operators::AddKernel); \ No newline at end of file