merge

theano/compile/debugmode.py

@@ -103,7 +103,25 @@ class BadOptimization(DebugModeError):
 
 class BadDestroyMap(DebugModeError):
     """TODO #318"""
-    pass
+    def __init__(self, node, idx, old_val, new_val):
+        super(BadDestroyMap, self).__init__()
+        self.node = node
+        self.idx = idx
+        self.old_val = old_val
+        self.new_val = new_val
+    
+    def __str__(self):
+        sio = StringIO()
+        print >> sio, "  node:", self.node
+        print >> sio, "  node.inputs:", [(str(i), id(i)) for i in self.node.inputs]
+        print >> sio, "  destroy_map:", getattr(self.node.op, 'destroy_map', {})
+        print >> sio, "  changed input idx:", self.idx
+        print >> sio, "  changed input type:", self.node.inputs[self.idx].type
+        print >> sio, "  old val:", self.old_val
+        print >> sio, "  new val:", self.new_val
+        print >> sio, ""
+        print >> sio, "  Hint: this can be caused by a deficient values_eq_enough() or __eq__() implementation that compares node input values"
+        return sio.getvalue()
 
 class StochasticOrder(DebugModeError):
     """TODO #319"""
@@ -113,6 +131,17 @@ class FloatError(DebugModeError):
     """TODO #320"""
     pass
 
+class InvalidValueError(DebugModeError):
+    """Exception: some Op an output value that is inconsistent with the Type of that output"""
+    def __init__(self, r, v):
+        super(InvalidValueError, self).__init__()
+        self.r = r
+        self.v = v
+
+    def __str__(self):
+        r, v = self.r, self.v
+        return "InvalidValueError: Result %s,  Type %s, type(Value) %s, Value %s"\
+                % (str(r), str(r.type), str(type(v)), str(v)[0:100])
 
 def _debugprint(r, prefix='', depth=-1, done=None, file=sys.stdout):
     """Print the graph leading to `r` to given depth.
@@ -174,6 +203,29 @@ def _optcheck_env(input_specs, output_specs, accept_inplace = False):
     env.extend(Supervisor(input for spec, input in zip(input_specs, inputs) if not (spec.mutable or (hasattr(env, 'destroyers') and env.destroyers(input)))))
     return env, map(SymbolicOutput, updates), equivalence_tracker
 
+def _check_inputs(node, storage_map, r_vals, dr_vals, active_nodes):
+    """Raise BadDestroyMap if necessary, update dr_vals"""
+    destroyed_idx_list = []
+    destroy_map = getattr(node.op, 'destroy_map', {})
+    for o_pos, i_pos_list in destroy_map.iteritems():
+        destroyed_idx_list.extend(i_pos_list)
+    destroyed_res_list = [node.inputs[i] for i in destroyed_idx_list]
+
+    for r_idx, r in enumerate(node.inputs):
+        if not r.type.values_eq_enough(r_vals[r], storage_map[r][0]):
+            # some input node 'r' got changed by running the node
+            # this may or may not be ok...
+            if r in destroyed_res_list:
+                # ok, we expected r to be destroyed
+                if node in active_nodes:
+                    if dr_vals.get(r, (0, node))[1] is not node:
+                        # bad: there should only be one active node that destroys any result
+                        raise Exception('failure in topological ordering')
+                    dr_vals[r] = (storage_map[r][0], node) #no copy, this is the last use of this variable
+            else:
+                raise BadDestroyMap(node, r_idx, r_vals[r], storage_map[r][0])
+
+
 class _EnvEvent(object):
     """A record of an event in the life of an Env.
     
@@ -383,6 +435,9 @@ class _Linker(gof.link.LocalLinker):
         order_outputs.reverse()
         order = graph.io_toposort(env.inputs, order_outputs)
 
+        active_order = env.toposort()  #an ordering of just the active nodes
+        active_order_set = set(active_order)
+
         no_recycling = self.no_recycling
 
         input_storage, output_storage, storage_map = link.map_storage(env, order, input_storage, output_storage)
@@ -459,61 +514,78 @@ class _Linker(gof.link.LocalLinker):
             #
             # This dictionary is used to populate the storage_map as necessary
             r_vals = {} 
+
+            # dr_vals are the values taken by results after being destroyed
+            dr_vals = {}
             assert len(thunks_py) == len(order)
 
-            #put the initial values into the r_vals
+            # transfer the initial values from the storage_map to the r_vals
             for r in storage_map:
                 if storage_map[r][0] is not None:
-                    r_vals[r] = copy.copy(storage_map[r][0])
+                    if r.owner is not None:
+                        # DEBUG
+                        print r, storage_map[r], type(storage_map[r]), id(storage_map[r])
+                    assert r.owner is None
+                    r_vals[r] = storage_map[r][0]
                     storage_map[r][0] = None
+            #####
+            #  Precondition: the storage map is empty, transferred completely to r_vals
+            #####
+            for r, s in storage_map.iteritems():
+                assert s[0] is None
 
             try:
                 # compute the value of all results
                 for i, (thunk_py, thunk_c, node) in enumerate(zip(thunks_py, thunks_c, order)):
+                    this_node_destroyed_results = set()
 
-                    #put a copy of each input into the storage_map
+                    # put a copy of each input into the storage_map
                     for r in node.inputs:
                         storage_map[r][0] = copy.copy(r_vals[r])
 
                     thunk_py()
 
+                    _check_inputs(node, storage_map, r_vals, dr_vals, active_order_set)
+
                     #retrieve a copy of each output from the storage_map
                     for r in node.outputs:
+                        if not r.type.is_valid_value(storage_map[r][0]):
+                            raise InvalidValueError(r, storage_map[r][0])
                         if r in r_vals:
-                            # r has been constant-folded
-                            if not r.type.values_eq_enough(r_vals[r], storage_map[r][0]):
-                                raise DebugModeError('BadConstantFold', (r,  r_vals[r],
-                                    storage_map[r][0])) #TODO: make a proper exception class for this
-
-                        else:
-                            r_vals[r] = copy.copy(storage_map[r][0])
+                            print >> sys.stderr, 'OUTPUT', r, 'ALREADY HAS_VALUE!', r_vals[r], 'WHAT ABOUT', storage_map[r][0]
+                        assert r not in r_vals
+                        r_vals[r] = storage_map[r][0]
+                        storage_map[r][0] = None #clear the storage_map for the thunk_c
 
                     if thunk_c:
-                        for r in node.outputs:
-                            storage_map[r][0] = None #clear the storage_map for the thunk_c
 
-                        if 0:
-                            # TODO: check that Op didn't change any inputs that it wasn't allowed to
-                            # (Hint: use the destroy_map attribute)
-                            raise NotImplementedError()
-                        else:
-                            for r in node.inputs:
-                                storage_map[r][0] = copy.copy(r_vals[r])
+                        for r in node.inputs:
+                            # TODO:  we only need to overwrite the non-destroyed inputs
+                            storage_map[r][0] = copy.copy(r_vals[r])
 
                         thunk_c()
 
+                        _check_inputs(node, storage_map, r_vals, dr_vals, active_order_set)
+
                         for r in node.outputs:
+                            if not r.type.is_valid_value(storage_map[r][0]):
+                                raise InvalidValueError(r, storage_map[r][0])
                             # compares the version from thunk_py (in r_vals)
                             # to the version produced by thunk_c (in storage_map)
                             if not r.type.values_eq_enough(r_vals[r], storage_map[r][0]):
                                 raise BadClinkerOutput(r, val_py=r_vals[r], val_c=storage_map[r][0])
+                            storage_map[r][0] = None #clear the storage_map for the thunk_c
+
+                    # we're done with this thunk
+                    # clear everything out of the storage_map
+                    for r in node.inputs:
+                        storage_map[r][0] = None
 
             except:
                 raise_with_op(node)
 
             # iterate over results looking for values that don't match the values of the
             # results they replaced.  This is the sign of a broken optimization.
-    
             for i, node in enumerate(order):
                 for new_r in node.outputs:
                     for reason, r, old_graph_str, new_graph_str in env.equivalence_tracker.reasons[new_r]:
@@ -532,10 +604,58 @@ class _Linker(gof.link.LocalLinker):
                                     reason=reason,
                                     old_graph=old_graph_str,
                                     new_graph=new_graph_str)
+            
+
+            #####
+            #  Postcondition: the input and output results are in the storage map, nothing more
+            #####
+
+            # Nothing should be in storage map after evaluating each the thunk (specifically the
+            # last one)
+            for r, s in storage_map.iteritems():
+                assert type(s) is list
+                assert s[0] is None
+
+            # store our output results to their respective storage lists
+            for output, storage in zip(env.outputs, output_storage):
+                storage[0] = r_vals[output]
+
+            # transfer all inputs back to their respective storage lists
+            for r in r_vals:
+                if r.owner is None:
+                    if r in env.inputs:
+                        assert storage_map[r] is input_storage[env.inputs.index(r)]
+                    storage_map[r][0] = r_vals[r]
+
+            # if an input was destroyed, the destroyed value should be returned
+            for r in dr_vals:
+                if r.owner is None:
+                    assert r in env.inputs
+                    #HACK TO LOOK LIKE A REAL DESTRUCTIVE ACTION TOOK PLACE
+                    if type(dr_vals[r][0]) is numpy.ndarray \
+                            and dr_vals[r][0].dtype == storage_map[r][0].dtype \
+                            and dr_vals[r][0].shape == storage_map[r][0].shape:
+                        if len(dr_vals[r][0].shape):
+                            storage_map[r][0][:] = dr_vals[r][0]
+                        else:
+                            storage_map[r][0].itemset(dr_vals[r][0])
+                    else:
+                        storage_map[r][0] = dr_vals[r][0]
+            #print ""
+            #print output_storage
+            #print dr_vals
+            #print storage_map
+
+            ###############
+            # Done f
+            ##############
 
         f.allow_gc = True
-        return f, [link.Container(input, storage) for input, storage in zip(env.inputs, input_storage)], \
-            [link.Container(output, storage, True) for output, storage in zip(env.outputs, output_storage)], \
+        assert len(env.inputs) == len(input_storage)
+        assert len(env.outputs) == len(output_storage)
+        #print 'make_all returning output', [id(z) for z in output_storage]
+        return f, [link.Container(input, storage, readonly=False) for input, storage in zip(env.inputs, input_storage)], \
+            [link.Container(output, storage, readonly=True) for output, storage in zip(env.outputs, output_storage)], \
             thunks_py, order
 
 _NODEFAULT = ['NODEFAULT']
@@ -737,6 +857,9 @@ class DebugMode(Mode):
 
     - incomplete `destroy_map` specification (raises `BadDestroyMap`)
 
+    - an op that returns an illegal value not matching the output Result Type (raises
+      InvalidValueError)
+
     Each of these exceptions inherits from the more generic `DebugModeError`.
 
     If there are no internal errors, this mode behaves like FAST_RUN or FAST_COMPILE, but takes

theano/gof/link.py

@@ -163,7 +163,7 @@ class Container(object):
 
 
 def map_storage(env, order, input_storage, output_storage):
-    """Ensure there is storage for inputs, outputs, and interior nodes.
+    """Ensure there is storage (a length-1 list) for inputs, outputs, and interior nodes.
 
     :param env: The current env.  This function uses the inputs and outputs attributes.
     :param order: an iterable over Apply instances (in program running order)

theano/gof/opt.py

@@ -431,7 +431,7 @@ class OpSub(LocalOptimizer):
                 new_output.tag = copy(output.tag)
         return repl.outputs
 
-    def str(self):
+    def __str__(self):
         return "%s -> %s" % (self.op1, self.op2)
 
 
@@ -444,10 +444,6 @@ class OpRemove(LocalOptimizer):
     reentrant = False      # no nodes are added at all
 
     def __init__(self, op):
-        """
-        op1.make_node and op2.make_node must take the same number of
-        inputs and have the same number of outputs.
-        """
         self.op = op
 
     def op_key(self):
@@ -461,7 +457,7 @@ class OpRemove(LocalOptimizer):
             return False
         return node.inputs
 
-    def str(self):
+    def __str__(self):
         return "%s(x) -> x" % (self.op)
 
 

theano/gof/type.py

@@ -218,6 +218,10 @@ class PureType(object):
 
         """
         raise AbstractFunctionError()
+
+    def is_valid_value(self, a):
+        """Required: Return True for any python object `a` that would be a legal value for a Result of this Type"""
+        raise AbstractFunctionError()
     
     def make_result(self, name = None):
         """Return a new `Result` instance of Type `self`.
@@ -325,6 +329,9 @@ class Generic(SingletonType):
     def filter(self, data, strict = False):
         return data
 
+    def is_valid_value(self, a):
+        return True
+
     def c_declare(self, name, sub):
         return """
         PyObject* %(name)s;
@@ -348,6 +355,7 @@ class Generic(SingletonType):
         Py_XINCREF(py_%(name)s);
         """ % locals()
 
+
 generic = Generic()
 
 

theano/scalar/basic.py

@@ -52,6 +52,14 @@ class Scalar(Type):
         except Exception, e:
             raise TypeError("Could not convert %s (value=%s) to %s" % (type(data), data, self.dtype), e)
 
+    def values_eq_enough(self, a, b):
+        return abs(a - b) / (a+b) < 1e-4
+    
+    def is_valid_value(self, a):
+        _a = numpy.asarray(a)
+        rval = (_a.ndim == 0) and (str(_a.dtype) == self.dtype)
+        return rval
+
     def __eq__(self, other):
         return type(self) == type(other) and other.dtype == self.dtype
 

theano/sparse/basic.py

@@ -9,6 +9,7 @@ To read about different sparse formats, see U{http://www-users.cs.umn.edu/~saad/
 import sys, operator
 import numpy
 from scipy import sparse
+import scipy.sparse
 
 from .. import gof
 from .. import tensor
@@ -185,6 +186,14 @@ class Sparse(gof.Type):
     def __repr__(self):
         return "Sparse[%s, %s]" % (str(self.dtype), str(self.format))
 
+    def values_eq_enough(self, a, b, eps=1e-6):
+        return scipy.sparse.issparse(a) \
+                and scipy.sparse.issparse(b) \
+                and abs(a-b).sum() < (1e-6 * a.nnz)
+
+    def is_valid_value(self, a):
+        return scipy.sparse.issparse(a) and (a.format == self.format)
+
 csc_matrix = Sparse(format='csc')
 csr_matrix = Sparse(format='csr')
 

theano/tensor/basic.py

@@ -226,6 +226,17 @@ class Tensor(Type):
         return type(a) is numpy.ndarray and type(b) is numpy.ndarray \
                 and (a.shape == b.shape) and numpy.allclose(a, b)
 
+    def is_valid_value(self, a):
+        rval = (type(a) is numpy.ndarray) and (self.ndim == a.ndim) \
+                and (str(a.dtype) == self.dtype) \
+                and all([((si == 1) or not bi) for si, bi in zip(a.shape, self.broadcastable)])
+        if not rval:
+            print type(a),(type(a) is numpy.ndarray)
+            print a.ndim, (self.ndim == a.ndim)
+            print a.dtype, (str(a.dtype) == self.dtype)
+            print a.shape, self.broadcastable, ([(shp_i == 1) for shp_i in a.shape] == self.broadcastable)
+        return rval
+
     def __hash__(self):
         """Hash equal for same kinds of Tensor"""
         return hash(self.dtype) ^ hash(self.broadcastable)

theano/tensor/tests/test_basic.py

@@ -114,8 +114,8 @@ def make_restet(name, op, expected, checks = {}, good = {}, bad_build = {}, bad_
 
                 for description, check in self.checks.items():
                     if not check(inputs, results):
-                        self.fail("Test %s::%s: Failed check: %s (inputs were %s)"
-                                  % (self.op, testname, description, inputs))
+                        self.fail("Test %s::%s: Failed check: %s (inputs were %s, outputs were %s)"
+                                  % (self.op, testname, description, inputs, results))
 
         def test_bad_build(self):
             for testname, inputs in self.bad_build.items():
@@ -195,8 +195,11 @@ def make_broadcast_restet(op, expected, checks = {}, **kwargs):
         if kwargs['inplace']:
             _expected = expected
             expected = lambda *inputs: numpy.array(_expected(*inputs), dtype = inputs[0].dtype)
-            checks = dict(checks,
-                          inplace_check = lambda inputs, outputs: inputs[0] is outputs[0])
+            def inplace_check(inputs, outputs):
+                # this used to be inputs[0] is output[0]
+                # I changed it so that it was easier to satisfy by the DebugMode
+                return numpy.all(inputs[0] == outputs[0])
+            checks = dict(checks, inplace_check=inplace_check) #lambda inputs, outputs: numpy.all(inputs[0] == outputs[0]))
         del kwargs['inplace']
     return make_restet(name, op, expected, checks, **kwargs)
 

theano/tensor/tests/test_blas.py

@@ -141,9 +141,10 @@ class t_gemm(TestCase):
         """test that dot args can be aliased"""
         Z = value(self.rand(2,2))
         A = value(self.rand(2,2))
-        eval_outputs([gemm(Z, 1.0, A, A, 1.0)])
-        eval_outputs([gemm(Z, 1.0, A, A.T, 1.0)])
-
+        f = inplace_func([A,Z], gemm(Z, 1.0, A, A, 1.0))
+        f(A.data, Z.data)
+        f = inplace_func([A,Z], gemm(Z, 1.0, A, A.T, 1.0))
+        f(A.data, Z.data)
 
     def test_transposes(self):
         # three square matrices which are not contiguous