Add convolution Function #2282

hedaoyuan · 2017-05-26T06:34:26Z

Follow comments in issue #2196

Defines the data structure of the input, output, and filter.
Migrate the code based on sgemm to the GemmConvOp.cpp.
Refactor the ExpandConvBaseLayer, ExpandConvLayer and ExpandConvTransLayer.

Code refactoring:

number	old code	new code	about
1	ExpandConvTransLayer, ExpandConvLayer	ExpandConvLayer	Remove ExpandConvTransLayer, both exconv and exconvt are based on ExpandConvLayer.
2	ExpandConvBaseLayer	ConvFunctionBase
3	ExpandConvBaseLayer::expandOneFrame, Matrix::convExpand	Im2ColFunctor	Refactor code.
4	Matrix::convShrink	Col2ImFunctor	Refactor code.
5	ExpandConvBaseLayer::expandFwdOnce	GemmConvFunction	Refactor code.
6	ExpandConvBaseLayer::bpropActs	GemmConvGradInputFunction	Refactor code.
7	ExpandConvBaseLayer::bpropWeights	GemmConvGradFilterFunction	Refactor code.
8		NaiveConvFunction	Add a naive convolution calculation.
9	FunctionCompare	Compare2Function, CpuGpuFuncCompare	Replace FunctionCompare with the Compare2Function type. The original FunctionCompare is actually a CpuGpuFuncCompare.
10		ConvolutionTest	Added unit tests for the implementation of various convolution calculations.

To do:

Remove and Matrix::convExpand and Matrix::convShrink and Refactor BlockExpandLayer Remove and Matrix::convExpand and Matrix::convShrink and Refactor BlockExpandLayer #2424
Output size calculation Whether can remove caffeMode. #2460 .
Migrate the code based on cudnn to the CudnnConvOp.cpp. Refactor CudnnConvLayer, ConvProjection, ConvTransProjection #2425

…wo CPU functions.

…inition of backward input and backward filter function.

…GemmConvFunction.

qingqing01

还没看完，后续继续看 :)

qingqing01 · 2017-06-06T11:56:19Z

paddle/function/ConvOpTest.cpp

+  FORWARD_TEST = 0,
+  BACKWARD_INPUT_TEST = 1,
+  BACKWARD_FILTER_TEST = 2,
+};


命名约定: https://github.com/PaddlePaddle/cpp-primer-digest/pull/1/files#diff-04c6e90faac2675aa89e2176d2eec7d8R264

qingqing01 · 2017-06-06T11:58:58Z

paddle/function/ConvOpTest.cpp

+                  if (padding >= filterSize) break;
+                  size_t outputSize =
+                      (inputSize - filterSize + 2 * padding + stride) / stride;
+                  LOG(INFO) << " batchSize=" << batchSize


LOG(INFO) -> VLOG

qingqing01 · 2017-06-06T12:00:17Z

paddle/function/ConvOpTest.cpp

+      for (size_t inputSize : {7, 14, 54}) {
+        for (size_t filterSize : {1, 3, 5}) {
+          for (size_t inputChannels : {3, 64}) {
+            for (size_t outputChannels : {3, 64, 128}) {


是否需要增加长方形input,output, filter的单测？

在这里加会让TEST变的时间很长，后续我单独增加针对长方形的测试吧。

qingqing01 · 2017-06-06T12:04:03Z

paddle/function/GemmConvOp.cpp

+                  int paddingWidth,
+                  int outputHeight,
+                  int outputWidth,
+                  T* colData) {


感觉接口直接传TensorShape会简单一些吧，就不用这么多参数了~

修改成TensorShape的话，接口里面还需要做一次input/filter shape对应参数的检查。而且，这里没有batchSize这个维度，还不能直接用Function里面传入的参数的shape。

Will be fixed in the next PR 2449.

class Im2ColFunctor { public: void operator()(float* colData, const float* imData, const TensorShape& colShape, const TensorShape& imShape, int strideHeight, int strideWidth, int paddingHeight, int paddingWidth); };

…ution

qingqing01 · 2017-06-16T12:35:33Z

paddle/function/ConvOp.h

+ *      image channels, C is the number of input image channels,
+ *      H and W is height and width of filter.
+ *
+ *      If groups is greater than 1, the filter's data format should be GMCHW,


groups -> groups_

这个注释里面我觉得不用修改成groups_，FuncConfig的配置参数名称上也是groups。

上面提出修改原因是：If groups is greater than 1 不符合语法，也提醒我以后comments需要解释清楚 :)

qingqing01 · 2017-06-16T12:37:38Z

paddle/function/ConvOp.h

+
+protected:
+  size_t getFilterHeight(const TensorShape& filter) const {
+    if (filter.ndims() == 5) {


qingqing01 · 2017-06-16T12:37:39Z

paddle/function/ConvOp.h

+
+protected:
+  size_t getFilterHeight(const TensorShape& filter) const {
+    if (filter.ndims() == 5) {


qingqing01 · 2017-06-16T12:37:48Z

paddle/function/ConvOp.h

+  }
+
+  size_t getFilterWidth(const TensorShape& filter) const {
+    if (filter.ndims() == 5) {


qingqing01 · 2017-06-16T12:39:19Z

paddle/function/ConvOp.h

+    if (filter.ndims() == 5) {
+      return filter[4];
+    } else {
+      return filter[3];


可以加个类似filter[-1], filter[-2] 这样的接口？

没有理解这个comment，filter[-1],filter[-2]指的是啥？

-1, -2指的是倒数第一个第二个值这样？

qingqing01 · 2017-06-16T12:50:07Z

paddle/function/FunctionTest.h


-  std::shared_ptr<FunctionBase> getGpuFunction() const { return gpuFunc_; }
+  std::shared_ptr<FunctionBase> getGpuFunction() const { return function2_; }


从上面看function1_不一定是CPU Type, function2_不一定是GPU Type吧

嗯，这里接口名称需要修改。

qingqing01 · 2017-06-16T12:52:12Z

paddle/function/GemmConvOp.cpp

+ *            output_height, output_width]
+ */
+template <class T>
+class Im2ColFunctor<DEVICE_TYPE_CPU, T> {


Im2Col, Col2Im是不是可以单独一个Function？可以用在其他Function里，比如BlockExpand里。

在PR #2424 里面会移到一个单独的文件里面，但不是Function，被ImageExpandFunction调用。

qingqing01 · 2017-06-16T12:57:46Z

paddle/function/GemmConvOp.cpp

+    size_t filterWidth = getFilterWidth(filter);
+    size_t outputChannels = output[1];
+    size_t outputHeight = output[2];
+    size_t outputWidth = output[3];


n, ci, hi, wi, kh, kw, co, ho,wo感觉命名更短些，当然现在这样容易理解 :)

个人觉得Function的代码逻辑可读性比较重要。如果用n, ci, hi...这种缩写的话，对于不熟悉代码逻辑的人并不一定很友好。

qingqing01 · 2017-06-16T13:07:17Z

paddle/gserver/layers/ExpandConvLayer.cpp

+    std::vector<size_t> paddings = {(size_t)paddingY_[i], (size_t)padding_[i]};
+    std::vector<size_t> strides = {(size_t)strideY_[i], (size_t)stride_[i]};
+    createFunction(forward_,
+                   !isDeconv_ ? "GemmConv" : "GemmConvGradInput",


isDeconv_ -> isConvTrans_

isDeconv_是ConvBaseLayer里面原来的名字，换成isConvTrans_更好？

嗯，命名和对用户接口一致叫做convolution transpose layer觉得更好些，这样也可以吧~

qingqing01 · 2017-06-16T13:09:12Z

paddle/gserver/layers/ExpandConvLayer.cpp

+    inputs.addArg(*weights_[i]->getW(), filterShape_[i]);
+    outputs.addArg(*getOutputValue(),
+                   outputShape_[i],
+                   !isDeconv_ && i == 0 ? ASSIGN_TO : ADD_TO);


isDeconv_为什么需要这个参数？

主要是目前GemmConvGradInput的实现只支持ADD_TO模式。
https://github.com/PaddlePaddle/Paddle/pull/2282/files/2608c4854273e47bd0958fbb03ca67050ddfb35c#diff-b71c34e8a73d71627e2b185b4dd33aafR215

明白了。感觉Function是不是应该和blas库类似，加scale/beta这样的系数更合理些，而不是ASSIGN_TO、ADD_TO~

qingqing01

LGTM. 清爽了很多~

qingqing01 · 2017-06-17T10:01:26Z

paddle/function/ConvOp.h

+ *      image channels, C is the number of input image channels,
+ *      H and W is height and width of filter.
+ *
+ *      If groups is greater than 1, the filter's data format should be GMCHW,


上面提出修改原因是：If groups is greater than 1 不符合语法，也提醒我以后comments需要解释清楚 :)

qingqing01 · 2017-06-17T10:03:26Z

paddle/function/GemmConvOpGpu.cu

+          int c_col = int(c * blockH* blockW) + \
+            (h - h_col * (int)strideH) * (int)blockW +
+            (w - w_col * (int)strideW);
+          val += data_col[(c_col * height_col + h_col) * width_col + w_col];


cuda kernel里有多种命名规则~

qingqing01 · 2017-06-17T10:06:36Z

paddle/gserver/layers/ExpandConvLayer.cpp

+    inputs.addArg(*weights_[i]->getW(), filterShape_[i]);
+    outputs.addArg(*getOutputValue(),
+                   outputShape_[i],
+                   !isDeconv_ && i == 0 ? ASSIGN_TO : ADD_TO);


明白了。感觉Function是不是应该和blas库类似，加scale/beta这样的系数更合理些，而不是ASSIGN_TO、ADD_TO~

qingqing01 · 2017-06-17T10:09:35Z

paddle/gserver/layers/ExpandConvLayer.cpp

+    std::vector<size_t> paddings = {(size_t)paddingY_[i], (size_t)padding_[i]};
+    std::vector<size_t> strides = {(size_t)strideY_[i], (size_t)stride_[i]};
+    createFunction(forward_,
+                   !isDeconv_ ? "GemmConv" : "GemmConvGradInput",


嗯，命名和对用户接口一致叫做convolution transpose layer觉得更好些，这样也可以吧~

hedaoyuan

目前来看只有ASSIGN_TO和ADD_TO两种赋值模式。修改成一个scale会增加Function的复杂度，scale需要是Function的参数，ASSIGN_TO和ADD_TO可以是BufferArg的参数；另外，结果是覆盖掉Arg中原来的值还是需要add上去，这个也是BufferArg自身的属性。

hedaoyuan added 6 commits May 26, 2017 14:17

Add a naive convolution implement

3b65bc7

Bug fix

b6de52c

Add a convolution Function based on gemm.

1846d9e

Modify FunctionCompare to Compare2Function to support comparison of t…

1879332

…wo CPU functions.

Add ConvOpTest for NaiveConv and GemmConv

455888c

Change stride to strides, and change padding to paddings.

048b14a

hedaoyuan requested a review from qingqing01 May 27, 2017 09:38

hedaoyuan added 10 commits May 31, 2017 13:01

Add group argument in ConvFunctionBase

3ce974b

Add GPU GemmConvFunction implementation

3c0aa0c

Some bug fix

c70d3e1

Bug fix

3408b4b

Modify the arguments description of ConvFunctionBase. And add the def…

afbe556

…inition of backward input and backward filter function.

Add the calculation implementation of GemmConvGradFilterFunction

6a93f0f

Bug fix & add test of GemmConvGradFilter.

9032619

Add the calculation implementation of GemmConvGradInputFunction.

d99faf3

format

9885c57

Refactoring the code implementation of exconv adn exconvt layer with …

7aac38c

…GemmConvFunction.

qingqing01 reviewed Jun 6, 2017

View reviewed changes

hedaoyuan added 3 commits June 7, 2017 17:56

Fix the error of group convolution.

784e218

follow comments

95a7bc0

Remove the code of ExpandConvTransLayer.

e039410

hedaoyuan mentioned this pull request Jun 12, 2017

Add ImageExpandFunction. #2449

Merged

hedaoyuan added 3 commits June 13, 2017 10:53

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

1e0cc74

…ution

Fix RowConvOpTest use CpuGpuFuncCompare.

01d52eb

Add test cases where the height and width (input, filter) are not equal.

2608c48

qingqing01 reviewed Jun 16, 2017

View reviewed changes

hedaoyuan added 2 commits June 16, 2017 23:06

Follow comments.

c6e010d

Bug fix.

1ed31b4

qingqing01 approved these changes Jun 17, 2017

View reviewed changes

hedaoyuan commented Jun 19, 2017

View reviewed changes

Change the groups in the comment to 1049089.

9c47c42

hedaoyuan merged commit 17fe832 into PaddlePaddle:develop Jun 19, 2017

hedaoyuan mentioned this pull request Sep 15, 2017

Refine ExpandConvLayer.cpp #4128

Closed


		std::shared_ptr<FunctionBase> getGpuFunction() const { return gpuFunc_; }
		std::shared_ptr<FunctionBase> getGpuFunction() const { return function2_; }

Add convolution Function #2282

Add convolution Function #2282

Conversation

hedaoyuan commented May 26, 2017 • edited Loading

qingqing01 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

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

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

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

hedaoyuan left a comment

Choose a reason for hiding this comment

hedaoyuan commented May 26, 2017 •

edited

Loading