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

Op refine for making build system more automatic #3852

Closed
wants to merge 32 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
c20a01d
Add cuDNN Wrapper.
qingqing01 Aug 31, 2017
3b16201
Merge branch 'develop' of /~https://github.com/PaddlePaddle/Paddle into…
qingqing01 Aug 31, 2017
2071322
Add unit testing for cuDNN wrapper.
qingqing01 Aug 31, 2017
d4087ef
Merge branch 'develop' of /~https://github.com/PaddlePaddle/Paddle into…
qingqing01 Aug 31, 2017
9d2909b
rename add_op to add_two_op
luotao1 Sep 1, 2017
96cdb2c
Merge branch 'develop' of /~https://github.com/PaddlePaddle/Paddle into…
qingqing01 Sep 4, 2017
8c048aa
Remove cudnn_helper.cc
qingqing01 Sep 4, 2017
58369d5
Merge branch 'develop' into op_refine
luotao1 Sep 4, 2017
faa4da4
fix ctc edit distance in v2 API.
lcy-seso Sep 4, 2017
a60128a
move identity_op from scale_op.cc to be a single file
luotao1 Sep 4, 2017
fb49bc2
rename mse_cost into square_error_cost.
lcy-seso Sep 4, 2017
c2c5df2
update doc.
lcy-seso Sep 4, 2017
0b478e9
follow comments.
lcy-seso Sep 4, 2017
5b93ac7
package a new USE_NO_KERNEL_OP for USE_OP_ITSELF
luotao1 Sep 4, 2017
10eacac
follow comments and fix unittest.
lcy-seso Sep 4, 2017
740c8ba
remove scatter_op.cu/gather_op.cu as they support only_cpu now
luotao1 Sep 4, 2017
d7b2058
Merge pull request #3845 from lcy-seso/rename_mse_to_square_error
lcy-seso Sep 4, 2017
3542027
Merge pull request #3791 from qingqing01/cudnn_wrapper
qingqing01 Sep 4, 2017
a523bea
fix getType.
lcy-seso Sep 4, 2017
7c78542
add missing protostr for sub_nested_seq_layer.
lcy-seso Sep 4, 2017
77a15b6
Merge pull request #3844 from lcy-seso/fix_ctc_evaluator
lcy-seso Sep 4, 2017
9a3c69c
Revert "Remove `grad_op_type` in `REGISTER_OP`"
JiayiFeng Sep 4, 2017
94f7562
Merge pull request #3859 from PaddlePaddle/revert-3824-remove_grad_op…
JiayiFeng Sep 4, 2017
bb0a11c
Add a graph building example
Sep 4, 2017
9f3bcd0
Update image
Sep 4, 2017
e4494bb
Correct optimziation part
Sep 4, 2017
35bb0c0
Merge pull request #3860 from wangkuiyi/graph_construction_design_doc
wangkuiyi Sep 4, 2017
b3463bf
Merge pull request #3851 from luotao1/no_kernel_op
luotao1 Sep 5, 2017
843a8b1
Merge pull request #3856 from lcy-seso/fix_sub_nested_seq_protostr
luotao1 Sep 5, 2017
3ab9327
Merge branch 'op_refine' of /~https://github.com/luotao1/Paddle into op…
luotao1 Sep 5, 2017
020e45f
follow comments to revert add_two_op to add_op
luotao1 Sep 5, 2017
d44663f
auto find .cc or .cu in operator/CMakeLists.txt
luotao1 Sep 5, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion paddle/framework/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ cc_test(operator_test SRCS operator_test.cc DEPS operator op_registry)
cc_library(grad_op_builder SRCS grad_op_builder.cc DEPS operator)
cc_library(op_registry SRCS op_registry.cc DEPS grad_op_builder)
cc_test(op_registry_test SRCS op_registry_test.cc DEPS op_registry)
cc_test(grad_op_builder_test SRCS grad_op_builder_test.cc DEPS grad_op_builder op_registry add_op)
cc_test(grad_op_builder_test SRCS grad_op_builder_test.cc DEPS grad_op_builder op_registry add_two_op)

py_proto_compile(framework_py_proto SRCS framework.proto)
# Generate an empty __init__.py to make framework_py_proto as a valid python module.
Expand Down
24 changes: 17 additions & 7 deletions paddle/operators/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,30 @@ endfunction()

add_subdirectory(math)

list(REMOVE_ITEM GENERAL_OPS
net_op
minus_op
mul_op
recurrent_op
scale_op)
set(ONLYCPU_OPS
net_op
gather_op
scatter_op)
foreach(src ${ONLYCPU_OPS})
op_library(${src} SRCS ${src}.cc)
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 make it simpler if we make op_library smarter, just call op_library(${src}), then it can do the following:

find if have both .cc and .cu file, call cc_library and nv_library to add corresponding library.

So we can call op_library(${src}) for general ops and op_library(${src} DEPS xxx) for ops have dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, I will refine it.

Most ops have the same name for .cc and .cu. But for recurrent_op, the ${src} is not recurrent_op.cc and recurrent_op.cu.

Thus, if ${src} is none, cmake will auto find ${src}.cc and ${src}.cu. If user specific ${src}, cmake will use what user specify.

endforeach()

op_library(net_op SRCS net_op.cc)
set(DEPS_OPS
identity_op
minus_op
mul_op
recurrent_op
scale_op)
op_library(identity_op SRCS identity_op.cc DEPS scale_op)
op_library(minus_op SRCS minus_op.cc minus_op.cu DEPS scale_op)
op_library(mul_op SRCS mul_op.cc mul_op.cu DEPS math_function)
op_library(recurrent_op SRCS recurrent_op.cc rnn/recurrent_op_utils.cc
DEPS framework_proto tensor operator net_op)
op_library(scale_op SRCS scale_op.cc scale_op.cu DEPS net_op)

list(REMOVE_ITEM GENERAL_OPS
${ONLYCPU_OPS}
${DEPS_OPS})
foreach(src ${GENERAL_OPS})
op_library(${src} SRCS ${src}.cc ${src}.cu)
endforeach()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License. */

#include "paddle/operators/add_op.h"
#include "paddle/operators/add_two_op.h"

namespace paddle {
namespace operators {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

#define EIGEN_USE_GPU
#include "paddle/framework/op_registry.h"
#include "paddle/operators/add_op.h"
#include "paddle/operators/add_two_op.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why rename to add_two_op?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because REGISTER_OP(add_two, ops::AddOp, ops::AddOpMaker, add_two_grad, ops::AddOpGrad); in add_op.cc, and USE_OP(add_two) in pybind.cc

Copy link
Contributor

Choose a reason for hiding this comment

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

I see.

Copy link
Contributor

Choose a reason for hiding this comment

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

add_two_op seems odd, add_op may be more clear? Since we have mul_op.

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 agree, then I will revert and change add_two in REGISTER_OP and USE_OP to be add.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks very much!


namespace ops = paddle::operators;
REGISTER_OP_GPU_KERNEL(add_two,
Expand Down
File renamed without changes.
20 changes: 0 additions & 20 deletions paddle/operators/gather_op.cu

This file was deleted.

54 changes: 54 additions & 0 deletions paddle/operators/identity_op.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License. */

#include "paddle/operators/net_op.h"
#include "paddle/operators/scale_op.h"

namespace paddle {
namespace operators {

// identity is a alias of scale op. This is also a example for creating a alias
// operator.
template <typename AttrType>
class IdentityOpMaker : public framework::OpProtoAndCheckerMaker {
public:
IdentityOpMaker(framework::OpProto *proto,
framework::OpAttrChecker *op_checker)
: OpProtoAndCheckerMaker(proto, op_checker) {
AddInput("X", "input tensor of identity op");
AddOutput("Out", "output tensor of identity op");
AddComment("identity operator. Just a alias of scale op which scale = 1.0");
}
};

template <typename AttrType>
class IdentityOp : public NetOp {
public:
IdentityOp(const std::string &type, const framework::VariableNameMap &inputs,
const framework::VariableNameMap &outputs,
const framework::AttributeMap &attrs)
: NetOp(type, inputs, outputs, attrs) {
AppendOp(framework::OpRegistry::CreateOp(
"scale", {{"X", {Input("X")}}}, {{"Out", {Output("Out")}}},
{{"scale", static_cast<AttrType>(1)}}));
}
};

} // namespace operators
} // namespace paddle

namespace ops = paddle::operators;

REGISTER_OP_WITHOUT_GRADIENT(identity, ops::IdentityOp<float>,
ops::IdentityOpMaker<float>);
31 changes: 1 addition & 30 deletions paddle/operators/scale_op.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ The equation is: Out = scale*X
}
};

// Identity Op's gradient is identity op, too.
// Scale Op's gradient is scale op, too.
// Grad(Out=scale(X)) => Grad(X) = scale(Grad(Out))
template <typename AttrType>
class ScaleGradOp : public NetOp {
Expand All @@ -65,33 +65,6 @@ class ScaleGradOp : public NetOp {
}
};

// identity is a alias of scale op. This is also a example for creating a alias
// operator.
template <typename AttrType>
class IdentityOpMaker : public framework::OpProtoAndCheckerMaker {
public:
IdentityOpMaker(framework::OpProto *proto,
framework::OpAttrChecker *op_checker)
: OpProtoAndCheckerMaker(proto, op_checker) {
AddInput("X", "input tensor of identity op");
AddOutput("Out", "output tensor of identity op");
AddComment("identity operator. Just a alias of scale op which scale = 1.0");
}
};

template <typename AttrType>
class IdentityOp : public NetOp {
public:
IdentityOp(const std::string &type, const framework::VariableNameMap &inputs,
const framework::VariableNameMap &outputs,
const framework::AttributeMap &attrs)
: NetOp(type, inputs, outputs, attrs) {
AppendOp(framework::OpRegistry::CreateOp(
"scale", {{"X", {Input("X")}}}, {{"Out", {Output("Out")}}},
{{"scale", static_cast<AttrType>(1)}}));
}
};

} // namespace operators
} // namespace paddle

Expand All @@ -101,5 +74,3 @@ REGISTER_OP(scale, ops::ScaleOp, ops::ScaleOpMaker<float>,
ops::ScaleGradOp<float>);
REGISTER_OP_CPU_KERNEL(scale,
ops::ScaleKernel<paddle::platform::CPUPlace, float>);
REGISTER_OP_WITHOUT_GRADIENT(identity, ops::IdentityOp<float>,
ops::IdentityOpMaker<float>);
20 changes: 0 additions & 20 deletions paddle/operators/scatter_op.cu

This file was deleted.