Added comments to the code to document understanding of Gradient of Subtensor patch

theano/sparse/basic.py

@@ -21,9 +21,13 @@ import theano.tests.unittest_tools as utt
 from theano.gradient import grad_not_implemented
 from theano.sparse.type import SparseType, _is_sparse
 
+#Column compressed (CSC)
+#Row compressed (CSR)
 sparse_formats = ['csc', 'csr']
 
 
+#Register an optimization that does a specialization
+#Does the same thing but better
 # TODO: move this decorator to the compile submodule
 def register_specialize(lopt, *tags, **kwargs):
     compile.optdb['specialize'].register((kwargs and kwargs.pop('name')) or
@@ -1711,32 +1715,54 @@ class AddSD(gof.op.Op):
     :note: The grad implemented is structured on `x`.
     """
    
+    #Constructor of the object 
     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:
+            #This is a hint to the local optimizer that says that the first
+            #output is the same as the 3rd input and no intermdiate storage
+            #needs to be allocated
             self.destroy_map = {0: [3]} 
 
     def __eq__(self, other):
+        #Compare the inplace flag as well
         return (type(self) == type(other)) and self.inplace == other.inplace
 
     def __hash__(self):
+        #Now use the hash of inplace as well
         return hash(type(self)) ^ hash(self.inplace)
 
     def __str__(self):
+        #If we are running the inplace version, display that 
+        # so that it is useful for debugging
         if self.inplace:
           return self.__class__.__name__ + '{inplace}'
         return self.__class__.__name__
         
 
+    # Op Contract implementation: make_node:
+    # Should return a Apply object that specifies what:
+    #   1. Input variables type are etc for the operation
+    #   2. What are the types of the output variables
+    #      These should be Theano variables
     def make_node(self, x, y):
+        # x is a sparse matrix, y is a dense one
+        # Wrap them around theano variables as this must be symbolic
         x, y = as_sparse_variable(x), tensor.as_tensor_variable(y)
+
+        # If the types of both variables are of different types
+        # this is bad as theres a type mismatch
         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)
+        #Obtains the indices, indpt, data of NNZ sparse matrix x
         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],
@@ -1789,21 +1815,25 @@ 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]
+        if self.inplace:  #inplace enabled
+          if self.format == 'csc': #column compressed
+            for c in xrange(y.shape[1]): #Loop through each column
+              low = indptr[c] #indptr will pint to slice of indices array for column  
               high = indptr[c+1]
               for ind in xrange(low, high):
-                y[(indices[ind], c)] += data[ind]
+                y[(indices[ind], c)] += data[ind] #Add that data element
           elif self.format == 'csr':
+            #Case for row's. Symmetric to what was done for columns
             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
+
+          out[0] = y #Output storage cell is y
         else:
+          #If in place is not enabled, create back the sparse matrix and 
+          # and just add them normally.
           if self.format == 'csr':
             x = scipy.sparse.csr_matrix( (data,indices,indptr), shape=y.shape)
           elif self.format == 'csc':
@@ -1818,6 +1848,7 @@ class AddSD(gof.op.Op):
         return sp_ones_like(x) * gz, gz
 
     def infer_shape(self, node, shapes):
+        #Shape of output is the shape of y
         return [shapes[3]]
 
 add_s_d = AddSD()

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),

theano/tensor/basic.py

@@ -6479,6 +6479,7 @@ class AdvancedSubtensor1(Op):
 
         assert len(inputs) == 2
 #        rval1 = [advanced_inc_subtensor1(zeros_like(inputs[0]), gz, inputs[1])]
+        #Construct a sparse matrix to boost performance of gradient
         rval1 = [ConstructSparse()(inputs[0], gz, inputs[1])]
 
         return rval1 + [DisconnectedType()()] * (len(inputs) - 1)
@@ -6493,7 +6494,7 @@ class AdvancedSubtensor1(Op):
         return [ilist + x[1:]]
 
 class ConstructSparse(Op):
-    """Construct a sparse matrix out of a list of 2-D matrix rows"""
+    """Construct a sparse CSC matrix out of a list of 2-D matrix rows"""
 
     def __hash__(self):
         return hash((type(self)))
@@ -6504,9 +6505,16 @@ class ConstructSparse(Op):
     def __str__(self):
         return self.__class__.__name__
 
+    # self: this object
+    #  x: x is a dense matrix
+    #  y: y is a dense matrix (small) which has data
+    #  ilist is the list of rows to which we want to copy rows of y into x
+    #  Output must be a sparse representation of x 
     def make_node(self, x, y, ilist):
 
+        #Convert to a sparse matrix, the shape is what is needed
         x_sparse = ssparse.csc_matrix(tuple(x.shape.eval()), dtype=x.dtype)
+        # Wrap into a Theano variable
         x__ = theano.sparse.as_sparse_variable(x_sparse)
 
         x_ = as_tensor_variable(x)
@@ -6528,30 +6536,44 @@ class ConstructSparse(Op):
             ' by y with ndim=%s to x subtensor with ndim=%s ' % (
                 opname, x_.type.ndim, y_.type.ndim))
 
+         # Return the Apply instance
         return Apply(self, [x_, y_, ilist_], [x__.type()])
 
+    # Inp: Will contain x, values: that is y, and list of row indices of X,
+    # we want to copy the rows of y 
     def perform(self, node, inp, out_):
-        x, values, idx = inp
-        out, = out_
-        rows, cols = values.shape
-        assert rows == len(idx)
+        x, values, idx = inp #Get all the 3 inputs
+        out, = out_ #get the output
+        rows, cols = values.shape #Get the shape of Y
+        assert rows == len(idx) #Each row is copied to a row in X.
+
+        # Setup the index pointer array
         indptr = numpy.arange(cols+1) * rows
+
+        #Set up the indices array
         indices = as_strided(idx,
                              strides=(0, idx.strides[0]),
                              shape=(cols, idx.shape[0])).flatten()
+
+        #The data values we need to construct the sparse matrix from
         data = values.T.flatten()
+
+        #Construct the sparse CSC matrix using data, indices and index pointer
         out[0] = ssparse.csc_matrix((data,indices,indptr), shape=x.shape, dtype=x.dtype)
 
+    #Same as advancedIncSubTensor
     def infer_shape(self, node, ishapes):
         x, y, ilist = ishapes
         return [x]
 
+    #Same as advancedIncSubTensor
     def R_op(self, inputs, eval_points):
         if None in eval_points[:2]:
             return [None]
         return self.make_node(eval_points[0], eval_points[1],
                               *inputs[2:]).outputs
 
+    #Same as advancedIncSubTensor
     def connection_pattern(self, node):
 
         rval = [[True], [True]]
@@ -6561,6 +6583,7 @@ class ConstructSparse(Op):
 
         return rval
 
+    #Same as advancedIncSubTensor
     def grad(self, inputs, grads):
         g_output, = grads
         x, y = inputs[:2]