Enhance reshape #9008

lcy-seso · 2018-03-12T14:36:50Z

kuke · 2018-03-14T07:20:54Z

Is there some problem in CI?

kuke

BTW, any specially consideration to move this operator from ops to nn?

kuke · 2018-03-14T12:55:45Z

paddle/fluid/operators/reshape_op.cc

+  bool ValidateShape(const std::vector<int> &shape,
+                     const framework::DDim &input_dim,
+                     std::vector<int64_t> &output_shape) const {
+    // only one dimension canbe set to -1, whose size will be automatically


canbe -> can be

kuke · 2018-03-14T13:00:29Z

paddle/fluid/operators/reshape_op.cc

      }
    }
+    PADDLE_ENFORCE_LE(
+        neg_dims_idx.size(), 1,
+        "Only one input dimension of Attr(shape) may be unknown.");


may -> can

kuke · 2018-03-14T13:08:16Z

paddle/fluid/operators/reshape_op.cc

@@ -99,7 +127,7 @@ and target shape = [1, 4], the reshape operator will transform
 the tensor X into a 2-D tensor: [[1, 2, 3, 4]]

 One dimension in the target shape can be set -1, representing that its
-size is unknown. In this case, the real dimension will be infered from 
+size is unknown. In this case, the real dimension will be infered from
 the original shape of Input(X) and other dimensions in the target shape.
 )DOC");


Need to explain when the dimension can be set to 0 (necessary) and in-place reshape (optional) in the doc.

kuke · 2018-03-14T13:08:47Z

paddle/fluid/operators/reshape_op.cc

-                              "(vector<int>) "
-                              "Target shape of reshape operator.");
+    AddAttr<std::vector<int>>(
+        "shape", "(std::vector<int>) Target shape of reshape operator.");
    AddAttr<bool>("inplace",
                  "Change the source tensor's shape without copy memory.")


copy -> copying

kuke · 2018-03-14T13:11:17Z

python/paddle/fluid/layers/nn.py

+
+        Given a 3-D tensor x with shape [2, 3, 8] and the new shape:
+        [-1, 0, 2, 2]. The reshape layer will change tensor X into a 4-D tensor
+        with shape [4, 3, 2, 2] with its data unchanged.


Also need to refine this doc

lcy-seso

follow comments.

lcy-seso · 2018-03-19T02:27:04Z

paddle/fluid/operators/reshape_op.cc

+  bool ValidateShape(const std::vector<int> &shape,
+                     const framework::DDim &input_dim,
+                     std::vector<int64_t> &output_shape) const {
+    // only one dimension canbe set to -1, whose size will be automatically


lcy-seso · 2018-03-19T02:27:37Z

paddle/fluid/operators/reshape_op.cc

      }
    }
+    PADDLE_ENFORCE_LE(
+        neg_dims_idx.size(), 1,
+        "Only one input dimension of Attr(shape) may be unknown.");


lcy-seso · 2018-03-19T03:13:48Z

paddle/fluid/operators/reshape_op.cc

-                              "(vector<int>) "
-                              "Target shape of reshape operator.");
+    AddAttr<std::vector<int>>(
+        "shape", "(std::vector<int>) Target shape of reshape operator.");
    AddAttr<bool>("inplace",
                  "Change the source tensor's shape without copy memory.")


lcy-seso · 2018-03-19T03:28:05Z

python/paddle/fluid/layers/nn.py

+
+        Given a 3-D tensor x with shape [2, 3, 8] and the new shape:
+        [-1, 0, 2, 2]. The reshape layer will change tensor X into a 4-D tensor
+        with shape [4, 3, 2, 2] with its data unchanged.


lcy-seso · 2018-03-19T03:28:12Z

paddle/fluid/operators/reshape_op.cc

@@ -99,7 +127,7 @@ and target shape = [1, 4], the reshape operator will transform
 the tensor X into a 2-D tensor: [[1, 2, 3, 4]]

 One dimension in the target shape can be set -1, representing that its
-size is unknown. In this case, the real dimension will be infered from 
+size is unknown. In this case, the real dimension will be infered from
 the original shape of Input(X) and other dimensions in the target shape.
 )DOC");


lcy-seso · 2018-03-19T05:17:13Z

@kuke Reply to the previous comment about "why remove this operator from ops into nn".

Actually, my only consideration is to generate a more accurate documentation for users. Currently, our API documents are generated by Python API helpers. Helpers for layers in ops.py are automatically generated, and their documentation is from the C++ codes (including comments for inputs, outputs, attributes and op comments written in C++ codes). The display format is hard to control, and parameter's data type is not correct for users who use python APIs.

guoshengCS · 2018-03-21T12:44:23Z

This update expects to make InferShape suitable to both compile-time and run-time. Specifically, InferShape will act as follows:

For each dimension size of the output, copy the corresponding dimension size of input if the given value of that dimension is 0, otherwise, use the given value as that dimension size. And multiply the dimension sizes to compute capacity.
If unknown dimension exists, divide the product of input dimension sizes by the capacity computed before to infer the unknown dimension size.

pkuyym · 2018-03-22T02:57:14Z

python/paddle/fluid/layers/nn.py

+
+    Examples:
+        .. code-block:: python
+            data = fluid.layers.data(name='data', shape=[2, 4, 6], dtype='float32')


Exceed 80 columns.

kuke · 2018-03-22T03:00:36Z

@lcy-seso Thx! I get the point. Please let me know if this pr is ready.

guoshengCS · 2018-03-22T05:38:51Z

Refine by following comments and ready to be reviewed again. Thanks.

… enhance-ReshapeOp

guoshengCS · 2018-03-28T05:37:27Z

This update adds an optional input Shape acting as the target shape at runtime. If Shape is provided, the actual output shape will be this rather than the attribute specifying shape. The rules of copying dimension sizes and unknown dimension inference is same for Input(Shape) and Attr(shape).

kuke · 2018-03-29T07:39:59Z

paddle/fluid/operators/reshape_op.cc

+specified by Attr(shape) is [6, 8], the reshape operator will transform Input(X)
+into a 2-D tensor with shape [6, 8] and leaving Input(X)'s data unchanged.
+
+1. Given a 3-D tensor Input(X) with a shape [2, 4, 6], and the target shape


1 --> 2 --> 3 ... Please also modify the doc the python API

kuke · 2018-03-29T07:41:22Z

paddle/fluid/operators/reshape_op.cc

+1. One and only one dimension in Attr(shape) can be set -1. In this case,
+the actual dimension value will be infered from the total element number of
+Input(X) and remaining dimensions.
+1. More than one dimensions in Attr(shape) can be set to 0, which means the real


1 --> 2 --> 3

kuke · 2018-03-29T07:45:36Z

paddle/fluid/operators/reshape_op.h

+    int64_t capacity = 1;
+    int unk_dim_idx = -1;
+    for (size_t i = 0; i < shape.size(); ++i) {
+      // std::cout<< shape[i] << "haha";


remove this line

kuke

LGTM

lcy-seso and others added 3 commits March 8, 2018 23:12

enhance reshape operator.

d3d16f7

fix bugs.

1d4dfc0

Merge branch 'develop' into enhance_reshape

a8cdd97

lcy-seso requested review from kuke and guoshengCS March 12, 2018 14:36

lcy-seso force-pushed the enhance_reshape branch 7 times, most recently from 4caf848 to 8bfaf07 Compare March 12, 2018 16:52

fix bugs and complete codes.

cf08185

lcy-seso force-pushed the enhance_reshape branch from 8bfaf07 to cf08185 Compare March 12, 2018 16:56

guoshengCS mentioned this pull request Mar 14, 2018

add conv sequence to sequence model PaddlePaddle/models#686

Closed

kuke reviewed Mar 14, 2018

View reviewed changes

Merge branch 'develop' into enhance_reshape

c87d11a

lcy-seso commented Mar 19, 2018

View reviewed changes

lcy-seso force-pushed the enhance_reshape branch 4 times, most recently from 46f2a03 to abae95c Compare March 19, 2018 05:06

follow comments.

a6e6424

lcy-seso force-pushed the enhance_reshape branch from abae95c to a6e6424 Compare March 19, 2018 07:38

guoshengCS added 3 commits March 21, 2018 18:13

Refine reshape_op infershape

eb12cbe

Resolve conflict according to the latest code

437f7a3

Remove the extra call of ValidateShape in ReshapeKernel

454b0a9

pkuyym reviewed Mar 22, 2018

View reviewed changes

Follow comments and refine the python wrapper of reshape_op

d4bb2ca

guoshengCS added 3 commits March 22, 2018 17:55

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

b7e83d2

… enhance-ReshapeOp

Enhance reshape_op by adding Input(Shape)

c078ed4

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

4bfbc59

… enhance-ReshapeOp

guoshengCS mentioned this pull request Mar 28, 2018

InferShape of some operators might fail in CompileTime #8083

Closed

Refine test_reshape_op

09743b6

guoshengCS mentioned this pull request Mar 28, 2018

Decouple the program desc with batch_size in Transformer. PaddlePaddle/models#783

Merged

kuke reviewed Mar 29, 2018

View reviewed changes

Refine reshape_op by following comments.

5b8bb34

kuke approved these changes Mar 29, 2018

View reviewed changes

guoshengCS merged commit d908c3b into PaddlePaddle:develop Apr 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance reshape #9008

Enhance reshape #9008

lcy-seso commented Mar 12, 2018 •

edited

kuke commented Mar 14, 2018

kuke left a comment

kuke Mar 14, 2018

lcy-seso Mar 19, 2018

kuke Mar 14, 2018

lcy-seso Mar 19, 2018

kuke Mar 14, 2018

lcy-seso Mar 19, 2018

kuke Mar 14, 2018

lcy-seso Mar 19, 2018

kuke Mar 14, 2018

lcy-seso Mar 19, 2018

lcy-seso left a comment

lcy-seso Mar 19, 2018

lcy-seso Mar 19, 2018

lcy-seso Mar 19, 2018

lcy-seso Mar 19, 2018

lcy-seso Mar 19, 2018

lcy-seso commented Mar 19, 2018 •

edited

guoshengCS commented Mar 21, 2018

pkuyym Mar 22, 2018

guoshengCS Mar 22, 2018

kuke commented Mar 22, 2018

guoshengCS commented Mar 22, 2018

guoshengCS commented Mar 28, 2018

kuke Mar 29, 2018 •

edited

kuke Mar 29, 2018

kuke Mar 29, 2018

kuke left a comment

Enhance reshape #9008

Enhance reshape #9008

Conversation

lcy-seso commented Mar 12, 2018 • edited

kuke commented Mar 14, 2018

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

lcy-seso 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

lcy-seso commented Mar 19, 2018 • edited

guoshengCS commented Mar 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuke commented Mar 22, 2018

guoshengCS commented Mar 22, 2018

guoshengCS commented Mar 28, 2018

kuke Mar 29, 2018 • edited

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

lcy-seso commented Mar 12, 2018 •

edited

lcy-seso commented Mar 19, 2018 •

edited

kuke Mar 29, 2018 •

edited