support max pooling with padding #2543

yaoli · 2015-02-27T21:18:44Z

the grad is not right now...working on it

lamblin · 2015-02-27T21:25:43Z

theano/tensor/signal/downsample.py

@@ -19,7 +19,7 @@ def max_pool2D(*args, **kwargs):
    return max_pool_2d(*args, **kwargs)


-def max_pool_2d(input, ds, ignore_border=False, st=None):
+def max_pool_2d(input, ds, ignore_border=False, st=None, padding=(0,0)):


PEP8: (0, 0) with a space after the comma

Also, add documentation for the padding parameter in the docstring.

lamblin · 2015-02-27T22:22:44Z

The gradient of the gradient is also missing, but I'm not sure there is a cudnn op for that.
In that case, the grad method of DownsampleFactorMaxGrad should return grad_not_implemented for these cases.

lamblin · 2015-02-27T23:23:29Z

theano/tensor/signal/downsample.py

                                    gx[n, k, row_ind, col_ind] += gz[n, k, r, c]
+        import ipdb; ipdb.set_trace()


That should not be here...

yaoli · 2015-03-02T19:00:46Z

addressed some comments.

The gradient check still fails in "def test_DownsampleFactorMaxPaddingStride_grad(self)" in tests/test_downsample.py

yaoli · 2015-03-02T19:50:20Z

grad is still not working....

lamblin · 2015-03-02T21:02:14Z

theano/tensor/signal/downsample.py

@@ -320,16 +334,17 @@ def c_code_cache_version(self):
 class DownsampleFactorMaxGrad(Op):
    __props__ = ('ds', 'ignore_border', 'st')


'padding' should be in there too

yaoli · 2015-03-02T21:45:38Z

Just simplified some minor stuff, grad test is fast now, but still broken

lamblin · 2015-03-02T21:58:54Z

theano/tensor/signal/downsample.py

+
+        # pad the image
+        fill = x.min()-1
+        y = numpy.zeros((x.shape[0], x.shape[1], img_rows, img_cols)) + fill


You have to give y the same dtype as x, otherwise the comparison later will not work.

lamblin · 2015-03-02T22:28:38Z

theano/tensor/signal/downsample.py

@@ -101,6 +100,9 @@ def out_shape(imgshape, ds, ignore_border=False, st=None):
            extra row/col of partial downsampling (False) or ignore it (True).
        :type ignore_border: bool

+        :param padding: pad zeros on four borders of the images
+        :type padding: tuple of two ints 


Explain that the first int will be the padding both on top and on bottom, and the second int is the padding to the left and to the right.

lamblin · 2015-03-02T22:30:34Z

You also have to pass the padding to the Grad Op in the grad function.

yaoli · 2015-03-02T23:04:19Z

I've tried to fix as many pep8 as I could.
The grad is good now.

On Mon, Mar 2, 2015 at 5:30 PM, Pascal Lamblin notifications@github.com
wrote:

You also have to pass the padding to the Grad Op in the grad function.

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

lamblin · 2015-03-02T23:43:16Z

OK, I'll make another pass.

lamblin · 2015-03-02T23:46:24Z

theano/tensor/signal/downsample.py

@@ -101,6 +101,10 @@ def out_shape(imgshape, ds, ignore_border=False, st=None):
            extra row/col of partial downsampling (False) or ignore it (True).
        :type ignore_border: bool

+        :param padding: (pad_h, pad_w), pad zeros on four borders
+            of the images, pad_h for padding rows, and pad_w for columns.


That is a bit ambiguous: "padding rows" could mean that you pad each row so the rows are longer, but actually it adds new rows on top and bottom. Can you say that pad_h is the size of the top and bottom borders, and pad_w is the size of the left and right bordres instead?

lamblin · 2015-03-02T23:56:23Z

theano/tensor/signal/tests/test_downsample.py

+
+    def test_DownsampleFactorMaxPaddingStride_grad(self):
+        rng = numpy.random.RandomState(utt.fetch_seed())
+        imval = rng.rand(1, 1, 10, 10) * 10.0


Also try with a non-square image.

lamblin · 2015-03-03T00:02:30Z

So, to summarize, the things that are still needed are:

define pad_h and pad_w before using them, both in the forward op and the grad
clarification of pad_h and pad_w in the documentation
test gradient with non-square image and non-square padding
my sub-PR about pep8

yaoli · 2015-03-03T00:14:21Z

I addressed first three. What shall I do with the sub-PR?

On Mon, Mar 2, 2015 at 7:02 PM, Pascal Lamblin notifications@github.com
wrote:

So, to summarize, the things that are still needed are:

define pad_h and pad_w before using them, both in the forward op and
the grad

clarification of pad_h and pad_w in the documentation

test gradient with non-square image and non-square padding

my sub-PR about pep8

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

Additional pep8 fixes

yaoli · 2015-03-03T00:17:51Z

I've just pulled it

On Mon, Mar 2, 2015 at 7:13 PM, Li Yao yaoli.email@gmail.com wrote:

I addressed first three. What shall I do with the sub-PR?

On Mon, Mar 2, 2015 at 7:02 PM, Pascal Lamblin notifications@github.com
wrote:

So, to summarize, the things that are still needed are:

define pad_h and pad_w before using them, both in the forward op
and the grad

clarification of pad_h and pad_w in the documentation

test gradient with non-square image and non-square padding

my sub-PR about pep8

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

lamblin · 2015-03-03T00:18:21Z

Thanks, I think it's correct.
I'll merge when travis passes.

yaoli · 2015-03-03T00:23:10Z

Awesome, thank you so much for the help!

On Mon, Mar 2, 2015 at 7:18 PM, Pascal Lamblin notifications@github.com
wrote:

Thanks, I think it's correct.
I'll merge when travis passes.

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

support max pooling with padding

Li added 3 commits February 27, 2015 16:13

added support for padding

5b220ef

better

125cca7

tentative push

b7b1060

lamblin reviewed Feb 27, 2015
View reviewed changes

not ready

1abd8ee

lamblin reviewed Feb 27, 2015
View reviewed changes

Li added 2 commits March 2, 2015 13:57

address some comments, grad of maxpooling with padding still not working

cdfd55e

grad test failed

7d25979

Li added 2 commits March 2, 2015 14:02

syntax

ab6c7a3

grad still failed

1fd2472

lamblin reviewed Mar 2, 2015
View reviewed changes

more corrections, grad not work still

218c02f

lamblin reviewed Mar 2, 2015
View reviewed changes

grad still not working

33551a3

lamblin reviewed Mar 2, 2015
View reviewed changes

pep8, and grad finally worked

bd5dc5a

lamblin reviewed Mar 2, 2015
View reviewed changes

Additional pep8 fixes

1b01e2f

lamblin reviewed Mar 2, 2015
View reviewed changes

better wording

b14a0e3

Merge pull request #4 from lamblin/yaoli_pool_pad

38da1f3

Additional pep8 fixes

lamblin added a commit that referenced this pull request Mar 3, 2015

Merge pull request #2543 from yaoli/pool_pad

471a171

support max pooling with padding

lamblin merged commit 471a171 into Theano:master Mar 3, 2015

This was referenced Mar 3, 2015

Fix opt crash due to new CPU parameter #2550

Merged

Make Theano's max_pool_2d support strides and support padding with 0 (to support size preserving version) #2196

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support max pooling with padding #2543

support max pooling with padding #2543

yaoli commented Feb 27, 2015

lamblin Feb 27, 2015

lamblin Feb 27, 2015

lamblin commented Feb 27, 2015

lamblin Feb 27, 2015

yaoli commented Mar 2, 2015

yaoli commented Mar 2, 2015

lamblin Mar 2, 2015

yaoli commented Mar 2, 2015

lamblin Mar 2, 2015

lamblin Mar 2, 2015

lamblin commented Mar 2, 2015

yaoli commented Mar 2, 2015

lamblin commented Mar 2, 2015

lamblin Mar 2, 2015

lamblin Mar 2, 2015

lamblin commented Mar 3, 2015

yaoli commented Mar 3, 2015

yaoli commented Mar 3, 2015

lamblin commented Mar 3, 2015

yaoli commented Mar 3, 2015

		gx[n, k, row_ind, col_ind] += gz[n, k, r, c]
		import ipdb; ipdb.set_trace()

		@@ -320,16 +334,17 @@ def c_code_cache_version(self):
		class DownsampleFactorMaxGrad(Op):
		__props__ = ('ds', 'ignore_border', 'st')

support max pooling with padding #2543

support max pooling with padding #2543

Conversation

yaoli commented Feb 27, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lamblin commented Feb 27, 2015

Choose a reason for hiding this comment

yaoli commented Mar 2, 2015

yaoli commented Mar 2, 2015

Choose a reason for hiding this comment

yaoli commented Mar 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lamblin commented Mar 2, 2015

yaoli commented Mar 2, 2015

lamblin commented Mar 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lamblin commented Mar 3, 2015

yaoli commented Mar 3, 2015

yaoli commented Mar 3, 2015

lamblin commented Mar 3, 2015

yaoli commented Mar 3, 2015