Feature/backward #3068

reyoung · 2017-07-26T08:05:49Z

No description provided.

Remove bool argument, use a class to handle that.

…into feature/backward

…backward

…ture/backward

…into feature/backward

…ture/backward

…into feature/backward

…backward

wangkuiyi · 2017-08-01T03:00:10Z

https://stackoverflow.com/questions/9243646/recursive-functions-two-functions-or-last-optional-parameter/9243666#9243666 BackwardImpl => BackwardRecursive

jacquesqiao · 2017-08-01T03:01:47Z

paddle/operators/fill_zeros_like_op.h

+class FillZerosLikeKernel : public framework::OpKernel {
+public:
+  void Compute(const framework::KernelContext& context) const override {
+    auto* output = context.Output(0)->GetMutable<framework::Tensor>();


auto output

jacquesqiao · 2017-08-01T03:04:49Z

paddle/operators/fill_zeros_like_op.cu

+#include "paddle/operators/fill_zeros_like_op.h"
+
+REGISTER_OP_GPU_KERNEL(
+    fill_zeros_like,


why name this fill_zerors_like, numpy call it zeros_like

zeros_like is not a verb. But all operator should be a verb. I think fill_zeros_like is more explicit.

Also, zeros_like may be implemented without filling anything into memory but a flag. Here named fill indicate that actually a new memory is allocated.

jacquesqiao · 2017-08-01T03:05:48Z

paddle/operators/fill_zeros_like_op.h

+
+template <typename Place, typename T>
+class FillZerosLikeKernel : public framework::OpKernel {
+public:


please add a Python unit test for this operator

We will add the unit test in other PR. @Canpio @dzhwinter

@reyoung get it

jacquesqiao · 2017-08-01T03:16:24Z

paddle/framework/operator.cc

  auto output_format = GetAttr<std::vector<int>>("output_format");
  auto offset = in_out_idxs_->at(name);
-
+  PADDLE_ENFORCE(output_format.at((size_t)offset + 1) <= (int)outputs_.size(),


(int) ==> static_cast

jacquesqiao · 2017-08-01T03:24:04Z

paddle/framework/net_op_test.cc

+  op1->inputs_ = {"x", "w1", "b1"};
+  op1->outputs_ = {"y"};
+  net.AddOp(op1);
+  net.InsertOp(0, op1);


check if the inserted op is in the right place

jacquesqiao · 2017-08-01T03:46:30Z

paddle/framework/backward.cc

+ *
+ * See Backward.h for details
+ */
+static std::shared_ptr<OperatorBase> BackwardImpl(


Is MakeBackwardOp or GetBackwardOp a better name?

Done. BackwardRecursive.

jacquesqiao · 2017-08-01T03:54:20Z

paddle/framework/backward.cc

+  }
+
+  /**
+   * All output gradients of forwarding operator do not need to calculate. Then


add If All output gradient ...

jacquesqiao · 2017-08-01T04:34:50Z

paddle/framework/backward.cc

+           OpRegistry::CreateOp(
+               "add", {dup_outputs}, {name},
+               {{"input_format",
+                 std::vector<int>{0, (int)dup_outputs.size()}}})});


static_cast

jacquesqiao · 2017-08-01T05:32:20Z

paddle/framework/backward.cc

+ *
+ * See Backward.h for details
+ */
+static std::shared_ptr<OperatorBase> BackwardImpl(


This function is not in a class, why need static?

jacquesqiao · 2017-08-01T05:40:39Z

paddle/framework/backward.cc

+
+    //! Map from output gradient variable name to operator's indices in backward
+    //! net. That operator generates that variable.
+    std::unordered_map<std::string, std::vector<size_t>> dup_output_ops;


what is dup mean？

jacquesqiao · 2017-08-01T05:47:02Z

paddle/framework/backward.cc

+namespace paddle {
+namespace framework {
+
+static bool AllInSet(const std::vector<std::string>& names,


Why do we need to use static?

Because we do not export them to global symbols.

jacquesqiao · 2017-08-01T06:15:23Z

paddle/framework/backward.cc

+      insert_position.push_back(
+          {dup_op.back(),
+           OpRegistry::CreateOp(
+               "add", {dup_outputs}, {name},


这个add op现在应该还没实现？

是的。这个Op的实现不影响Backward算法的实现和单测。

jacquesqiao · 2017-08-01T06:51:31Z

paddle/framework/backward_test.cc

+
+  ASSERT_EQ("X" + f::OperatorBase::GRAD_VAR_SUFFIX(),
+            gop->Output("X" + f::OperatorBase::GRAD_VAR_SUFFIX()));
+  //  LOG(INFO) << gop->Output("X" + "@GRAD");


remove unused code

jacquesqiao · 2017-08-01T07:08:42Z

paddle/framework/backward_test.cc

+  auto fwd = f::OpRegistry::CreateOp("rowwise_add", {"X", "b"}, {"Out"}, {});
+  ASSERT_NE(fwd, nullptr);
+  auto gop = f::OpRegistry::CreateGradOp(*fwd);
+  ASSERT_EQ(1UL, gop->inputs_.size());


记得说GradientOp的Input是I,O,OG，这里为什么gop的input size为1？

因为有一些Op的梯度可以不使用I和O，这个在注册的时候注册过了。

jacquesqiao · 2017-08-01T07:11:45Z

paddle/framework/backward_test.cc

+            gop->outputs_.end());
+
+  auto no_input_gop = f::Backward(*fwd, {"X", "b"});
+  ASSERT_NE(no_input_gop, nullptr);


好像没有做任何测试？

…ture/backward

jacquesqiao · 2017-08-01T07:50:14Z

paddle/operators/fill_zeros_like_op.h

@@ -23,8 +23,8 @@ namespace operators {
 template <typename Place, typename T>
 class FillZerosLikeKernel : public framework::OpKernel {
 public:
-  void Compute(const framework::KernelContext& context) const override {
-    auto* output = context.Output(0)->GetMutable<framework::Tensor>();
+  void Compute(const framework::ExecutionContext& context) const override {


framework::xxx ===> xxx

Changed in Next PR

jacquesqiao · 2017-08-01T08:06:35Z

有一些注释的comment，也可以顺便update一下

jacquesqiao

LGTM！except for some typo in comment and doc

reyoung · 2017-08-01T09:01:28Z

@dzhwinter @Canpio @wangkuiyi

I prepare to merge this PR soon, all code-style comments are followed. But it still lacks documentation and comments. But it has been a huge PR already. Documentation and comments will be added in following PR.

reyoung and others added 30 commits July 26, 2017 15:06

Rename PlainNet --> NetOp

b1b4364

Update Backward

ecf23ce

Update Interface

b1b13f8

Refine OpRegistry::AddInput/AddOutput

00615eb

Remove bool argument, use a class to handle that.

Add fill_zeros_like op

a2dc961

Develop backward building precess of single op

e32e306

Refining Unittest

831d4e1

Merge branch 'feature/backward' of https://github.com/reyoung/Paddle …

f77c63b

…into feature/backward

"backward is NetOp"

fa7cbfd

Merge remote-tracking branch 'reyoung/feature/backward' into feature/…

0ac79a3

…backward

"split to generic add PR"

292f2ab

Stash

05d9aff

Merge branch 'feature/backward' of github.com:reyoung/Paddle into fea…

fa6a46a

…ture/backward

Fix compile error

03f418c

Merge branch 'feature/backward' of https://github.com/reyoung/Paddle …

5297bcb

…into feature/backward

Merge branch 'feature/backward' of github.com:reyoung/Paddle into fea…

9475972

…ture/backward

Fix compile error

f9fab14

Add unittest for part_of_output_are_not_need

3d18737

Fix compile errors of FillZerosLikeOp

70bd07a

Stash for canpio

63636d6

Add unitest of Backward.part_of_input_are_not_need

04db418

Stash

28c0281

Merge branch 'feature/backward' of github.com:reyoung/Paddle into fea…

099bb53

…ture/backward

Add unitest of Backward.intermediate_variable_not_need_in_linear_net

3dd5fd0

Add unittest

84198f7

Merge branch 'feature/backward' of https://github.com/reyoung/Paddle …

4461f3c

…into feature/backward

rename test

b1d8419

InsertOp for NetOp

d2583bd

"wait add generic"

b9f2bb3

Merge remote-tracking branch 'reyoung/feature/backward' into feature/…

5713266

…backward

jacquesqiao reviewed Aug 1, 2017

View reviewed changes

reyoung added 3 commits August 1, 2017 15:11

Merge branch 'develop' of github.com:baidu/Paddle into feature/backward

bc146e8

Merge branch 'feature/backward' of github.com:reyoung/Paddle into fea…

80baf86

…ture/backward

Follow comments and merge develop

e2fd2bd

jacquesqiao reviewed Aug 1, 2017

View reviewed changes

jacquesqiao approved these changes Aug 1, 2017

View reviewed changes

Use static_cast, Fix unittest

737ea05

reyoung force-pushed the feature/backward branch from 47dbbdb to 737ea05 Compare August 1, 2017 08:57

reyoung added 2 commits August 1, 2017 19:25

Merge branch 'develop' of github.com:baidu/Paddle into feature/backward

9cc9907

Merge develop

051d6c8

reyoung merged commit 2646ef6 into PaddlePaddle:develop Aug 1, 2017

reyoung added this to Done in PaddlePaddle Refactoring: Phase 1 Aug 2, 2017

reyoung deleted the feature/backward branch August 2, 2017 11:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/backward #3068

Feature/backward #3068

reyoung commented Jul 26, 2017

wangkuiyi commented Aug 1, 2017

jacquesqiao Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

dzhwinter Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao Aug 1, 2017 •

edited

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao Aug 1, 2017

reyoung Aug 1, 2017

jacquesqiao commented Aug 1, 2017

jacquesqiao left a comment

reyoung commented Aug 1, 2017

Feature/backward #3068

Feature/backward #3068

Conversation

reyoung commented Jul 26, 2017

wangkuiyi commented Aug 1, 2017

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

jacquesqiao Aug 1, 2017 • edited

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

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

jacquesqiao commented Aug 1, 2017

jacquesqiao left a comment

Choose a reason for hiding this comment

reyoung commented Aug 1, 2017

jacquesqiao Aug 1, 2017 •

edited