Merge remote-tracking branch 'vivek/TheanoBugFix' into grad_advinc_subtensor

theano/sparse/basic.py

@@ -8,11 +8,9 @@ http://www-users.cs.umn.edu/~saad/software/SPARSKIT/paper.ps
 # Automatic methods for determining best sparse format?
 
 import sys
-from itertools import izip
 import numpy
 import theano
 import scipy.sparse
-
 from theano import gof, tensor, compile, scalar, config
 from theano.gof.python25 import all
 from theano.gradient import DisconnectedType
@@ -1710,9 +1708,9 @@ class AddSD(gof.op.Op):
 
     :note: The grad implemented is structured on `x`.
     """
-    
     def __init__(self, inplace=False, *args, **kwargs):
         gof.Op.__init__(self, *args, **kwargs)
+        #Should we do inplace addition or not ?
         self.inplace = inplace
         if self.inplace:
             self.destroy_map = {0: [3]}
@@ -1728,15 +1726,18 @@ class AddSD(gof.op.Op):
             return self.__class__.__name__ + '{inplace}'
         return self.__class__.__name__
 
-
     def make_node(self, x, y):
         x, y = as_sparse_variable(x), tensor.as_tensor_variable(y)
+
         if x.type.dtype != y.type.dtype:
             raise NotImplementedError()
-        # The magic number two here arises because L{scipy.sparse}
-        # objects must be matrices (have dimension 2)
+
         indices, indptr, data = csm_indices(x), csm_indptr(x), csm_data(x)
+
+        # We either use CSC or CSR depending on the format of input
         self.format = x.format
+        # The magic number two here arises because L{scipy.sparse}
+        # objects must be matrices (have dimension 2)
         assert y.type.ndim == 2
         return gof.Apply(self,
                          [data, indices, indptr, y],
@@ -1748,7 +1749,7 @@ class AddSD(gof.op.Op):
         inplace = int(self.inplace)
         format = {'csc': 0, 'csr':1}[self.format]
         code = """
-                if(%(z)s) {Py_XDECREF(%(z)s);}
+                Py_XDECREF(%(z)s);
                 if (!%(inplace)s){
                   %(z)s = (PyArrayObject *) PyArray_NewCopy(%(y)s, NPY_CORDER);
                 }else{
@@ -1789,25 +1790,12 @@ class AddSD(gof.op.Op):
 
     def perform(self, node, (data, indices, indptr,  y), (out, )):
         assert _is_dense(y)
-        if self.inplace:
-          if self.format == 'csc':
-            for c in xrange(y.shape[1]):
-              low = indptr[c]
-              high = indptr[c+1]
-              for ind in xrange(low, high):
-                y[(indices[ind], c)] += data[ind]
-          elif self.format == 'csr':
-            for r in xrange(y.shape[0]):
-              low = indptr[r]
-              high = indptr[r+1]
-              for ind in xrange(low, high):
-                y[(r, indices[ind])] += data[ind]
-          out[0] = y
-        else:
+
         if self.format == 'csr':
-            x = scipy.sparse.csr_matrix( (data,indices,indptr), shape=y.shape)
+            x = scipy.sparse.csr_matrix((data, indices, indptr), shape = y.shape)
         elif self.format == 'csc':
-            x = scipy.sparse.csc_matrix( (data,indices,indptr), shape=y.shape)
+            x = scipy.sparse.csc_matrix((data, indices, indptr), shape = y.shape)
+
         # The asarray is needed as in some case, this return a
         # numpy.matrixlib.defmatrix.matrix object and not an ndarray.
         out[0] = theano._asarray(x + y, dtype=node.outputs[0].type.dtype)

theano/sparse/opt.py

@@ -36,11 +36,14 @@ def local_inplace_remove0(node):
     """
     Optimization to insert inplace versions of Remove0.
     """
+    # If inplace is not enabled, enable it and replace that op with a
+    # new op which has inplace enabled
     if isinstance(node.op, sparse.Remove0) and not node.op.inplace:
         new_op = node.op.__class__(inplace=True)
         new_node = new_op(*node.inputs)
         return [new_node]
     return False
+
 theano.compile.optdb.register('local_inplace_remove0',
                               gof.TopoOptimizer(local_inplace_remove0,
     failure_callback=gof.TopoOptimizer.warn_inplace),
@@ -48,7 +51,7 @@ theano.compile.optdb.register('local_inplace_remove0',
 @gof.local_optimizer([None])
 def local_inplace_addsd(node):
     """
-    Optimization to insert inplace versions of Remove0.
+    Optimization to insert inplace versions of AddSD.
     """
     if isinstance(node.op, sparse.AddSD) and not node.op.inplace:
         inputs = node.inputs[:3] + [node.inputs[3].shape]

theano/tensor/basic.py

@@ -7,8 +7,6 @@ import warnings
 from itertools import izip
 
 import numpy
-from numpy.lib.stride_tricks import as_strided
-import scipy.sparse as ssparse
 #from copy import copy as python_copy
 
 import theano
@@ -6522,12 +6520,10 @@ class AdvancedSubtensor1(Op):
         return rval
 
     def grad(self, inputs, grads):
-        gz, = grads
 
+        gz, = grads
         assert len(inputs) == 2
-#        rval1 = [advanced_inc_subtensor1(zeros_like(inputs[0]), gz, inputs[1])]
         rval1 = [ConstructSparse()(inputs[0], gz, inputs[1])]
-
         return rval1 + [DisconnectedType()()] * (len(inputs) - 1)
 
     def R_op(self, inputs, eval_points):
@@ -6539,8 +6535,9 @@ class AdvancedSubtensor1(Op):
         x, ilist = ishapes
         return [ilist + x[1:]]
 
+
 class ConstructSparse(Op):
-    """Construct a sparse matrix out of a list of 2-D matrix rows"""
+    """Constructs a sparse matrix out of a list of 2-D matrix rows"""
 
     def __hash__(self):
         return hash((type(self)))
@@ -6552,10 +6549,6 @@ class ConstructSparse(Op):
         return self.__class__.__name__
 
     def make_node(self, x, y, ilist):
-
-        x_sparse = ssparse.csc_matrix(tuple(x.shape.eval()), dtype=x.dtype)
-        x__ = theano.sparse.as_sparse_variable(x_sparse)
-
         x_ = as_tensor_variable(x)
         y_ = as_tensor_variable(y)
         ilist_ = as_tensor_variable(ilist)
@@ -6567,27 +6560,23 @@ class ConstructSparse(Op):
         if x_.type.ndim == 0:
             raise TypeError('cannot index into a scalar')
         if y_.type.ndim > x_.type.ndim:
-            if self.set_instead_of_inc:
-                opname = 'set'
-            else:
-                opname = 'increment'
-            raise TypeError('cannot %s x subtensor with ndim=%s'
-            ' by y with ndim=%s to x subtensor with ndim=%s ' % (
-                opname, x_.type.ndim, y_.type.ndim))
-
-        return Apply(self, [x_, y_, ilist_], [x__.type()])
+            raise TypeError('cannot construct sparse matrix as dimensions differ')    
+        return Apply(self, [x_, y_, ilist_], [theano.sparse.csc_matrix(dtype=x.dtype)])
 
     def perform(self, node, inp, out_):
+        import scipy.sparse as ssparse
+        from numpy.lib.stride_tricks import as_strided
         x, values, idx = inp
         out, = out_
         rows, cols = values.shape
         assert rows == len(idx)
-        indptr = numpy.arange(cols+1) * rows
+        indptr = numpy.arange(cols + 1) * rows
         indices = as_strided(idx,
                              strides=(0, idx.strides[0]),
-                             shape=(cols, idx.shape[0])).flatten()
+                             shape = (cols, idx.shape[0])).flatten()
         data = values.T.flatten()
-        out[0] = ssparse.csc_matrix((data,indices,indptr), shape=x.shape, dtype=x.dtype)
+        out[0] = ssparse.csc_matrix((data, indices, indptr), shape=x.shape,
+                                    dtype=x.dtype)
 
     def infer_shape(self, node, ishapes):
         x, y, ilist = ishapes
@@ -6618,7 +6607,6 @@ class ConstructSparse(Op):
 
         return [gx, gy] + [DisconnectedType()()] * len(idx_list)
 
-
 advanced_subtensor1 = AdvancedSubtensor1()