DownsampleFactorMax support strides: issue #2196 #2222

SinaHonari · 2014-10-28T20:18:20Z

No description provided.

nouiz · 2014-10-29T15:47:06Z

theano/tensor/signal/downsample.py

+                                continue
+                            for j in xrange(ds1):
+                                col_ind = col_st + j
+                                if col_ind >= img_cols:


You can remove the if, but using for j in xrange(ds1, ...). ... is the max number of iteration. It is excluded.

nouiz · 2014-10-29T15:49:40Z

I get this error:

======================================================================
ERROR: test_DownsampleFactorMax_hessian (theano.tensor.signal.tests.test_downsample.TestDownsampleFactorMax)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/u/bastienf/repos/Theano.Clean/theano/tensor/signal/tests/test_downsample.py", line 116, in test_DownsampleFactorMax_hessian
    fn_hess = function(inputs=[x_vec], outputs=grad_hess)
  File "/u/bastienf/repos/Theano.Clean/theano/compile/function.py", line 265, in function
    profile=profile)
  File "/u/bastienf/repos/Theano.Clean/theano/compile/pfunc.py", line 511, in pfunc
    on_unused_input=on_unused_input)
  File "/u/bastienf/repos/Theano.Clean/theano/compile/function_module.py", line 1540, in orig_function
    defaults)
  File "/u/bastienf/repos/Theano.Clean/theano/compile/function_module.py", line 1403, in create
    _fn, _i, _o = self.linker.make_thunk(input_storage=input_storage_lists)
  File "/u/bastienf/repos/Theano.Clean/theano/gof/link.py", line 490, in make_thunk
    output_storage=output_storage)[:3]
  File "/u/bastienf/repos/Theano.Clean/theano/gof/vm.py", line 891, in make_all
    no_recycling))
  File "/u/bastienf/repos/Theano.Clean/theano/gof/op.py", line 726, in make_thunk
    output_storage=node_output_storage)
  File "/u/bastienf/repos/Theano.Clean/theano/gof/cc.py", line 1023, in make_thunk
    keep_lock=keep_lock)
  File "/u/bastienf/repos/Theano.Clean/theano/gof/cc.py", line 965, in __compile__
    keep_lock=keep_lock)
  File "/u/bastienf/repos/Theano.Clean/theano/gof/cc.py", line 1403, in cthunk_factory
    key=key, fn=self.compile_cmodule_by_step, keep_lock=keep_lock)
  File "/u/bastienf/repos/Theano.Clean/theano/gof/cmodule.py", line 950, in module_from_key
    src_code = next(compile_steps)
  File "/u/bastienf/repos/Theano.Clean/theano/gof/cc.py", line 1287, in compile_cmodule_by_step
    mod = self.build_dynamic_module()
  File "/u/bastienf/repos/Theano.Clean/theano/gof/cc.py", line 1331, in build_dynamic_module
    self.code_gen()
  File "/u/bastienf/repos/Theano.Clean/theano/gof/cc.py", line 701, in code_gen
    raise NotImplementedError("%s cannot produce C code" % op)
  File "/u/bastienf/repos/Theano.Clean/theano/tensor/signal/downsample.py", line 148, in __str__
    self.ds, self.st, self.ignore_border)
TypeError: <unprintable TypeError object>

nouiz · 2014-10-29T15:50:29Z

theano/tensor/signal/downsample.py


    def __str__(self):
        return '%s{%s,%s}' % (self.__class__.__name__,
-                              self.ds, self.ignore_border)
+                              self.ds, self.st, self.ignore_border)


You forgot to modify the string on line 147. This cause crash.

In fact, I would take this opportunity to make this op use the new props = (...) attribute.

Remove hash, eq and str in that op. Add the attribute props = ('ds', 'st', 'ignore_border') to that class. This will cause the 3 methods to be generated automatically.

nouiz · 2014-10-29T15:58:25Z

I wasn't able to reproduce the error you wrote me about c_code in an email. So I guess it was the str problem. If not, give me the error message.

thanks

lamblin · 2014-11-13T20:49:54Z

theano/tensor/signal/downsample.py

        :type ds: list or tuple of two ints

+        :param st: the stride size 


Add that this is the distance between pooling regions.
Also add that it can be None, and in that case the value provided as ds will be used, and that it corresponds to the case of adjacent pooling regions.

lamblin · 2014-11-19T18:20:31Z

OK, I did not see your comment about my formula before I reviewed, we should discuss what the right answer should be before trying to implement it.

lamblin · 2014-11-25T01:17:23Z

theano/tensor/signal/downsample.py

+        nr = 0
+        nc = 0
+        if isinstance(r, theano.Variable):
+            nr = tensor.switch(tensor.ge(r - ds[0], 0), out_r, 0)


This can be simplified in nr = tensor.max(out_r, 0) if r is a Variable, and nr = numpy.max(out_r, 0) otherwise.
Idem for nc. And you don't have to assign them 0 before.

lamblin · 2014-11-29T02:21:30Z

Otherwise, it looks good, thanks for completing that part.

lamblin · 2014-12-04T01:08:40Z

You will need to rebase and force-push so that Travis can run.

lamblin · 2014-12-15T23:20:02Z

theano/tensor/signal/downsample.py

@@ -183,10 +232,13 @@ def grad(self, inp, grads):
        gz, = grads
        maxout = self(x)
        return [DownsampleFactorMaxGrad(self.ds,
-                                        ignore_border=self.ignore_border)(
+                                        ignore_border=self.ignore_border,
+                                        st=self.st)(
                                            x, maxout, gz)]


Before it is tested, can you return theano.gradient.grad_not_implemented when self.st != self.ds? That way, we are sure not to give wrong results.

lamblin · 2014-12-15T23:20:49Z

theano/tensor/signal/downsample.py


-    def c_code(self, node, name, inp, out, sub):
+    def c_code_tmp(self, node, name, inp, out, sub):


Same as the forward op, re-enable the C code when st == ds.

SinaHonari · 2014-12-16T20:12:51Z

this is for ticket #2196

… is greater than the pooling size

…eano#2196

…or pep8 for issue Theano#2196

…#2196

lamblin · 2014-12-18T21:39:27Z

theano/tensor/signal/downsample.py

@@ -322,10 +386,14 @@ def infer_shape(self, node, in_shapes):
    def grad(self, inp, grads):
        x, maxout, gz = inp
        ggx, = grads
+        if self.st != self.ds:
+            return [theano.gradient.grad_not_implemented(self, 0, x),
+                    theano.gradient.grad_not_implemented(self, 1, maxout)


There is a missing comma at the end of the line, this is a syntax error.

DownsampleFactorMax support strides: issue #2196

lamblin · 2014-12-19T21:21:27Z

Thanks for completing that part!

nouiz · 2015-02-05T18:19:22Z

theano/tensor/signal/downsample.py

-    :param ds: factor by which to downscale (vertical ds, horizontal ds). 
-        (2,2) will halve the image in each dimension. 
+    :param ds: factor by which to downscale (vertical ds, horizontal ds).
+        (2,2) will halve the image in each dimension.


The method max_pool_2d need an st parameter. I'll fix it.

Also the GPU opt that move this op the GPU do not take check the new parameter! This introduce a bug. I'll fix it too.

nouiz reviewed Oct 29, 2014
View reviewed changes

nouiz closed this Oct 29, 2014

nouiz reopened this Oct 29, 2014

nouiz reviewed Oct 29, 2014
View reviewed changes

nouiz changed the title ~~initial changes for issue #2196~~ DownsampleFactorMax support strides: issue #2196 Oct 29, 2014

lamblin reviewed Nov 13, 2014
View reviewed changes

lamblin reviewed Nov 25, 2014
View reviewed changes

SinaHonari force-pushed the issue2196 branch from 4bcca93 to 6991637 Compare December 4, 2014 17:44

lamblin self-assigned this Dec 15, 2014

lamblin reviewed Dec 15, 2014
View reviewed changes

Sina Honari added 14 commits December 17, 2014 17:24

initial changes for issue Theano#2196

0dbb619

issue Theano#2196: adding functionality for the case when stride size…

54d5d47

… is greater than the pooling size

fixing the pooling with stride plus adding a test.

ad22f51

fixing the stride computation case and adding test cases for issue Th…

b3a48f4

…eano#2196

removing an extra line issue Theano#2196

a127689

applying changes to the case where stride is used for issue Theano#2196

26c105c

applying the changes for the case of ignore_border plus the changes f…

1d2eec5

…or pep8 for issue Theano#2196

resolving the conflict for git rebase

84c5191

enabling the c code for DownsampleFactorMax when st==ds, issue Theano…

bf64aee

…#2196

correcting DownsampleFactorMaxGrad, issue Theano#2196

9ef594d

changeing the GradGrad section, issue Theano#2196

cac6d8c

changes to throw exception if st!=ds

4fe11a7

updating the st != ds case

b94dc4a

updating the return method for the case where ds != st

a925585

SinaHonari force-pushed the issue2196 branch from 95e1afe to a925585 Compare December 17, 2014 22:27

lamblin reviewed Dec 18, 2014
View reviewed changes

correcting grad_not_implemented

7518621

lamblin added a commit that referenced this pull request Dec 19, 2014

Merge pull request #2222 from SinaHonari/issue2196

c2895dc

DownsampleFactorMax support strides: issue #2196

lamblin merged commit c2895dc into Theano:master Dec 19, 2014

nouiz reviewed Feb 5, 2015
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DownsampleFactorMax support strides: issue #2196 #2222

DownsampleFactorMax support strides: issue #2196 #2222

SinaHonari commented Oct 28, 2014

nouiz Oct 29, 2014

nouiz commented Oct 29, 2014

nouiz Oct 29, 2014

nouiz Oct 29, 2014

nouiz commented Oct 29, 2014

lamblin Nov 13, 2014

lamblin commented Nov 19, 2014

lamblin Nov 25, 2014

lamblin commented Nov 29, 2014

lamblin commented Dec 4, 2014

lamblin Dec 15, 2014

lamblin Dec 15, 2014

SinaHonari commented Dec 16, 2014

lamblin Dec 18, 2014

lamblin commented Dec 19, 2014

nouiz Feb 5, 2015

		:type ds: list or tuple of two ints

		:param st: the stride size


		def c_code(self, node, name, inp, out, sub):
		def c_code_tmp(self, node, name, inp, out, sub):

DownsampleFactorMax support strides: issue #2196 #2222

DownsampleFactorMax support strides: issue #2196 #2222

Conversation

SinaHonari commented Oct 28, 2014

Choose a reason for hiding this comment

nouiz commented Oct 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nouiz commented Oct 29, 2014

Choose a reason for hiding this comment

lamblin commented Nov 19, 2014

Choose a reason for hiding this comment

lamblin commented Nov 29, 2014

lamblin commented Dec 4, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SinaHonari commented Dec 16, 2014

Choose a reason for hiding this comment

lamblin commented Dec 19, 2014

Choose a reason for hiding this comment