Merge pull request #796 from nouiz/fix_vm_cvm_gc

theano/compile/mode.py

@@ -63,11 +63,11 @@ def check_equal(x, y):
 predefined_linkers = {
     'py': gof.PerformLinker(),
     'c': gof.CLinker(),
-    'c|py': gof.OpWiseCLinker(allow_gc=True),
+    'c|py': gof.OpWiseCLinker(),
     'c|py_nogc': gof.OpWiseCLinker(allow_gc=False),
     'c&py': gof.DualLinker(checker=check_equal),
-    'vm': gof.vm.VM_Linker(allow_gc=True, use_cloop=False),
-    'cvm': gof.vm.VM_Linker(allow_gc=True, use_cloop=True),
+    'vm': gof.vm.VM_Linker(use_cloop=False),
+    'cvm': gof.vm.VM_Linker(use_cloop=True),
     'vm_nogc': gof.vm.VM_Linker(allow_gc=False, use_cloop=False),
     'cvm_nogc': gof.vm.VM_Linker(allow_gc=False, use_cloop=True),
     }

theano/configdefaults.py

@@ -117,6 +117,16 @@ except OSError:
 
 del dummy_stdin
 
+#Keep the default optimizer the same as the one for the mode FAST_RUN
+AddConfigVar('allow_gc',
+             "Do we default to delete intermediate results during Theano"
+             " function call? Doing so lower the memory requirement, but ask"
+             " that we reallocate memory at the next function call."
+             " This is implemented for the default linker, but not work all"
+             " linker",
+             BoolParam(True),
+             in_c_key=False)
+
 #Keep the default optimizer the same as the one for the mode FAST_RUN
 AddConfigVar('optimizer',
         ("Default optimizer. If not None, will use this linker with the Mode "

theano/gof/cc.py

@@ -1395,8 +1395,10 @@ class OpWiseCLinker(link.LocalLinker):
 
     def __init__(self,
             fallback_on_perform=True,
-            allow_gc=True,
+            allow_gc=None,
             nice_errors=True):
+        if allow_gc is None:
+            allow_gc = config.allow_gc
         self.fgraph = None
         self.fallback_on_perform = fallback_on_perform
         self.nice_errors = nice_errors

theano/gof/lazylinker_c.c.txt

@@ -771,7 +771,8 @@ int lazy_rec_eval(CLazyLinker * self, Py_ssize_t var_idx, PyObject*one, PyObject
 
           Py_INCREF(Py_None);
           err = PyList_SetItem(self->var_value_cells[i_idx], 0, Py_None);
-          self->var_computed[i_idx] = 0;
+//See the Stack gc implementation for why we change it to 2 and not 0.
+          self->var_computed[i_idx] = 2;
           if (err) goto fail;
         }
     }
@@ -972,7 +973,7 @@ static PyTypeObject lazylinker_ext_CLazyLinkerType = {
 
 static PyObject * get_version(PyObject *dummy, PyObject *args)
 {
-  PyObject *result = PyFloat_FromDouble(0.17);
+  PyObject *result = PyFloat_FromDouble(0.18);
   return result;
 }
 

theano/gof/lazylinker_c.py

@@ -13,7 +13,7 @@ if config.compiledir not in sys.path:
     sys.path.append(config.compiledir)
 
 force_compile = False
-version = 0.17 # must match constant returned in function get_version()
+version = 0.18 # must match constant returned in function get_version()
 
 
 try:

theano/gof/tests/test_vm.py

@@ -279,3 +279,30 @@ if run_memory_usage_tests:
         time_linker('vmLinker_C', lambda : vm.VM_Linker(allow_gc=False, use_cloop=True))
 
 
+class RunOnce(theano.Op):
+    def __init__(self):
+        self.nb_run = 0
+
+    def make_node(self, x):
+        return theano.Apply(self, [x], [x.type()])
+
+    def perform(self, node, inputs, outputs):
+        assert self.nb_run == 0
+        self.nb_run += 1
+        outputs[0][0] = inputs[0].copy()
+
+
+def test_vm_gc():
+    """This already caused a bug in the trunk of Theano.
+
+    The bug was introduced in the trunk the July 5, 2012 and fixed the
+    July 30
+
+    """
+    pass
+    x = theano.tensor.vector()
+    p = RunOnce()(x)
+    mode = theano.Mode(linker=theano.gof.vm.VM_Linker(lazy=True))
+    f = theano.function([theano.In(x, mutable=True)], [p + 1, p + 2],
+                        mode=mode)
+    f([1, 2, 3])

theano/gof/vm.py

@@ -12,7 +12,7 @@ from theano.gof.python25 import all
 import theano
 config = theano.config
 
-from theano.configparser import config, AddConfigVar, BoolParam
+from theano.configparser import config, AddConfigVar, BoolParam, ConfigParam
 
 logger = logging.getLogger(__name__)
 
@@ -23,6 +23,25 @@ AddConfigVar('profile_optimizer',
         "If VM should collect optimizer profile information",
         BoolParam(False))
 
+
+def filter_vm_lazy(val):
+    if val == 'False' or val is False:
+        return False
+    elif val == 'True' or val is True:
+        return True
+    elif val == 'None':
+        return None
+    else:
+        raise ValueError('Valid values for an vm.lazy parameter '
+                        'should be None, False or True, not `%s`.' % val)
+
+AddConfigVar('vm.lazy',
+             "Useful only for the vm linkers. When lazy is None,"
+             " auto detect if lazy evaluation is needed and use the apropriate"
+             " version. If lazy it True/False, force the version used between"
+             " Loop/LoopGC and Stack.",
+         ConfigParam('None', filter_vm_lazy))
+
 raise_with_op = link.raise_with_op
 
 
@@ -369,7 +388,14 @@ class Stack(VM):
                                 if all(compute_map[v][0]
                                         for v in dependencies[i]):
                                     storage_map[i][0] = None
-                                    compute_map[i][0] = 0
+                                    #DO NOT set compute_map to 0
+
+                                    #If values become False and the
+                                    #current_apply is still in the
+                                    #stack, this will cause it to be
+                                    #recomputed! This can cause wrong value
+                                    #with some combiation of inplace op.
+                                    compute_map[i][0] = 2
                 elif not computed_ins:
                     # -- Non-lazy case, need inputs
                     apply_stack.append(current_apply)
@@ -429,7 +455,9 @@ class Stack(VM):
                                         break
                                 if empty_storage_map:
                                     storage_map[i][0] = None
-                                    compute_map[i][0] = None
+                                    #See the not lazy gc code for explanations
+                                    #Of compute_map change
+                                    compute_map[i][0] = 2
 
         # Hacky coarse gc final pass
         # This is required until we have a proper gc algorithm for graphs with
@@ -464,11 +492,12 @@ class VM_Linker(link.LocalLinker):
     Class that satisfies the Linker interface by acting as a VM factory.
     """
 
-    def __init__(self, allow_gc=True, use_cloop=False, callback=None):
+    def __init__(self, allow_gc=None, use_cloop=False, callback=None, lazy=None):
         """
         allow_gc - force the virtual machine to clean up unnecessary
             references, in order to allow garbage collection on
             intermediate values during computation of a function.
+            If None use as default the Theano flag allow_gc value.
 
         use_cloop - use the C-based virtual machine if possible
 
@@ -476,11 +505,19 @@ class VM_Linker(link.LocalLinker):
             the virtual machine.  It will be called with four arguments called
             'node', 'thunk', 'storage_map', and 'compute_map'.
 
+        lazy - Useful only when use_cloop is False. When lazy is None, auto
+            detect if lazy evaluation is needed and use the apropriate
+            version. If lazy it True/False, force the version used between
+            Loop/LoopGC and Stack.
+
         """
+        if allow_gc is None:
+            allow_gc = config.allow_gc
         self.fgraph = None
         self.allow_gc = allow_gc
         self.use_cloop = use_cloop
         self.callback = callback
+        self.lazy = lazy
         self.updated_vars = {}
 
     def accept(self, fgraph, no_recycling=None):
@@ -674,7 +711,10 @@ class VM_Linker(link.LocalLinker):
                     )
             assert c0 == sys.getrefcount(node_n_inputs)
         else:
-            if all([(not th.lazy) for th in thunks]):
+            lazy = self.lazy
+            if lazy is None:
+                lazy = not all([(not th.lazy) for th in thunks])
+            if not lazy:
                 # there is no conditional in the graph
                 if self.allow_gc:
                     vm = LoopGC(

theano/misc/do_nightly_build

@@ -68,6 +68,11 @@ THEANO_FLAGS=${FLAGS},mode=FAST_RUN,floatX=float32 ${NOSETESTS} ${ARGS}
 echo "Number of elements in the compiledir:"
 ls ${COMPILEDIR}|wc -l
 
+echo "Executing nosetests with mode=vm,vm.lazy=True,floatX=float32"
+THEANO_FLAGS=${FLAGS},mode=vm,vm.lazy=True,floatX=float32 ${NOSETESTS} ${ARGS}
+echo "Number of elements in the compiledir:"
+ls ${COMPILEDIR}|wc -l
+
 #we change the seed and record it everyday to test different combination. We record it to be able to reproduce bug caused by different seed. We don't want multiple test in DEBUG_MODE each day as this take too long.
 seed=$RANDOM
 echo "Executing nosetests with mode=DEBUG_MODE with seed of the day $seed"