padding support for dnn_conv

[jia-kai]

cudnn supports padding and I exported this functionality by adding a new param padding to dnn_conv; to enable padding, border_mode must be set to 'padding'.

[f0k]

+1 for adding this functionality, but the interface should probably be different. In #2118, we more or less agreed on using border_mode for padding as well, such that border_mode can be either a string ("valid", "full" or "half"), an integer or an integer tuple. GpuCorrMM currently still has a different interface with a pad parameter. Adding a padding parameter to the DNN-based convolution would be inconsistent with both.

I'd suggest to use the border_mode parameter for custom padding, to make it easy to subsequently add the same interface to ConvOp, GpuConv and GpuCorrMM.

[f0k]

The docstring for padding is missing. But I'd suggest to merge the parameter with border_mode anyway, and add the corresponding documentation to it.

[jia-kai]

@f0k Thanks for your suggestion :) I added a new commit jia-kai@291f8c1 . Should I issue another pull request ?

[nouiz]

When you push to your repo on github a new commit to a branch that you used for a PR, the PR is updated.

Told otherwise, the PR is a branch and the most up to date version of that branch, not a commit. So no need for a new PR.

thanks

[nouiz]

The PR seem good, but it need tests. Can you add them in the file sandbox/cuda/tests/test_conv_cuda_ndarray.py?

[jia-kai]

@nouiz Thanks for your information:) I've added the test cases. BTW, all test_gemm tests fail with a message like the following, which I guess is due to that cuDNN is preferred over gemm:

Cannot find class <class 'theano.sandbox.cuda.blas.BaseGpuCorrMM'> in [GpuContiguous(<CudaNdarrayType(float32, 4D)>), GpuContiguous(<CudaNdarrayType(float 32, 4D)>), GpuContiguous(GpuContiguous.0), GpuContiguous(GpuContiguous.0), Shape(GpuContiguous.0), Shape(GpuContiguous.0), GpuDnnConvDesc{border_mode='full', subsample=(2, 2), conv_mode='conv'}(Shape.0, Shape.0), GpuDnnConv(GpuContiguous.0, GpuContiguous.0, GpuDnnConvDesc{border_mode='full', subsample=(2, 2), conv_mode='conv'}.0)]

[nouiz]

For the error, thanks for tell it. To prevent conflict, can you add the fix to your PR?

diff --git a/theano/sandbox/cuda/tests/test_conv_cuda_ndarray.py b/theano/sandbox/cuda/tests/test_conv_cuda_ndarray.py
index e582864..7dd6287 100644
--- a/theano/sandbox/cuda/tests/test_conv_cuda_ndarray.py
+++ b/theano/sandbox/cuda/tests/test_conv_cuda_ndarray.py
@@ -574,7 +574,7 @@ def test_gemm_valid():
     extra_shapes += get_shapes2(scales_kern=(2, 2), kern_stride=(2, 2))

     for t in _test_valid(cuda.blas.BaseGpuCorrMM,
-                         mode=theano_mode.including("conv_gemm"),
+                         mode=theano_mode.excluding("cudnn"),
                          extra_shapes=extra_shapes):
         yield t

@@ -683,7 +683,7 @@ def test_full():

 def test_gemm_full():
     for t in _test_full(cuda.blas.BaseGpuCorrMM,
-                        mode=theano_mode.including("conv_gemm")):
+                        mode=theano_mode.excluding("cudnn")):
         yield t


@@ -735,7 +735,7 @@ def test_subsample():

 def test_gemm_subsample():
     for t in _test_subsample(cuda.blas.BaseGpuCorrMM,
-                             theano_mode.including("conv_gemm")):
+                             theano_mode.excluding("cudnn")):
         yield t

[jia-kai]

@nouiz I added tests for GpuCorrMM. There are still 3 failed tests:

2 of them seem to be caused by precision loss:

Max Abs Diff:  1.52344
Mean Abs Diff:  0.273218
Median Abs Diff:  0.226562
Std Abs Diff:  0.206641
Max Rel Diff:  7.05835e-06
Mean Rel Diff:  1.265e-06
Median Rel Diff:  1.05107e-06
Std Rel Diff:  9.56672e-07

rtol, atol: 1.1e-05 1e-08

the other is test_default_conv, it still uses cuDNN, although 'local_gpu_conv' and 'local_conv_gemm' have been excluded; I've also checked my theanorc and env to make sure cudnn is not enabled explicitly:

assert any([isinstance(a.op, cuda.blas.GpuConv) for a in nodes]), nodes
AssertionError: set([GpuDnnConv(GpuContiguous.0, GpuContiguous.0, GpuDnnConvDesc{border_mode='valid', subsample=(1, 1), conv_mode='conv'}.0), Shape(GpuContiguous.0), GpuContiguous(GpuFromHost.0), Shape(GpuContiguous.0), GpuFromHost(<TensorType(float32, 4D)>), GpuContiguous(GpuContiguous.0), GpuContiguous(GpuFromHost.0), GpuFromHost(<TensorType(float32, 4D)>), GpuDnnConvDesc{border_mode='valid', subsample=(1, 1), conv_mode='conv'}(Shape.0, Shape.0), HostFromGpu(GpuDnnConv.0), GpuContiguous(GpuContiguous.0)])

[f0k]

Max Abs Diff: 1.52344

This seems like a lot. But judging from the relative differences, the numbers are just very large. Which test is it? And shouldn't it run through because the relative differences are all smaller than the rtol of 1.1e-05?

the other is test_default_conv, it still uses cuDNN, although 'local_gpu_conv' and 'local_conv_gemm' have been excluded

Since a few weeks, cuDNN is not opt-in anymore, but opt-out. For test_default_conv, you will need to use mode.excluding('conv_dnn', 'conv_gemm') (after #2255 is merged, otherwise you need to exclude 'cudnn', 'local_conv_gemm').

By the way, thanks for fixing the tests!

[nouiz]

thanks.

[nouiz]

I'm running the tests here. The example you gave seem just that the test is
too strict.

On Tue, Nov 18, 2014 at 4:23 AM, Jan Schlüter notifications@github.com
wrote:

the other is test_default_conv, it still uses cuDNN, although
'local_gpu_conv' and 'local_conv_gemm' have been excluded

Since a few weeks, cuDNN is not opt-in anymore, but opt-out. For
test_default_conv, you will need to use mode.excluding('conv_dnn',
'conv_gemm') (after #2255 #2255 is
merged, otherwise you need to exclude 'cudnn', 'local_conv_gemm').

—
Reply to this email directly or view it on GitHub
#2259 (comment).