Add im2sequence op. #4866

gongweibao · 2017-10-17T12:44:36Z

… blockexpand

qingqing01 · 2017-10-20T06:16:48Z

paddle/operators/block_expand_op.cc

+                   "Output of BlockExpandOp op should not be null.");
+
+    auto in_dim = ctx->GetInputDim("X");
+    PADDLE_ENFORCE_EQ(in_dim.size(), 4, "Input format  must be NCHW.");


Input(x) must be 4D tensor.

qingqing01 · 2017-10-20T06:24:14Z

paddle/operators/block_expand_op.h

+                                         int block_height, int block_width,
+                                         int stride_height, int stride_width,
+                                         int padding_height, int padding_width,
+                                         int& outputHeight, int& outputWidth) {


这里觉得类似conv , pool的写法，一组参数(不用height, width都在这里计算)就好了，用的地方调用多次就好了。另外，函数命名： "CamelCase": https://google.github.io/styleguide/cppguide.html#Function_Names

Thx. Fixed.

qingqing01 · 2017-10-20T06:26:33Z

paddle/operators/block_expand_op.cc

+
+    auto in_dim = ctx->GetInputDim("X");
+    PADDLE_ENFORCE_EQ(in_dim.size(), 4, "Input format  must be NCHW.");
+    PADDLE_ENFORCE_GE(in_dim[0], 1, "Input batchsize must >= 1.");


It's better not to do this check for batch size. Maybe the batch size is a special value in the compile time.

没太看懂。in_dim[0]这一维可以忽略？或者==0?

Have removed this checker.

qingqing01 · 2017-10-20T06:30:09Z

paddle/operators/block_expand_op.cc

+    // output_height * output_width, stepSize is equal
+    // input_channels * blockHeight * blockWidth
+    ctx->SetOutputDim(
+        "Out", {N, output_height, output_width, C, block_height, block_width});


The output shape is not correct.

Height = N * output_height * output_width,
Width = C * block_height * block_width

Please to look the original implementation carefully and confirm it.

Thx. Fixed.

qingqing01 · 2017-10-20T06:31:11Z

paddle/operators/block_expand_op.cc

+    ctx->SetOutputDim(
+        "Out", {N, output_height, output_width, C, block_height, block_width});
+
+    // ctx->ShareLoD("X", /*->*/ "Out");


Need to set LoD in the InferShape. And the input is tenor without lod and the output is LoDTensor with lod.

暂时先在run time计算并设置了LoD，并增加TODO注释：待InferShape 开放 set_lod接口后，将计算LoD逻辑转移到InferShape

qingqing01 · 2017-10-20T06:35:06Z

paddle/operators/block_expand_op.cc

+    1 + (2 * paddingWidth + img_width - blockWidth + strideWidth - 1) /
+            strideWidth;
+
+The expand method is the same with ExpandConvLayer, but saved the transposed


There is no ExpandConvLayer in the new framework. And the original comments in the BlockExpandLayer is not good. This comment is for users. Actually, the users do not familiar with the implementation details in code. Please help to improve the doc. And maybe the simple examples are also to help to explain clearly.

Add an exmple.

qingqing01 · 2017-10-20T06:42:44Z

paddle/operators/block_expand_op.h

+  void Compute(const framework::ExecutionContext& ctx) const override {
+    using namespace framework;
+    const Tensor* in = ctx.Input<Tensor>("X");
+    Tensor* out = ctx.Output<Tensor>("Out");


老框架这个层有两个主要的特点：1. 对输入按照Sliding Window展开 2. 给输出填充LoD.

因为输入是Tensor，输出LoD是定长序列，按照现在这样实现，这个Op后面还需要接一个设置LoD的Op。不然，需要在这个Op里设置输出的LoD。

Thx. Fixed.

qingqing01 · 2017-10-20T07:23:24Z

paddle/operators/block_expand_op.h

+
+    for (int i = 0; i < N; i++) {
+      Tensor src = in->Slice<T>(i, i + 1).Resize({C, img_height, img_width});
+      Tensor dst = out->Slice<T>(i, i + 1).Resize(


Update code, since Slice has removed template T.

qingqing01 · 2017-10-20T07:48:20Z

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

+            'strideWidth': 1,
+            'paddingHeight': 1,
+            'paddingWidth': 1,
+        }


需要测试更多的case， stride>1, blockHeight, blockWidth不相等的情况。

增加了batch_size>1的case, 并重构了单测代码，方便增加测试用例。

qingqing01 · 2017-10-20T07:55:24Z

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

+    strideHeight = attrs['strideHeight']
+    strideWidth = attrs['strideWidth']
+    paddingHeight = attrs['paddingHeight']
+    paddingWidth = attrs['paddingWidth']


这里Python的命名混合了多种，比如： input_channels， inputHeight，觉得修改一致吧。

… blockexpand

1. Add lod to output 2. Fix im2col arguments list 3. Refine code and doc 4. Fix output shape

… blockexpand

pkuyym · 2018-01-17T03:23:39Z

paddle/operators/block_expand_op.cc

+ protected:
+  void InferShape(framework::InferShapeContext* ctx) const override {
+    PADDLE_ENFORCE(ctx->HasInput("X"),
+                   "Input of BlockExpandOp should not be null.");


Input --> Input(X)

pkuyym · 2018-01-17T03:23:49Z

paddle/operators/block_expand_op.cc

+    PADDLE_ENFORCE(ctx->HasInput("X"),
+                   "Input of BlockExpandOp should not be null.");
+    PADDLE_ENFORCE(ctx->HasOutput("Out"),
+                   "Output of BlockExpandOp op should not be null.");


Output --> Output(Out)

pkuyym · 2018-01-17T03:24:58Z

paddle/operators/block_expand_op.cc

+    int padding_height = ctx->Attrs().Get<int>("padding_height");
+    int padding_width = ctx->Attrs().Get<int>("padding_width");
+
+    int batch_size = in_dim[0];


Need to confirm the layout is NCHW.

It seems that the layout of input tensor is not available in InferShape context. ~~So I add layout checker into the runtime.~~ I add a TODO comments to remind me adding layout checker after 'layout' being available in framework.proto .

pkuyym · 2018-01-17T07:59:51Z

paddle/operators/block_expand_op.cc

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


We can discuss the name of this operator. It seems that BlockExpandOp isn't self-explain well. I think we need a better name here such as flatten_block_conv_way_op

Rename 'block_expand' to im2sequence. Because this op is actually a wrapper of im2col functor.

pkuyym · 2018-01-17T08:06:49Z

paddle/operators/block_expand_op.h

+    // TODO(wanghaoshuang): Move this to InferShape
+    framework::LoD lod(1);
+    for (int i = 0, offset = 0; i < batch_size + 1; ++i) {
+      lod[0].push_back(offset);


Please reserve memory for lod[0] first.

Thx. Fixed.

pkuyym · 2018-01-17T08:08:46Z

python/paddle/v2/fluid/tests/test_block_expand_op.py

+
+def get_output_shape(attrs, x):
+    img_height = x.shape[2]
+    img_width = x.shape[3]


I think it's better to pass x.shape instead x.

Thx. Fixed.

2. Refine code and doc

qingqing01 · 2018-01-22T03:24:52Z

paddle/operators/im2sequence_op.cc

+    AddAttr<int>("stride_height", "(int)height of stride.");
+    AddAttr<int>("stride_width", "(int)width of stride.");
+    AddAttr<int>("padding_height", "(int)height of padding.");
+    AddAttr<int>("padding_width", "(int)width of padding.");


The attributes are too much. Please refer the attribues in https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/operators/conv_op.cc#L112

Use std::vector<int> instead of int .

AddAttr<std::vector<int>>("kernels", ...) // or blocks? AddAttr<std::vector<int>>("strides", ...) AddAttr<std::vector<int>>("paddings", ...)

qingqing01 · 2018-01-22T03:26:10Z

paddle/operators/im2sequence_op.h

+inline int get_output_size(int img_size, int block_size, int stride,
+                           int padding) {
+  return (1 + (img_size + 2 * padding - block_size + stride - 1) / stride);
+}


get_output_size -> OutputSize, make the name same with https://github.com/PaddlePaddle/Paddle/blob/develop/paddle/operators/conv_op.h#L30

OutputSize seems not a good name for a function. GetOutputSize may be a better one.

pkuyym · 2018-01-22T03:37:33Z

paddle/operators/im2sequence_op.cc

+
+    auto in_dim = ctx->GetInputDim("X");
+    PADDLE_ENFORCE_EQ(in_dim.size(), 4,
+                      "Input(X) format  must be 4D tensor, eg., NCHW.");


Remove redundant whitespace char.

pkuyym · 2018-01-22T03:39:19Z

paddle/operators/im2sequence_op.h

+inline int get_output_size(int img_size, int block_size, int stride,
+                           int padding) {
+  return (1 + (img_size + 2 * padding - block_size + stride - 1) / stride);
+}


OutputSize seems not a good name for a function. GetOutputSize may be a better one.

2. Rename 'get_output_size' to 'OutputSize' 3. Remove redundant whitespace char.

… blockexpand

qingqing01

Approved. Im2SequenceOp以后想出更合理在更正~

… blockexpand

gongweibao added 10 commits October 11, 2017 12:39

add block_expand_op

48556ba

add expand comment

d2fda53

add block forward

f1ca3f7

modify styles

6197c09

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

d5a3745

… blockexpand

add gpu

5a9dd8a

add py test

45f16c9

fix bugs

32db8db

fix bugs

d3ac339

rm not need

4422a55

gongweibao requested review from qingqing01, hedaoyuan, Xreki and luotao1 October 17, 2017 12:47

qingqing01 added the OpPorting label Oct 18, 2017

qingqing01 reviewed Oct 20, 2017

View reviewed changes

gongweibao added 4 commits November 21, 2017 08:34

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

4838a57

… blockexpand

mv test position to fluid

dbe0583

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

e11d442

… blockexpand

fix by comments

25a3d2d

qingqing01 mentioned this pull request Jan 5, 2018

The image recognition and detection model on Fluid. #7253

Closed

wanghaoshuang self-assigned this Jan 16, 2018

wanghaoshuang added 4 commits January 17, 2018 00:42

Finish block expand op

e82f100

1. Add lod to output 2. Fix im2col arguments list 3. Refine code and doc 4. Fix output shape

Fix code style

92baa88

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

bfe7e24

… blockexpand

Fix code style

09adb76

wanghaoshuang removed request for luotao1 and Xreki January 17, 2018 03:18

wanghaoshuang requested review from pkuyym and removed request for hedaoyuan January 17, 2018 03:18

wanghaoshuang mentioned this pull request Jan 17, 2018

Add python API for im2sequence op #7595

Merged

pkuyym reviewed Jan 17, 2018

View reviewed changes

wanghaoshuang changed the title ~~Block expand op.~~ Add im2sequence op. Jan 17, 2018

1. Rename 'block_expand' to im2sequence

fe45f21

2. Refine code and doc

qingqing01 reviewed Jan 22, 2018

View reviewed changes

pkuyym reviewed Jan 22, 2018

View reviewed changes

wanghaoshuang added 5 commits January 22, 2018 13:11

1. Reduce attributes

500e29a

2. Rename 'get_output_size' to 'OutputSize' 3. Remove redundant whitespace char.

Fix unitest

3a48282

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

648ca7a

… blockexpand

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

da0d95c

… blockexpand

Fix white space in comments.

c9e208c

qingqing01 approved these changes Jan 22, 2018

View reviewed changes

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

09544bc

… blockexpand

wanghaoshuang merged commit 6d2cfe9 into PaddlePaddle:develop Jan 23, 2018

gongweibao deleted the blockexpand branch January 17, 2021 07:41

Add im2sequence op. #4866

Add im2sequence op. #4866

Conversation

gongweibao commented Oct 17, 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

wanghaoshuang Jan 16, 2018 • 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

wanghaoshuang Jan 16, 2018 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wanghaoshuang Jan 17, 2018 • edited Loading

Choose a reason for hiding this comment

pkuyym Jan 17, 2018 • 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

qingqing01 left a comment

Choose a reason for hiding this comment

wanghaoshuang Jan 16, 2018 •

edited

Loading

wanghaoshuang Jan 16, 2018 •

edited

Loading

wanghaoshuang Jan 17, 2018 •

edited

Loading

pkuyym Jan 17, 2018 •

edited

Loading