Prelu with forward, backward and python test passed #4121

zchen0211 · 2017-09-15T05:33:07Z

Will add GPU when gpu parts are ready.

… develop

qingqing01 · 2017-09-15T05:39:59Z

paddle/operators/prelu_op.cc

+namespace paddle {
+namespace operators {
+
+class PreluOp : public framework::OperatorWithKernel {


PreluOp -> PReluOp

qingqing01 · 2017-09-15T05:41:27Z

paddle/operators/prelu_op.cc

+ protected:
+  void InferShape(const framework::InferShapeContext &ctx) const override {
+    auto *in = ctx.Input<framework::Tensor>("X");
+    auto *out = ctx.Output<framework::LoDTensor>("Out");


Add not-null check for Input, like : https://github.com/PaddlePaddle/Paddle/pull/4086/files#diff-1fcd5ee1c1e63ed40789a0e60fdb1bf6R29 . Since the more check, the more readable error messages.

qingqing01 · 2017-09-15T05:42:53Z

paddle/operators/prelu_op.cc

+
+The equation is:
+f(x) = alpha * x , for x < 0
+f(x) = x         , for x >= 0


Add space before and after formula. Also add indent for formula, since the doc may be converted MarkDown.

qingqing01 · 2017-09-15T05:43:11Z

paddle/operators/prelu_op.cc

+f(x) = x         , for x >= 0
+)DOC");
+    AddAttr<float>("alpha", "The scaling factor alpha of prelu.")
+        .SetDefault(0.0);


Put the Attr before doc. Should use type template for attr alpha. Please refer to scale_op.

qingqing01 · 2017-09-15T05:44:24Z

paddle/operators/prelu_op.cc

+};
+
+// The operator to calculate gradients of a prelu operator.
+class PreluGradOp : public framework::OperatorWithKernel {


PreluGradOp -> PReluGradOp

qingqing01 · 2017-09-15T05:44:40Z

paddle/operators/prelu_op.h

+using EigenVector = framework::EigenVector<T, MajorType, IndexType>;
+
+template <typename Place, typename T>
+class PreluKernel : public framework::OpKernel {


PreluKernel -> PReluKernel

qingqing01 · 2017-09-15T05:45:51Z

paddle/operators/prelu_op.h

+
+    // auto place = context.GetEigenDevice<Place>();
+    // Out_vec.device(place)
+    Out_vec = X_vec.cwiseMax(0.f) + X_vec.cwiseMin(0.f) * alpha;


Need to use Eigen device to support GPU and CPU at the same time.

qingqing01 · 2017-09-15T05:48:23Z

python/paddle/v2/framework/tests/test_prelu_op.py

+class PreluTest(OpTest):
+    def setUp(self):
+        self.op_type = "prelu"
+        self.inputs = {'X': np.random.normal(size=(3, 5)).astype("float32")}


size=(3, 5) may be too small.

qingqing01 · 2017-09-15T05:58:17Z

paddle/operators/prelu_op.h

+        dX->data<T>()[i] = dO->data<T>()[i];
+      } else {
+        dX->data<T>()[i] = dO->data<T>()[i] * alpha;
+      }


可以使用paddle::platform::Transform + functor同时支持CPU和GPU.

Transfrom: https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/platform/transform.h
Transfrom用例可以参考单测： https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/platform/transform_test.cu

… develop

qingqing01 · 2017-09-18T10:27:33Z

paddle/operators/prelu_op.cc

+
+ protected:
+  void InferShape(const framework::InferShapeContext &ctx) const override {
+    PADDLE_ENFORCE_NOT_NULL(ctx.InputVar("X"), "Input(X) should not be null");


Sorry for the incomplete comments last time, the not-null check for output is also needed:

PADDLE_ENFORCE_NOT_NULL(ctx.OutputVar("Out"), "Output(X) should not be null");

I think the comment "Output(X) should not be null" is unnecessary, PADDLE_ENFORCE_NOT_NULL is enough semantically.

qingqing01 · 2017-09-18T10:28:18Z

paddle/operators/prelu_op.cc

+
+)DOC");
+    AddAttr<AttrType>("alpha", "The scaling factor alpha of prelu.")
+        .SetDefault(0.0);


Put the AddComment(R"DOC )DOC") at last.

qingqing01 · 2017-09-18T10:32:01Z

paddle/operators/prelu_op.cc

+  f(x) = x         , for x >= 0
+
+)DOC");
+    AddAttr<AttrType>("alpha", "The scaling factor alpha of prelu.")


Different from other activations, the alpha in PRelu is a learnable weight, so it should be the input, not the attr. And need to calculate the gradient of this weight in the backward op.

qingqing01 · 2017-09-18T10:39:52Z

paddle/operators/prelu_op.h

+using platform::Transform;
+
+template <typename T>
+class Prelu_functor {


Use google C++ Style: https://google.github.io/styleguide/cppguide.html#Type_Names

template <typename T> class PReluFunctor { };

qingqing01 · 2017-09-18T10:40:20Z

paddle/operators/prelu_op.h

+};
+
+template <typename T>
+class Prelu_Grad_functor {


Prelu_Grad_functor -> PReluGradFunctor

qingqing01 · 2017-09-18T10:41:25Z

paddle/operators/prelu_op.h

+class PReluGradKernel : public framework::OpKernel {
+ public:
+  void Compute(const framework::ExecutionContext& context) const override {
+    auto* dX = context.Output<Tensor>(framework::GradVarName("X"));


dX -> dx , https://google.github.io/styleguide/cppguide.html#Variable_Names

The names of variables (including function parameters) and data members are all lowercase, with underscores between words.

qingqing01 · 2017-09-18T10:41:35Z

paddle/operators/prelu_op.h

+
+    auto alpha = static_cast<T>(context.Attr<AttrType>("alpha"));
+
+    T* dX_ptr = dX->mutable_data<T>(context.GetPlace());


dX_ptr -> dx_ptr

qingqing01 · 2017-09-18T10:42:12Z

python/paddle/v2/framework/tests/test_prelu_op.py

+from op_test import OpTest
+
+
+class PreluTest(OpTest):


PreluTest -> PReluTest

qingqing01 · 2017-09-18T10:43:52Z

python/paddle/v2/framework/tests/test_prelu_op.py

+        self.check_output()
+
+    def test_check_grad(self):
+        self.check_grad(['X'], 'Out')


If modify the alpha as input, also add the check for ignoring one of the input's gradient, like mul_op:
: https://github.com/PaddlePaddle/Paddle/blob/develop/python/paddle/v2/framework/tests/test_mul_op.py#L49

… develop

Superjomn · 2017-09-18T22:36:59Z

paddle/operators/prelu_op.cc

+ protected:
+  void InferShape(const framework::InferShapeContext &ctx) const override {
+    PADDLE_ENFORCE_NOT_NULL(ctx.InputVar("X"), "Input(X) should not be null");
+    auto *in = ctx.Input<framework::Tensor>("X");


first time to get inputs, enforce all the pointers are not null, or this will core.

Superjomn · 2017-09-18T22:38:18Z

paddle/operators/prelu_op.cc

+ public:
+  PReluOpMaker(framework::OpProto *proto, framework::OpAttrChecker *op_checker)
+      : OpProtoAndCheckerMaker(proto, op_checker) {
+    AddInput("X", "The input tensor of prelu operator.");


prelu -> PRELU or some other names, make sure this represents a shorthand for some algorithm.

Superjomn · 2017-09-18T22:39:52Z

paddle/operators/prelu_op.cc

+        ctx.Output<framework::LoDTensor>(framework::GradVarName("X"));
+    auto *X = ctx.Input<framework::Tensor>("X");
+
+    X_grad->Resize(X->dims());


enforce X_grad is not null

Superjomn · 2017-09-18T22:40:55Z

paddle/operators/prelu_op.h

+  HOSTDEVICE T operator()(const T& X) const {
+    if (X > 0)
+      return X;
+    else


if () {} else {}
or just return X>0 ? X : X * alpha_;

Superjomn · 2017-09-18T22:41:22Z

paddle/operators/prelu_op.h

+  explicit Prelu_Grad_functor(const T& alpha) : alpha_(alpha) {}
+
+  HOSTDEVICE T operator()(const T& Out, const T& dOut) const {
+    if (Out > 0)


same comment as above

Superjomn · 2017-09-18T22:41:38Z

paddle/operators/prelu_op.h

+
+    auto alpha = static_cast<T>(context.Attr<AttrType>("alpha"));
+
+    T* dX_ptr = dX->mutable_data<T>(context.GetPlace());


enforce dX is not null first

qingqing01

@zchen0211 @Superjom

I approve this PR. Since PReluGradKernel will be be updated later. @zchen0211 can update codes based on @Superjom 's comments later.

zchen0211 added 4 commits September 14, 2017 17:06

prelu op

58b5b08

prelu modify

260026f

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

e615c84

… develop

prelu_op

490ca5f

qingqing01 added the OpPorting label Sep 15, 2017

qingqing01 reviewed Sep 15, 2017

View reviewed changes

zchen0211 and others added 7 commits September 15, 2017 11:55

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

384368f

… develop

fix

c7dfec1

new prelu with functor

1b2374a

Do not invoke GPU method when use_gpu=false

490482a

prelu with gpu

86afb85

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

f5516b5

… develop

prelu fix

c165d23

qingqing01 requested a review from Superjomn September 18, 2017 09:10

qingqing01 reviewed Sep 18, 2017

View reviewed changes

zchen0211 added 3 commits September 18, 2017 10:50

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

4a23788

… develop

prelu fix

b6347fb

prelu

1b79746

Superjomn reviewed Sep 18, 2017

View reviewed changes

zchen0211 added 3 commits September 18, 2017 16:36

prelu finalize

3c3a6d9

fix gradient not stable

154d88c

prelu

6d1446e

qingqing01 approved these changes Sep 19, 2017

View reviewed changes

zchen0211 merged commit f86c1cc into PaddlePaddle:develop Sep 19, 2017

qingqing01 mentioned this pull request Sep 19, 2017

prelu_op compile error #4189

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prelu with forward, backward and python test passed #4121

Prelu with forward, backward and python test passed #4121

zchen0211 commented Sep 15, 2017 •

edited by qingqing01

Loading

qingqing01 Sep 15, 2017

qingqing01 Sep 15, 2017

qingqing01 Sep 15, 2017

qingqing01 Sep 15, 2017

qingqing01 Sep 15, 2017

qingqing01 Sep 15, 2017

qingqing01 Sep 15, 2017

qingqing01 Sep 15, 2017

qingqing01 Sep 15, 2017

qingqing01 Sep 18, 2017 •

edited

Loading

Superjomn Sep 19, 2017

qingqing01 Sep 18, 2017

qingqing01 Sep 18, 2017 •

edited

Loading

qingqing01 Sep 18, 2017

qingqing01 Sep 18, 2017

qingqing01 Sep 18, 2017

qingqing01 Sep 18, 2017

qingqing01 Sep 18, 2017

qingqing01 Sep 18, 2017

Superjomn Sep 18, 2017

Superjomn Sep 18, 2017

Superjomn Sep 18, 2017

Superjomn Sep 18, 2017

Superjomn Sep 18, 2017

Superjomn Sep 18, 2017

qingqing01 left a comment •

edited

Loading


		auto alpha = static_cast<T>(context.Attr<AttrType>("alpha"));

		T* dX_ptr = dX->mutable_data<T>(context.GetPlace());

Prelu with forward, backward and python test passed #4121

Prelu with forward, backward and python test passed #4121

Conversation

zchen0211 commented Sep 15, 2017 • edited by qingqing01 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qingqing01 Sep 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qingqing01 Sep 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qingqing01 left a comment • edited Loading

Choose a reason for hiding this comment

zchen0211 commented Sep 15, 2017 •

edited by qingqing01

Loading

qingqing01 Sep 18, 2017 •

edited

Loading

qingqing01 Sep 18, 2017 •

edited

Loading

qingqing01 left a comment •

edited

Loading