Add 3D Convolution operator implemented by GEMM. #4709

chengduoZH · 2017-10-11T08:16:34Z

chengduoZH · 2017-10-11T08:51:31Z

conv

conv2d
- gemm
  - Conv2DOp, Conv2DOpGrad, Conv2DOpMaker, GemmConv2DKernel
- cudnn
  - Conv2DOp, Conv2DOpGrad, CudnnConv2DOpMaker, CudnnConvOp2DKernel
conv3d
- gemm
  - Conv3DOp, Conv3DOpGrad, Conv3DOpMaker, GemmConv3DKernel
- cudnn
  - Conv3DOp, Conv3DOpGrad, CudnnConv3DOpMaker, CudnnConvOp3DKernel

In convolution operations, Conv2DOpGrad and Conv3DOpGrad are completely the same. Conv2DOp and Conv3DOp, Conv2DOpMaker and CudnnConv2DOpMaker, Conv3DOpMaker and CudnnConv3DOpMaker are respectively similar.
I think it is necessary to write a base class or write in one class, respectively.
This problem is also encountered in pooling operations and deconv.

typhoonzero

In convolution operations, Conv2DOpGrad and Conv3DOpGrad are completely the same. Conv2DOp and Conv3DOp, Conv2DOpMaker and CudnnConv2DOpMaker, Conv3DOpMaker and CudnnConv3DOpMaker are respectively similar.
I think it is necessary to write a base class or write in one class, respectively.
This problem is also encountered in pooling operations and deconv.

So are you going to add a base class in this PR or next one?

Need to pull/rebase from develop branch.

typhoonzero · 2017-10-12T03:01:36Z

paddle/operators/conv3d_op.cc

@@ -0,0 +1,117 @@
+/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
+
+Licensed under the Apache License, Version 2.0 (the "License");


这里缩进下，保持和第一行一致。和其他的文件保持一致。

typhoonzero · 2017-10-12T03:03:36Z

paddle/operators/conv3d_op.cc

+Conv3DOpMaker::Conv3DOpMaker(framework::OpProto* proto,
+                             framework::OpAttrChecker* op_checker)
+    : OpProtoAndCheckerMaker(proto, op_checker) {
+  AddInput(


这部分的确和2d很类似，可以用一个base class做一些代码复用。

typhoonzero · 2017-10-12T03:03:47Z

paddle/operators/conv3d_op.cu

@@ -0,0 +1,22 @@
+/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.
+
+Licensed under the Apache License, Version 2.0 (the "License");


License缩进同上

typhoonzero · 2017-10-12T03:08:57Z

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

+
+        output_group_channels = output_channels / self.groups
+        input_group_channels = input_channels / self.groups
+        for batchid in xrange(batch_size):


c++和python代码都实现一次计算感觉比较冗余，python的unitest相当于认定python的conv实现一定是对的，然后来检测c++实现，但我们不一定都能保证python的实现都是对的（比如unitest部分某次commit更新错了，c++也有同样的错误，这样unitest虽然过了但不能保证计算是对的）。

我理解unitest中，需要有使用其他权威计算程序，比如matlab计算一组结果作为验证数据（包含边界条件的数据），同时验证python实现和c++实现，或者只验证c++实现

python的unitest相当于认定python的conv实现一定是对的，然后来检测c++实现，但我们不一定都能保证python的实现都是对的

有道理。我觉得当前应该把python代码的写更直观易懂一些，如果某些包（如 numpy）中已经存在这些操作，最好直接调用。

convolution在sci里的实现貌似只有1d的。放一个matlab算出来的结果作为test case会保险点。个人感觉，比如实现了一个add function，unitest必然是test下1+1=2

c++和python代码都实现一次计算感觉比较冗余，python的unitest相当于认定python的conv实现一定是对的，然后来检测c++实现，但我们不一定都能保证python的实现都是对的（比如unitest部分某次commit更新错了，c++也有同样的错误，这样unitest虽然过了但不能保证计算是对的）。

恰巧看到这个comment。我也遇到类似的问题。CRF 的计算逻辑比较特殊化，单测我先用比较粗暴的策略写了一下。写的时候体会到，效果就是：python 写一遍，C++ 写一遍。到底哪个是对的呢？很可能都是错的。因为会直接照着python的逻辑抄到c++，或者反过来。

后面会再仔细想办法换策略来单测。python 写一遍，c++ 写一遍实在不是个好办法。

卷积跟CRF可能还不完全一样，卷积这个例子，Python中用的是direct方式实现的，C++中一般用的是gemm或其他算法实现的；所以，相当于用一种算法实现检查另一种算法实现，单元测试这么写并没有什么问题。

嗯，卷积这样比较常规的运算情况会好一些。CRF 有些特殊。

后面会再仔细想办法换策略来单测。python 写一遍，c++ 写一遍实在不是个好办法。

这个是说我自己这样测CRF的前向，不是个明智的做法。

可不可以用老paddle的输出做一下对比？不过这样的前提是老paddle的输出得是正确的。

… Add_conv3d_gemm_op

typhoonzero · 2017-10-17T11:46:33Z

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

+
+class TestCase1(TestConv3dOp):
+    def init_test_case(self):
+        # self.groups = 1


Remove these commented lines.

typhoonzero · 2017-10-17T11:49:30Z

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

+            no_grad_set=set(['Input']))
+
+    def init_test_case(self):
+        # self.groups = 1


Remove these commented lines.

typhoonzero · 2017-10-17T11:57:31Z

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

+                    for k in range(sub_out_c):
+                        out[:, g * sub_out_c + k, d, i, j] = \
+                            np.sum(input_pad_masked * f_sub[k, :, :, :, :],
+                                   axis=(1, 2, 3,4))


Add a space between 3, and 4

chengduoZH · 2017-10-26T02:35:49Z

I think we can write conv2d and conv3d together.

typhoonzero

LGTM++

I approve this for now so it won't block following work, we can do the refine later.

typhoonzero · 2017-10-27T02:23:18Z

Need to pull/rebase from develop to resolve the conflict.

typhoonzero

LGTM++

chengduoZH added the OpPorting label Oct 11, 2017

chengduoZH requested review from qingqing01, hedaoyuan and typhoonzero October 11, 2017 08:18

chengduoZH force-pushed the Add_conv3d_gemm_op branch 5 times, most recently from 18f8136 to c51adfa Compare October 11, 2017 10:48

Add conv3d_gemm_op

96b4035

chengduoZH force-pushed the Add_conv3d_gemm_op branch from c51adfa to 96b4035 Compare October 11, 2017 11:32

Add unit test

c2fbf8c

typhoonzero requested changes Oct 12, 2017

View reviewed changes

chengduoZH force-pushed the Add_conv3d_gemm_op branch 3 times, most recently from 588b1b1 to 8c7edb5 Compare October 12, 2017 09:53

fix conv3d_gemm, unit test and follow comments

4aae1ff

chengduoZH force-pushed the Add_conv3d_gemm_op branch 5 times, most recently from c85db7e to 4a556b3 Compare October 12, 2017 15:50

remove conflict

557c7ae

chengduoZH force-pushed the Add_conv3d_gemm_op branch from 4a556b3 to 557c7ae Compare October 13, 2017 08:59

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

24a796f

… Add_conv3d_gemm_op

typhoonzero reviewed Oct 17, 2017

View reviewed changes

follow comments

91db457

chengduoZH force-pushed the Add_conv3d_gemm_op branch 2 times, most recently from 0155eb1 to 4f99b39 Compare October 18, 2017 12:20

chengduoZH force-pushed the Add_conv3d_gemm_op branch from 8f75b45 to 1bf8651 Compare October 26, 2017 02:58

write conv2d and conv3d together

eafbbc1

chengduoZH force-pushed the Add_conv3d_gemm_op branch from 1bf8651 to eafbbc1 Compare October 26, 2017 03:45

typhoonzero previously approved these changes Oct 27, 2017

View reviewed changes

chengduoZH dismissed typhoonzero’s stale review via b15224e October 27, 2017 03:34

chengduoZH force-pushed the Add_conv3d_gemm_op branch 3 times, most recently from 5d57f13 to 39945f7 Compare October 27, 2017 08:52

remove conflict

bf3ae06

chengduoZH force-pushed the Add_conv3d_gemm_op branch from 39945f7 to bf3ae06 Compare October 27, 2017 09:46

Merge branch 'develop' into Add_conv3d_gemm_op

e248866

chengduoZH force-pushed the Add_conv3d_gemm_op branch from c6a75ef to 4bc805b Compare October 30, 2017 07:28

remove conflict

9ee8a0d

chengduoZH force-pushed the Add_conv3d_gemm_op branch 2 times, most recently from e0f8a14 to 3b568c5 Compare October 30, 2017 09:47

fix code format and doc

1724815

chengduoZH force-pushed the Add_conv3d_gemm_op branch from 3b568c5 to 1724815 Compare October 30, 2017 09:49

typhoonzero previously approved these changes Oct 30, 2017

View reviewed changes

chengduoZH added 2 commits November 6, 2017 11:35

fix doc

8ac1178

remove conflict

ba7db29

chengduoZH dismissed typhoonzero’s stale review via ba7db29 November 6, 2017 04:02

write conv2d and conv3d together

f302c6a

chengduoZH force-pushed the Add_conv3d_gemm_op branch from 02871ed to f302c6a Compare November 6, 2017 12:29

qingqing01 changed the title ~~Add conv3d_gemm_op~~ Add 3D Convolution operator implemented by GEMM. Nov 6, 2017

qingqing01 added 2 commits November 6, 2017 21:10

Merge branch 'develop' into Add_conv3d_gemm_op

7ca6c88

Merge branch 'develop' into Add_conv3d_gemm_op

87eb071

qingqing01 approved these changes Nov 6, 2017

View reviewed changes

chengduoZH merged commit f78731c into PaddlePaddle:develop Nov 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 3D Convolution operator implemented by GEMM. #4709

Add 3D Convolution operator implemented by GEMM. #4709

chengduoZH commented Oct 11, 2017 •

edited

Loading

chengduoZH commented Oct 11, 2017 •

edited

Loading

typhoonzero left a comment

typhoonzero Oct 12, 2017

chengduoZH Oct 12, 2017

typhoonzero Oct 12, 2017

typhoonzero Oct 12, 2017

chengduoZH Oct 12, 2017

typhoonzero Oct 12, 2017

chengduoZH Oct 12, 2017 •

edited

Loading

typhoonzero Oct 12, 2017

lcy-seso Oct 12, 2017

hedaoyuan Oct 12, 2017

lcy-seso Oct 12, 2017 •

edited

Loading

chengduoZH Oct 12, 2017

typhoonzero Oct 17, 2017

chengduoZH Oct 18, 2017

typhoonzero Oct 17, 2017

chengduoZH Oct 18, 2017

typhoonzero Oct 17, 2017

chengduoZH Oct 18, 2017

chengduoZH commented Oct 26, 2017

typhoonzero left a comment

typhoonzero commented Oct 27, 2017

typhoonzero left a comment

		@@ -0,0 +1,117 @@
		/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.

		Licensed under the Apache License, Version 2.0 (the "License");

		@@ -0,0 +1,22 @@
		/* Copyright (c) 2016 PaddlePaddle Authors. All Rights Reserve.

		Licensed under the Apache License, Version 2.0 (the "License");

Add 3D Convolution operator implemented by GEMM. #4709

Add 3D Convolution operator implemented by GEMM. #4709

Conversation

chengduoZH commented Oct 11, 2017 • edited Loading

chengduoZH commented Oct 11, 2017 • edited Loading

typhoonzero 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

chengduoZH Oct 12, 2017 • 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

lcy-seso Oct 12, 2017 • 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

chengduoZH commented Oct 26, 2017

typhoonzero left a comment

Choose a reason for hiding this comment

typhoonzero commented Oct 27, 2017

typhoonzero left a comment

Choose a reason for hiding this comment

chengduoZH commented Oct 11, 2017 •

edited

Loading

chengduoZH commented Oct 11, 2017 •

edited

Loading

chengduoZH Oct 12, 2017 •

edited

Loading

lcy-seso Oct 12, 2017 •

edited

Loading