Add Lod Reset Operator #4747

kuke · 2017-10-12T04:16:20Z

Resolve #4719

qingqing01 · 2017-10-20T08:05:49Z

paddle/operators/lod_reset_op.cc

+    AddOutput("Out", "The output tensor of lod_reset operator.");
+    AddAttr<std::vector<int>>("target_lod_0",
+                              "Target level 0 LoD of "
+                              "lod_reset operator.");


Now, the attributes are usually fixed during training, that is to say, it was passed to the network when creating the graph. I'm not sure whether the attributes can be changed during training in the future. And since the batch size and the sequence length may be varying during training, I think it's better to pass LoD as an input, not attributes. The input can be calculated by others operator.

qingqing01 · 2017-11-09T12:55:38Z

paddle/operators/lod_reset_op.cc

+  LoDResetOpMaker(framework::OpProto *proto,
+                  framework::OpAttrChecker *op_checker)
+      : OpProtoAndCheckerMaker(proto, op_checker) {
+    AddInput("X", "(LoDTensor)The input tensor of lod_reset operator.");


(LoDTensor)The -> (LoDTensor) The

qingqing01 · 2017-11-09T12:56:19Z

paddle/operators/lod_reset_op.cc

+                  framework::OpAttrChecker *op_checker)
+      : OpProtoAndCheckerMaker(proto, op_checker) {
+    AddInput("X", "(LoDTensor)The input tensor of lod_reset operator.");
+    AddInput("target_lod_in",


target_lod_in -> TargetLoD

kuke

Thanks. Updated by following the comments

kuke · 2017-11-09T13:46:29Z

paddle/operators/lod_reset_op.cc

+  LoDResetOpMaker(framework::OpProto *proto,
+                  framework::OpAttrChecker *op_checker)
+      : OpProtoAndCheckerMaker(proto, op_checker) {
+    AddInput("X", "(LoDTensor)The input tensor of lod_reset operator.");


kuke · 2017-11-09T13:46:36Z

paddle/operators/lod_reset_op.cc

+                  framework::OpAttrChecker *op_checker)
+      : OpProtoAndCheckerMaker(proto, op_checker) {
+    AddInput("X", "(LoDTensor)The input tensor of lod_reset operator.");
+    AddInput("target_lod_in",


qingqing01 · 2017-11-09T14:12:27Z

paddle/operators/lod_reset_op.h

+                     "Target LoD should be an ascending vector.");
+    }
+
+    out->CopyFrom(*in, ctx.GetPlace(), ctx.device_context());


out->ShareDataWith(*in);

qingqing01 · 2017-11-09T14:13:04Z

paddle/operators/lod_reset_op.h

+    d_x->mutable_data<T>(ctx.GetPlace());
+
+    auto in_dims = d_x->dims();
+    d_x->CopyFrom(*d_out, ctx.GetPlace(), ctx.device_context());


d_x->ShareDataWith(*d_out);

qingqing01 · 2017-11-09T14:14:45Z

paddle/operators/lod_reset_op.cc

+An example:
+Given a float LoDTensor X with lod = [[0, 2, 5, 6]]
+
+    [[1, 2], [3, 4, 5], [6]]


In fact, the LodTensor X is saved as with shape (6, 1) in our codes:

[1, 2, 3, 4, 5, 6]

But [[1, 2], [3, 4, 5], [6]] is easy to understand, I wonder how to add this comments.

qingqing01 · 2017-11-09T14:17:32Z

paddle/operators/lod_reset_op.h

+    std::vector<int> level0;
+    if (lod_t) {
+      auto* lod = lod_t->data<int>();
+      if (!platform::is_cpu_place(ctx.GetPlace())) {


if (platform::is_gpu_place(ctx.GetPlace())) { }

kuke

Thanks

kuke · 2017-11-10T05:45:09Z

paddle/operators/lod_reset_op.cc

+An example:
+Given a float LoDTensor X with lod = [[0, 2, 5, 6]]
+
+    [[1, 2], [3, 4, 5], [6]]


kuke · 2017-11-10T05:45:22Z

paddle/operators/lod_reset_op.h

+    std::vector<int> level0;
+    if (lod_t) {
+      auto* lod = lod_t->data<int>();
+      if (!platform::is_cpu_place(ctx.GetPlace())) {


kuke · 2017-11-10T05:45:50Z

paddle/operators/lod_reset_op.h

+                     "Target LoD should be an ascending vector.");
+    }
+
+    out->CopyFrom(*in, ctx.GetPlace(), ctx.device_context());


kuke · 2017-11-10T05:45:56Z

paddle/operators/lod_reset_op.h

+    d_x->mutable_data<T>(ctx.GetPlace());
+
+    auto in_dims = d_x->dims();
+    d_x->CopyFrom(*d_out, ctx.GetPlace(), ctx.device_context());


qingqing01

LGTM.

pkuyym

LGTM

kuke changed the title ~~add lod_reset_op~~ Add Lod Reset Operator Oct 12, 2017

kuke requested a review from qingqing01 October 12, 2017 04:17

qingqing01 added the OpPorting label Oct 12, 2017

qingqing01 reviewed Oct 20, 2017

View reviewed changes

qingqing01 reviewed Nov 9, 2017

View reviewed changes

kuke commented Nov 9, 2017

View reviewed changes

qingqing01 reviewed Nov 9, 2017

View reviewed changes

kuke commented Nov 10, 2017

View reviewed changes

qingqing01 previously approved these changes Nov 10, 2017

View reviewed changes

kuke dismissed qingqing01’s stale review via f7ad5c1 November 13, 2017 04:36

kuke closed this Nov 13, 2017

kuke force-pushed the seq_reshape_op_dev branch from f7ad5c1 to e602c70 Compare November 13, 2017 08:42

bug fix in lod_reset_op: cast int to size_t in LoD

9bc7108

kuke reopened this Nov 13, 2017

Yibing Liu added 2 commits November 13, 2017 17:08

trigger ci for lod_reset_op

c6275ec

Merge branch 'develop' of upstream into seq_reshape_op_dev

6772dfa

pkuyym approved these changes Nov 14, 2017

View reviewed changes

kuke merged commit 2f3d1c3 into PaddlePaddle:develop Nov 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Lod Reset Operator #4747

Add Lod Reset Operator #4747

kuke commented Oct 12, 2017

qingqing01 Oct 20, 2017 •

edited

kuke Nov 9, 2017

qingqing01 Nov 9, 2017

kuke Nov 9, 2017

qingqing01 Nov 9, 2017

kuke Nov 9, 2017

kuke left a comment

kuke Nov 9, 2017

kuke Nov 9, 2017

qingqing01 Nov 9, 2017

kuke Nov 10, 2017

qingqing01 Nov 9, 2017

kuke Nov 10, 2017

qingqing01 Nov 9, 2017

kuke Nov 10, 2017

qingqing01 Nov 9, 2017

kuke Nov 10, 2017

kuke left a comment

kuke Nov 10, 2017

kuke Nov 10, 2017

kuke Nov 10, 2017

kuke Nov 10, 2017

qingqing01 left a comment

pkuyym left a comment

Add Lod Reset Operator #4747

Add Lod Reset Operator #4747

Conversation

kuke commented Oct 12, 2017

qingqing01 Oct 20, 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

kuke left a 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

kuke left a 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

Choose a reason for hiding this comment

pkuyym left a comment

Choose a reason for hiding this comment

qingqing01 Oct 20, 2017 •

edited