Refine InferShape for recurrent_network_op #3124

qingqing01 · 2017-07-31T12:24:14Z

The tensor only contains the shape and does not hold memory when inferring shape.

* the tensor only contains shape and does not hold memory when inferring shape.

… rnn_infershape

Superjomn · 2017-07-31T12:30:54Z

paddle/operators/recurrent_network_op.cc

      step_input->Resize(step_dims);
    }
  }
 }

 void ConcatOutputs(std::vector<std::shared_ptr<Scope>>& step_scopes,
                   const std::vector<Link>& outlinks,
-                   const size_t seq_len) {
+                   const size_t seq_len,
+                   bool infer_shape) {


is_infer or infer_mode?

infer_shape_mode

Superjomn · 2017-07-31T12:32:46Z

paddle/operators/recurrent_network_op.cc

@@ -97,18 +104,14 @@ void LinkMemories(std::vector<std::shared_ptr<Scope>>& scopes,
  std::shared_ptr<Scope> scope = scopes[step_id];
  std::shared_ptr<Scope> linked_scope = scopes[step_id + offset];
  for (auto& attr : memories) {
-    auto mem = scope->CreateVariable(attr.pre_var)->GetMutable<Tensor>();
+    auto mem = scope->GetVariable(attr.pre_var)->GetMutable<Tensor>();


need an enforce on scope->GetVariable(xxx) != nullptr ?

Superjomn · 2017-07-31T12:33:36Z

paddle/operators/recurrent_network_op.cc

  for (size_t i = 0; i < seq_len_; i++) {
    if (i > 0) {
-      rnn::LinkMemories(step_scopes, arg_->memories, i, -1);
+      rnn::LinkMemories(step_scopes, arg_->memories, i, -1, true);


true /* infer_mode */

Superjomn · 2017-07-31T12:33:49Z

paddle/operators/recurrent_network_op.cc

-        step_scopes[0]->GetVariable(outlinks[i].external)->GetMutable<Tensor>();
-    output->Resize(make_ddim(dims_vec));
-  }
+  rnn::ConcatOutputs(step_scopes, arg_->outlinks, seq_len_, true);


same as top

Superjomn · 2017-07-31T12:34:08Z

paddle/operators/recurrent_network_op.cc

    if (step_id > 0) {
-      rnn::LinkMemories(step_scopes, arg_->memories, step_id, -1);
+      rnn::LinkMemories(step_scopes, arg_->memories, step_id, -1, false);


same as the top

Superjomn · 2017-07-31T12:36:43Z

paddle/operators/recurrent_network_op_test.cc

    net->AddOp(
        OpRegistry::CreateOp("mul", {"rnn/h@pre", "rnn/w"}, {"rnn/s"}, {}));

    net->AddOp(
-        OpRegistry::CreateOp("add_two", {"rnn/x", "rnn/s"}, {"rnn/h"}, {}));
+        OpRegistry::CreateOp("add_two", {"x@alias", "rnn/s"}, {"rnn/h"}, {}));


an inline function for the action of adding @alias suffix ?

This unit test may move to Python, so not add an inline function for adding @alias suffix.

Superjomn · 2017-07-31T12:37:18Z

paddle/operators/recurrent_network_op_test.cc

@@ -318,7 +313,7 @@ class RecurrentGradientAlgorithmTest : public ::testing::Test {
        scope_->GetVariable("step_scopes")
            ->GetMutable<std::vector<std::shared_ptr<Scope>>>();
    for (int i = 1; i < 10; ++i) {
-      rnn::LinkMemories(*step_scopes, memories, i, -1);
+      rnn::LinkMemories(*step_scopes, memories, i, -1, true);


same as the top, `true /* infer_mode */

luotao1 · 2017-07-31T13:40:36Z

recurrentOp里可以调用infershape，也可以不调用infershape?请问分别是什么时候呢

jacquesqiao · 2017-07-31T23:39:47Z

paddle/operators/recurrent_network_op_test.cc

@@ -347,7 +342,7 @@ TEST(RecurrentOp, LinkMemories) {
    scope->CreateVariable("pre_h");
    auto tensor = scope->CreateVariable("h")->GetMutable<Tensor>();
    float* data = tensor->mutable_data<float>(make_ddim({15, 20}), CPUPlace());
-    for (int i = 0; i < 15 * 20; ++i) {
+    for (int j = 0; j < 15 * 20; ++j) {


data[i] ==> data[j] ?

jacquesqiao · 2017-08-01T02:38:20Z

paddle/operators/recurrent_network_op.cc

@@ -29,7 +29,8 @@ namespace rnn {

 void SegmentInputs(std::vector<std::shared_ptr<Scope>>& step_scopes,
                   const std::vector<Link>& inlinks,
-                   const size_t seq_len) {
+                   const size_t seq_len,


const size_t&

const size_t is ok.

jacquesqiao · 2017-08-01T02:38:32Z

paddle/operators/recurrent_network_op.cc

      step_input->Resize(step_dims);
    }
  }
 }

 void ConcatOutputs(std::vector<std::shared_ptr<Scope>>& step_scopes,
                   const std::vector<Link>& outlinks,
-                   const size_t seq_len) {
+                   const size_t seq_len,


const size_t&

const size_t is ok.

jacquesqiao · 2017-08-01T02:38:50Z

paddle/operators/recurrent_network_op.cc

@@ -79,8 +85,9 @@ void ConcatOutputs(std::vector<std::shared_ptr<Scope>>& step_scopes,

 void LinkMemories(std::vector<std::shared_ptr<Scope>>& scopes,
                  const std::vector<rnn::MemoryAttr>& memories,
-                  size_t step_id,
-                  int offset) {
+                  const size_t step_id,


const size_t&

… rnn_infershape

Superjomn · 2017-08-01T09:48:01Z

paddle/operators/recurrent_op.cc

    }
    net->GetMutable<NetOp>()->Run(step_scopes[step_id], dev_ctx);
  }
-  LinkBootMemoryGradients(step_scopes[0]);
-  rnn::ConcatOutputs(step_scopes, arg_->outlinks, seq_len_);
+  LinkBootMemoryGradients(step_scopes[0], false);


false add a comment ?

Superjomn · 2017-08-01T09:49:53Z

paddle/operators/recurrent_op_test.cc

@@ -362,7 +361,7 @@ TEST(RecurrentOp, LinkMemories) {
  memories.push_back(mem_attr);

  for (int i = 1; i < len; ++i) {
-    rnn::LinkMemories(step_scopes, memories, i, -1);
+    rnn::LinkMemories(step_scopes, memories, i, -1, false /*infer_shape_mode*/);
  }
  // check
  for (int i = 0; i < len - 1; ++i) {


unify all the array index from int to size_t

Superjomn

LGTM

jacquesqiao

LGTM！

qingqing01 added 2 commits July 31, 2017 20:23

Refine InferShape for recurrent_network_op.

6cfb9a3

* the tensor only contains shape and does not hold memory when inferring shape.

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

b89d15a

… rnn_infershape

qingqing01 requested review from Superjomn, reyoung, jacquesqiao and luotao1 July 31, 2017 12:25

Superjomn reviewed Jul 31, 2017

View reviewed changes

jacquesqiao reviewed Jul 31, 2017

View reviewed changes

jacquesqiao reviewed Aug 1, 2017

View reviewed changes

qingqing01 added 3 commits August 1, 2017 16:34

follow comments.

8925295

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

79e89ef

… rnn_infershape

Rename recurrent_network_op recurrent_op.

b94584c

Superjomn reviewed Aug 1, 2017

View reviewed changes

qingqing01 added 3 commits August 1, 2017 20:50

resolve conflicts.

18aa0dc

resolve conficts.

99b835c

resovle conflicts.

7c49a4f

Superjomn reviewed Aug 2, 2017

View reviewed changes

jacquesqiao approved these changes Aug 3, 2017

View reviewed changes

qingqing01 merged commit 017182c into PaddlePaddle:develop Aug 3, 2017

qingqing01 deleted the rnn_infershape branch March 7, 2018 12:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refine InferShape for recurrent_network_op #3124

Refine InferShape for recurrent_network_op #3124

qingqing01 commented Jul 31, 2017

Superjomn Jul 31, 2017

qingqing01 Aug 1, 2017

Superjomn Jul 31, 2017

qingqing01 Aug 1, 2017

Superjomn Jul 31, 2017

qingqing01 Aug 1, 2017

Superjomn Jul 31, 2017

qingqing01 Aug 1, 2017

Superjomn Jul 31, 2017

qingqing01 Aug 1, 2017

Superjomn Jul 31, 2017

qingqing01 Aug 1, 2017

Superjomn Jul 31, 2017

qingqing01 Aug 1, 2017

luotao1 commented Jul 31, 2017

jacquesqiao Jul 31, 2017

qingqing01 Aug 1, 2017

jacquesqiao Aug 1, 2017

qingqing01 Aug 1, 2017

jacquesqiao Aug 1, 2017

qingqing01 Aug 1, 2017

jacquesqiao Aug 1, 2017

Superjomn Aug 1, 2017

Superjomn Aug 1, 2017

Superjomn left a comment

jacquesqiao left a comment

Refine InferShape for recurrent_network_op #3124

Refine InferShape for recurrent_network_op #3124

Conversation

qingqing01 commented Jul 31, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luotao1 commented Jul 31, 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

Superjomn left a comment

Choose a reason for hiding this comment

jacquesqiao left a comment

Choose a reason for hiding this comment