Remove redundant AdvancedBoolean*Subtensor classes

This change also fixes the `set_subtensor` boolean gradient bug in #105.

Author: brandonwillard

tests/gpuarray/test_subtensor.py

@@ -11,7 +11,6 @@
     GpuSubtensor,
     GpuAdvancedSubtensor1,
     GpuAdvancedSubtensor,
-    GpuAdvancedBooleanSubtensor,
     GpuAdvancedIncSubtensor,
     GpuAdvancedIncSubtensor1,
     GpuAdvancedIncSubtensor1_dev20,
@@ -37,7 +36,6 @@ def shared(x, **kwargs):
         self.adv_sub1 = GpuAdvancedSubtensor1
         self.adv_incsub1 = GpuAdvancedIncSubtensor1
         self.adv_sub = GpuAdvancedSubtensor
-        self.adv_bool_sub = GpuAdvancedBooleanSubtensor
         self.dimshuffle = GpuDimShuffle
         self.mode = mode_with_gpu
         # avoid errors with limited devices
@@ -60,7 +58,6 @@ def shared(x, **kwargs):
         self.adv_sub1 = GpuAdvancedSubtensor1
         self.adv_incsub1 = GpuAdvancedIncSubtensor1
         self.adv_sub = GpuAdvancedSubtensor
-        self.adv_bool_sub = GpuAdvancedBooleanSubtensor
         self.dimshuffle = GpuDimShuffle
         self.mode = mode_with_gpu
         # avoid errors with limited devices

tests/tensor/test_subtensor.py

@@ -46,8 +46,6 @@
     AdvancedIncSubtensor1,
     AdvancedSubtensor,
     AdvancedSubtensor1,
-    AdvancedBooleanSubtensor,
-    AdvancedBooleanIncSubtensor,
     advanced_inc_subtensor,
     advanced_inc_subtensor1,
     advanced_set_subtensor,
@@ -67,8 +65,6 @@
     IncSubtensor,
     AdvancedSubtensor1,
     AdvancedIncSubtensor1,
-    AdvancedBooleanSubtensor,
-    AdvancedBooleanIncSubtensor,
 )
 
 
@@ -368,12 +364,10 @@ def numpy_inc_subtensor(x, idx, a):
 
         # indexing with a mask for some dimensions
         mask = np.array([True, False])
-        val = self.eval_output_and_check(
-            test_array[mask], op_type=AdvancedBooleanSubtensor
-        )
+        val = self.eval_output_and_check(test_array[mask], op_type=AdvancedSubtensor)
         assert_array_equal(test_array_np[mask], val)
         val = self.eval_output_and_check(
-            inc_subtensor(test_array[mask], 1), op_type=AdvancedBooleanIncSubtensor
+            inc_subtensor(test_array[mask], 1), op_type=AdvancedIncSubtensor
         )
         assert_array_equal(numpy_inc_subtensor(test_array_np, mask, 1), val)
         assert_array_equal(
@@ -580,8 +574,8 @@ def test_grad_1d(self):
         topo_ = [node for node in topo if not isinstance(node.op, DeepCopyOp)]
         if not self.fast_compile:
             assert len(topo_) == 6
-        assert np.sum([isinstance(node.op, IncSubtensor) for node in topo_]) == 1
-        assert np.sum([isinstance(node.op, Subtensor) for node in topo_]) == 1
+        assert any(isinstance(node.op, IncSubtensor) for node in topo_)
+        assert any(isinstance(node.op, Subtensor) for node in topo_)
         gval = f()
 
         good = np.zeros_like(data)
@@ -1161,47 +1155,6 @@ def test_shape_list(self):
             val = f()
             assert np.allclose(val, data[idx].shape)
 
-    def test_grad_advanced_inc_subtensor(self):
-        def inc_slice(*s):
-            def just_numeric_args(a, b):
-                cost = (a[s] + b).sum()
-                cost_wrt_a = tensor.grad(cost, a)
-                cost_wrt_b = tensor.grad(cost, b)
-                grads = cost_wrt_a.sum() + cost_wrt_b.sum()
-                return grads
-
-            return just_numeric_args
-
-        # vector
-        utt.verify_grad(
-            inc_slice(slice(2, 4, None)),
-            (
-                np.asarray([0, 1, 2, 3, 4, 5.0]),
-                np.asarray([9, 9.0]),
-            ),
-            mode=self.mode,
-        )
-
-        # matrix
-        utt.verify_grad(
-            inc_slice(slice(1, 2, None), slice(None, None, None)),
-            (
-                np.asarray([[0, 1], [2, 3], [4, 5.0]]),
-                np.asarray([[9, 9.0]]),
-            ),
-            mode=self.mode,
-        )
-
-        # single element
-        utt.verify_grad(
-            inc_slice(2, 1),
-            (
-                np.asarray([[0, 1], [2, 3], [4, 5.0]]),
-                np.asarray(9.0),
-            ),
-            mode=self.mode,
-        )
-
     def test_inc_and_set_subtensor(self):
         # Test increment and set with broadcast
 
@@ -1323,21 +1276,6 @@ def test_advanced1_inc_and_set(self):
                         all_params.append(
                             (set_instead_of_inc, inplace, data_shape, inc_shape)
                         )
-                        if False:  # Enable for debugging purpose.
-                            f = self.function(
-                                [data_var, idx_var, inc_var],
-                                output,
-                                accept_inplace=inplace,
-                                op=AdvancedIncSubtensor1,
-                            )
-                            if inplace:
-                                # Ensure calling `f` will not alter `data_num`.
-                                data_num = data_num.copy()
-                            f_out = f(data_num.copy(), idx_num, inc_num)
-                            assert np.allclose(f_out, data_copy)
-                            if not inplace:
-                                # Sanity check: `data_num` should be intact.
-                                assert (data_num == data_num_init).all()
 
         # Actual test (we compile a single Theano function to make it faster).
         orig_warn = theano.config.warn.gpu_set_subtensor1
@@ -1647,18 +1585,18 @@ def check(idx, y_val, x_val, true):
             rep[idx] += y_val
             check(idx, y_val, x_val, rep)
 
-    def eval_output_and_check(self, t):
+    def eval_output_and_check(self, t, op):
         f = inplace_func([], t, mode=self.mode)
         topo = f.maker.fgraph.toposort()
         topo_ = [node for node in topo if not isinstance(node.op, DeepCopyOp)]
         assert len(topo_) == 1
-        assert isinstance(topo_[0].op, AdvancedSubtensor)
+        assert isinstance(topo_[0].op, op)
         tval = f()
         return tval
 
     def test_cant_adv_idx_into_scalar(self):
         with pytest.raises(IndexError):
-            (lambda: self.s[self.ix1])()
+            self.s[self.ix1]
 
     def test_index_into_vec_w_vec(self):
         a = self.v[self.ix1]
@@ -1698,7 +1636,7 @@ def test_index_w_int_and_vec(self):
 
             assert isinstance(t.owner.op, AdvancedSubtensor)
 
-            val = self.eval_output_and_check(t)
+            val = self.eval_output_and_check(t, AdvancedSubtensor)
             if isinstance(idx, list):
                 good = data[0, idx]
             else:
@@ -1942,6 +1880,35 @@ def fun(x, y):
             mode=self.mode,
         )
 
+        # Test boolean gradients
+        def fun(x, y):
+            return advanced_inc_subtensor(
+                x, y, tensor.as_tensor(np.array([[True, False], [False, True]]))
+            )
+
+        utt.verify_grad(
+            fun,
+            [
+                np.random.rand(2, 2).astype(self.dtype),
+                np.random.rand(2).astype(self.dtype),
+            ],
+            mode=self.mode,
+        )
+
+        def fun(x, y):
+            return advanced_set_subtensor(
+                x, y, tensor.as_tensor(np.array([[True, False], [False, True]]))
+            )
+
+        utt.verify_grad(
+            fun,
+            [
+                np.random.rand(2, 2).astype(self.dtype),
+                np.random.rand(2).astype(self.dtype),
+            ],
+            mode=self.mode,
+        )
+
 
 class TestInferShape(utt.InferShapeTester):
     def test_IncSubtensor(self):
@@ -2216,7 +2183,7 @@ def test_AdvancedSubtensor(self):
             AdvancedSubtensor,
         )
 
-    def test_AdvancedBooleanSubtensor(self):
+    def test_AdvancedSubtensor_bool(self):
         n = dmatrix()
         n_val = np.arange(6).reshape((2, 3))
 
@@ -2225,14 +2192,14 @@ def test_AdvancedBooleanSubtensor(self):
             [n],
             [n[n[:, 0] > 2, n[0, :] > 2]],
             [n_val],
-            AdvancedBooleanSubtensor,
+            AdvancedSubtensor,
             check_topo=False,
         )
         self._compile_and_check(
             [n],
             [n[n[:, 0] > 2]],
             [n_val],
-            AdvancedBooleanSubtensor,
+            AdvancedSubtensor,
             check_topo=False,
         )
 

tests/tensor/test_var.py

@@ -11,7 +11,6 @@
 from theano.tensor.subtensor import (
     Subtensor,
     AdvancedSubtensor,
-    AdvancedBooleanSubtensor,
     AdvancedSubtensor1,
 )
 from theano.tensor.elemwise import DimShuffle
@@ -124,31 +123,30 @@ def test__getitem__Subtensor():
     assert op_types[-1] == Subtensor
 
 
-def test__getitem__AdvancedBooleanSubtensor():
-    # Make sure we get `AdvancedBooleanSubtensor`s for basic indexing operations
+def test__getitem__AdvancedSubtensor_bool():
     x = tt.matrix("x")
     i = tt.type.TensorType("bool", (False, False))("i")
 
     z = x[i]
     op_types = [type(node.op) for node in theano.gof.graph.io_toposort([x, i], [z])]
-    assert op_types[-1] == AdvancedBooleanSubtensor
+    assert op_types[-1] == AdvancedSubtensor
 
     i = tt.type.TensorType("bool", (False,))("i")
     z = x[:, i]
     op_types = [type(node.op) for node in theano.gof.graph.io_toposort([x, i], [z])]
-    assert op_types[-1] == AdvancedBooleanSubtensor
+    assert op_types[-1] == AdvancedSubtensor
 
     i = tt.type.TensorType("bool", (False,))("i")
     z = x[..., i]
     op_types = [type(node.op) for node in theano.gof.graph.io_toposort([x, i], [z])]
-    assert op_types[-1] == AdvancedBooleanSubtensor
+    assert op_types[-1] == AdvancedSubtensor
 
     with pytest.raises(TypeError):
         z = x[[True, False], i]
 
     z = x[tt.ivector("b"), i]
     op_types = [type(node.op) for node in theano.gof.graph.io_toposort([x, i], [z])]
-    assert op_types[-1] == AdvancedBooleanSubtensor
+    assert op_types[-1] == AdvancedSubtensor
 
 
 def test__getitem__AdvancedSubtensor():

theano/gpuarray/opt.py

@@ -130,11 +130,9 @@
     GpuSubtensor,
     GpuAdvancedSubtensor,
     GpuAdvancedSubtensor1,
-    GpuAdvancedBooleanSubtensor,
     GpuAdvancedIncSubtensor,
     GpuAdvancedIncSubtensor1,
     GpuAdvancedIncSubtensor1_dev20,
-    GpuAdvancedBooleanIncSubtensor,
     GpuAllocDiag,
     GpuExtractDiag,
 )
@@ -1291,13 +1289,6 @@ def local_gpua_advanced_subtensor(op, context_name, inputs, outputs):
     return GpuAdvancedSubtensor()
 
 
-@register_opt("fast_compile")
-@op_lifter([tensor.AdvancedBooleanSubtensor])
-@register_opt2([tensor.AdvancedBooleanSubtensor], "fast_compile")
-def local_gpua_advanced_boolean_subtensor(op, context_name, inputs, outputs):
-    return GpuAdvancedBooleanSubtensor()
-
-
 @register_opt("fast_compile")
 @op_lifter([tensor.AdvancedIncSubtensor1])
 @register_opt2([tensor.AdvancedIncSubtensor1], "fast_compile")
@@ -1342,20 +1333,6 @@ def local_gpua_advanced_incsubtensor(op, context_name, inputs, outputs):
         return False
 
 
-# Do not register this optimization for now, as it slows down the
-# execution by a lot in important cases.
-# @register_opt('fast_compile')
-# @op_lifter([tensor.AdvancedBooleanIncSubtensor])
-# @register_opt2([tensor.AdvancedBooleanIncSubtensor], 'fast_compile')
-def local_gpua_advanced_boolean_incsubtensor(op, context_name, inputs, outputs):
-    # GpuAdvancedIncSubtensor only works with a single boolean mask,
-    # but not with fancy combinations.
-    if not op.set_instead_of_inc and len(inputs) == 3:
-        return GpuAdvancedBooleanIncSubtensor()
-    else:
-        return False
-
-
 @register_inplace()
 @local_optimizer([GpuAdvancedIncSubtensor1, GpuAdvancedIncSubtensor1_dev20])
 def local_advincsub1_gpua_inplace(node):

theano/gpuarray/subtensor.py

@@ -683,9 +683,6 @@ class GpuAdvancedSubtensor(HideC, BaseGpuAdvancedSubtensor, tensor.AdvancedSubte
 
     def make_node(self, x, *inputs):
         ctx_name = infer_context_name(x)
-        # This method relies on AdvancedSubtensor.make_node to
-        # call tensor.subtensor.check_and_reject_bool(inputs),
-        # which raises an IndexError if there are any boolean indices.
         rval = tensor.AdvancedSubtensor.make_node(self, x, *inputs)
         otype = GpuArrayType(
             dtype=rval.outputs[0].type.dtype,
@@ -696,25 +693,6 @@ def make_node(self, x, *inputs):
         return gof.Apply(self, [x] + rval.inputs[1:], [otype()])
 
 
-class GpuAdvancedBooleanSubtensor(
-    HideC, BaseGpuAdvancedSubtensor, tensor.AdvancedBooleanSubtensor
-):
-    """
-    AdvancedBooleanSubtensor on the GPU.
-    """
-
-    def make_node(self, x, *inputs):
-        ctx_name = infer_context_name(x)
-        rval = tensor.AdvancedBooleanSubtensor.make_node(self, x, *inputs)
-        otype = GpuArrayType(
-            dtype=rval.outputs[0].type.dtype,
-            broadcastable=rval.outputs[0].type.broadcastable,
-            context_name=ctx_name,
-        )
-        x = as_gpuarray_variable(x, ctx_name)
-        return gof.Apply(self, [x] + rval.inputs[1:], [otype()])
-
-
 class BaseGpuAdvancedIncSubtensor(object):
     def perform(self, node, inp, out_):
         (out,) = out_
@@ -852,27 +830,6 @@ def make_node(self, x, y, *inputs):
         return gof.Apply(self, [x, y] + rval.inputs[2:], [otype()])
 
 
-class GpuAdvancedBooleanIncSubtensor(
-    HideC, BaseGpuAdvancedIncSubtensor, tensor.AdvancedBooleanIncSubtensor
-):
-    """
-    Implement AdvancedBooleanIncSubtensor on the gpu.
-
-    """
-
-    def make_node(self, x, y, *inputs):
-        ctx_name = infer_context_name(x, y)
-        rval = tensor.AdvancedBooleanIncSubtensor.make_node(self, x, y, *inputs)
-        otype = GpuArrayType(
-            dtype=rval.outputs[0].type.dtype,
-            broadcastable=rval.outputs[0].type.broadcastable,
-            context_name=ctx_name,
-        )
-        x = as_gpuarray_variable(x, ctx_name)
-        y = as_gpuarray_variable(y, ctx_name)
-        return gof.Apply(self, [x, y] + rval.inputs[2:], [otype()])
-
-
 class GpuAdvancedIncSubtensor1(Op):
     """
     Implement AdvancedIncSubtensor1 on the gpu.