Merge pull request #1180 from lamblin/fix_infinite_opt_loop

theano/compile/debugmode.py

@@ -677,8 +677,13 @@ def _optcheck_fgraph(input_specs, output_specs, accept_inplace=False):
     inputs, outputs = gof.graph.clone(orig_inputs, orig_outputs)
     equivalence_tracker = _VariableEquivalenceTracker()
     fgraph = gof.fg.FunctionGraph(inputs, outputs,
-            #DestroyHandler is not needed because it is actually installed by an optimization
-            # after canonicalization.  This variables in a big speed gain.
+            # DestroyHandler may not be needed yet, as there is usually no
+            # inplace operation in the graph at this stage. DestroyHandler
+            # will be installed by an optimization after canonicalization,
+            # before the inplace operations are applied.
+            # This results in a big speed gain.
+            # If inplace operations are accepted and present, however,
+            # DestroyHandler will be inserted in the loop below.
             #features=[equivalence_tracker, gof.DestroyHandler(do_imports_on_attach=False)])
             features=[equivalence_tracker])
 
@@ -687,6 +692,13 @@ def _optcheck_fgraph(input_specs, output_specs, accept_inplace=False):
             if getattr(node.op, 'destroy_map', None):
                 raise TypeError("Graph must not contain inplace operations",
                                 node)
+    else:
+        # However, if some inplace ops are already in the graph,
+        # DestroyHandler is needed for the Supervisor below to work correctly.
+        for node in fgraph.apply_nodes:
+            if getattr(node.op, 'destroy_map', None):
+                fgraph.attach_feature(gof.DestroyHandler())
+                break
 
     # We need to protect all immutable inputs from inplace operations.
     fgraph.attach_feature(Supervisor(input for spec, input in zip(input_specs, inputs)

theano/compile/function_module.py

@@ -141,7 +141,12 @@ def std_fgraph(input_specs, output_specs, accept_inplace = False):
                 break
 
     # We need to protect all immutable inputs from inplace operations.
-    fgraph.attach_feature(Supervisor(input for spec, input in zip(input_specs, inputs) if not (spec.mutable or (hasattr(fgraph, 'destroyers') and fgraph.destroyers(input)))))
+    fgraph.attach_feature(
+            Supervisor(input
+                for spec, input in zip(input_specs, inputs)
+                if not (spec.mutable or
+                        (hasattr(fgraph, 'destroyers') and
+                            fgraph.destroyers(input)))))
 
     # If named nodes are replaced, keep the name
     for feature in std_fgraph.features:

theano/gof/opt.py

@@ -1411,13 +1411,17 @@ class EquilibriumOptimizer(NavigatorOptimizer):
         max_use_abort = False
         opt_name = None
         process_count = {}
-        max_nb_nodes = 0
+        max_nb_nodes = len(fgraph.apply_nodes)
+        max_use = max_nb_nodes * self.max_use_ratio
 
         loop_timing = []
         global_opt_timing = []
         time_lopts = {}
         io_toposort_timing = []
         nb_nodes = []
+        for gopt in self.global_optimizers:
+            process_count.setdefault(gopt, 0)
+
         for lopt in self.local_optimizers:
             process_count.setdefault(lopt, 0)
             time_lopts.setdefault(lopt, 0)
@@ -1426,12 +1430,17 @@ class EquilibriumOptimizer(NavigatorOptimizer):
             t0 = time.time()
             changed = False
 
-            #apply global optimizer
-            fgraph.change_tracker.reset()
+            #apply global optimizers
             for gopt in self.global_optimizers:
+                fgraph.change_tracker.reset()
                 gopt.apply(fgraph)
                 if fgraph.change_tracker.changed:
+                    process_count[gopt] += 1
                     changed = True
+                    if process_count[gopt] > max_use:
+                        max_use_abort = True
+                        opt_name = (getattr(gopt, "name", None)
+                                    or getattr(gopt, "__name__", ""))
 
             global_opt_timing.append(float(time.time() - t0))
 

theano/tensor/tests/test_blas.py

@@ -258,8 +258,8 @@ class t_gemm(TestCase):
 
     def test_destroy_map4(self):
         """test that dot args can be aliased"""
-        Z = shared(self.rand(2, 2))
-        A = shared(self.rand(2, 2))
+        Z = shared(self.rand(2, 2), name='Z')
+        A = shared(self.rand(2, 2), name='A')
         one = T.constant(1.0).astype(Z.dtype)
         f = inplace_func([], gemm_inplace(Z, one, A, A, one))
         f()