Skip to content

add unfold op (new op)#17944

Merged
SunGaofeng merged 4 commits intoPaddlePaddle:developfrom
SunGaofeng:unfold
Jun 11, 2019
Merged

add unfold op (new op)#17944
SunGaofeng merged 4 commits intoPaddlePaddle:developfrom
SunGaofeng:unfold

Conversation

@SunGaofeng
Copy link
Contributor

@SunGaofeng SunGaofeng commented Jun 10, 2019

This is for unfold op (new op), which is also called im2col for batched 2D image tensor. Details are in the following description:

8e3d7be761c6d65dd63bce46c0f98d04

test=develop
@SunGaofeng SunGaofeng changed the title add unfold op,test=develop add unfold op (new op),test=develop Jun 10, 2019
This Operator is used to extract sliding local blocks from a batched input tensor, also known
as im2col when operated on batched 2D image tensor. For each block under the convolution filter,
all element will be rearranged as a column. While the convolution filter silding over the input
feature map, a series of such columns will be formed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

give same examples or formula

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the formula is given in the python api

strides.size(), kernel_sizes.size(),
"The dims of strides shold be the same with that of kernel_sizes. "
"But recieved dims(strides: %u) != dims(kernel_sizes: %u).",
strides.size(), dilations.size());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strides.size(), dilations.size() -> strides.size(), kernel_sizes.size()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

auto& dev_ctx = ctx.template device_context<DeviceContext>();

math::SetConstant<DeviceContext, T> set_zero;
set_zero(dev_ctx, input_grad, static_cast<T>(0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

下面for循环里input_grad会有点赋不到值么,如果没有的话这里应该可以不用初始化吧

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在Functor col2im里面,计算input_grad的时候会用的是累加,所以需要先把input_grad清零。

std::unique_ptr<framework::OpDesc> op(new framework::OpDesc());
op->SetType("unfold_grad");
op->SetInput(framework::GradVarName("Y"), OutputGrad("Y"));
op->SetInput("X", Input("X"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Input(X)是不是不需要

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要input(X)的dims,然后加上UnfoldGradOpNoNeedBufferVarsInference声明不需要input(X)的数据,这样在计算grad时只会保留input(X)的dims而不用保留数据。

wanghaoshuang
wanghaoshuang previously approved these changes Jun 10, 2019
Copy link
Contributor

@wanghaoshuang wanghaoshuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

提了一些小建议,可以根据时间情况和其它review建议,选择是否在当前PR修改。

@@ -0,0 +1,102 @@
# Copyright (c) 2018 PaddlePaddle Authors. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2018->2019

This is for test on unfold Op
"""

def init_data(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可以多补充些测试用例。

dkernel_h = self.dilations[0] * (self.kernel_sizes[0] - 1) + 1
dkernel_w = self.dilations[1] * (self.kernel_sizes[1] - 1) + 1
out_height = (self.input_height + self.paddings[0] + self.paddings[2] -
dkernel_h) / self.strides[0] + 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里确认python3环境下没问题么?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已改正,加上int类型转化

heavengate
heavengate previously approved these changes Jun 10, 2019
Copy link
Contributor

@heavengate heavengate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SunGaofeng SunGaofeng dismissed stale reviews from heavengate and wanghaoshuang via e658cb6 June 10, 2019 11:55
wanghaoshuang
wanghaoshuang previously approved these changes Jun 10, 2019
xsrobin
xsrobin previously approved these changes Jun 10, 2019
Copy link
Contributor

@xsrobin xsrobin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


def unfold(x, kernel_sizes, strides=1, paddings=0, dilations=1):
"""
**unfold**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

return output


def unfold(x, kernel_sizes, strides=1, paddings=0, dilations=1):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add name=None

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


int dkernel_h = dilations[0] * (kernel_sizes[0] - 1) + 1;
int dkernel_w = dilations[1] * (kernel_sizes[1] - 1) + 1;
int conv_out_height =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conv_out_height -> out_height remove prefix conv_

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

1;
int output_width =
(input_dims[3] + paddings[1] + paddings[3] - dkernel_w) / strides[1] +
1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些代码重复了3次,conv里有抽取一些公用的函数吧

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

另外写了一个inline fucntion, 没有用conv_op.h里面的计算conv output 尺寸的函数,那里面默认为padding左右是一样的,在unfold里面padding可以设置成上下左右四个值都不一样的。

@SunGaofeng SunGaofeng dismissed stale reviews from xsrobin and wanghaoshuang via e37c46f June 10, 2019 16:54
Copy link
Contributor

@xsrobin xsrobin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SunGaofeng SunGaofeng merged commit 40885c2 into PaddlePaddle:develop Jun 11, 2019
SunGaofeng added a commit to SunGaofeng/Paddle that referenced this pull request Jun 11, 2019
* add unfold op
test=develop

* fix divide bug in python3 when calculating output width and height
test=develop

* add name=None in python api, move redundant code into inline function

* try to trigger ci for this code
test=develop
@SunGaofeng SunGaofeng changed the title add unfold op (new op),test=develop add unfold op (new op) Jun 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants