Merge pull request #110 from brandonwillard/enforce-sane-test-values

Refactor test value framework so that test value validation is performed up-front.

Author: brandonwillard

doc/library/misc/pkl_utils.txt

@@ -15,10 +15,6 @@
 
 .. autoclass:: theano.misc.pkl_utils.StripPickler
 
-.. autoclass:: theano.misc.pkl_utils.CompatUnpickler
-
 .. seealso::
 
     :ref:`tutorial_loadsave`
-
-

tests/gof/test_compute_test_value.py

@@ -167,14 +167,16 @@ def test_constant(self):
 
     @theano.change_flags(compute_test_value="raise")
     def test_incorrect_type(self):
-        x = tt.fmatrix("x")
-        # Incorrect dtype (float64) for test_value
-        x.tag.test_value = np.random.rand(3, 4)
-        y = tt.dmatrix("y")
-        y.tag.test_value = np.random.rand(4, 5)
 
+        x = tt.vector("x")
         with pytest.raises(TypeError):
-            tt.dot(x, y)
+            # Incorrect shape for test value
+            x.tag.test_value = np.empty((2, 2))
+
+        x = tt.fmatrix("x")
+        with pytest.raises(TypeError):
+            # Incorrect dtype (float64) for test value
+            x.tag.test_value = np.random.rand(3, 4)
 
     @theano.change_flags(compute_test_value="raise")
     def test_overided_function(self):

tests/gof/test_fg.py

@@ -1,12 +1,7 @@
-import os
 import pickle
 
-import pytest
-
-import theano
-from theano.compat import PY3
-from theano.gof.fg import FunctionGraph
 from theano import tensor as tt
+from theano.gof.fg import FunctionGraph
 
 
 class TestFunctionGraph:
@@ -16,24 +11,3 @@ def test_pickle(self):
 
         s = pickle.dumps(func)
         pickle.loads(s)
-
-    @pytest.mark.skipif(
-        not theano.config.cxx, reason="G++ not available, so we need to skip this test."
-    )
-    @pytest.mark.slow
-    def test_node_outputs_not_used(self):
-        # In the past, we where removing some not used variable from
-        # fgraph.variables event if the apply had other output used in
-        # the graph. This caused a crash.
-        # This test run the pickle that reproduce this case.
-        with open(
-            os.path.join(os.path.dirname(__file__), "test_fg_old_crash.pkl"), "rb"
-        ) as f:
-            from theano.misc.pkl_utils import CompatUnpickler
-
-            if PY3:
-                u = CompatUnpickler(f, encoding="latin1")
-            else:
-                u = CompatUnpickler(f)
-            d = u.load()
-        f = theano.function(**d)

tests/gof/test_fg_old_crash.pkl

@@ -1,14 +1,14 @@
 import numpy as np
 import pytest
-
 import theano
 import theano.gof.op as op
+import theano.tensor as tt
+
 from six import string_types
-from theano.gof.type import Type, Generic
+from theano import scalar, shared
+from theano.configparser import change_flags
 from theano.gof.graph import Apply, Variable
-import theano.tensor as T
-from theano import scalar
-from theano import shared
+from theano.gof.type import Generic, Type
 
 config = theano.config
 Op = op.Op
@@ -238,15 +238,15 @@ class DoubleOp(Op):
 
             __props__ = ()
 
-            itypes = [T.dmatrix]
-            otypes = [T.dmatrix]
+            itypes = [tt.dmatrix]
+            otypes = [tt.dmatrix]
 
             def perform(self, node, inputs, outputs):
                 inp = inputs[0]
                 output = outputs[0]
                 output[0] = inp * 2
 
-        x_input = T.dmatrix("x_input")
+        x_input = tt.dmatrix("x_input")
         f = theano.function([x_input], DoubleOp()(x_input))
         inp = np.random.rand(5, 4)
         out = f(inp)
@@ -255,17 +255,17 @@ def perform(self, node, inputs, outputs):
 
 def test_test_value_python_objects():
     for x in ([0, 1, 2], 0, 0.5, 1):
-        assert (op.get_test_value(x) == x).all()
+        assert np.all(op.get_test_value(x) == x)
 
 
 def test_test_value_ndarray():
     x = np.zeros((5, 5))
     v = op.get_test_value(x)
-    assert (v == x).all()
+    assert np.all(v == x)
 
 
 def test_test_value_constant():
-    x = T.as_tensor_variable(np.zeros((5, 5)))
+    x = tt.as_tensor_variable(np.zeros((5, 5)))
     v = op.get_test_value(x)
 
     assert np.all(v == np.zeros((5, 5)))
@@ -278,62 +278,37 @@ def test_test_value_shared():
     assert np.all(v == np.zeros((5, 5)))
 
 
+@change_flags(compute_test_value="raise")
 def test_test_value_op():
-    try:
-        prev_value = config.compute_test_value
-        config.compute_test_value = "raise"
-        x = T.log(np.ones((5, 5)))
-        v = op.get_test_value(x)
-
-        assert np.allclose(v, np.zeros((5, 5)))
-    finally:
-        config.compute_test_value = prev_value
-
 
-def test_get_debug_values_no_debugger():
-    "get_debug_values should return [] when debugger is off"
+    x = tt.log(np.ones((5, 5)))
+    v = op.get_test_value(x)
 
-    prev_value = config.compute_test_value
-    try:
-        config.compute_test_value = "off"
+    assert np.allclose(v, np.zeros((5, 5)))
 
-        x = T.vector()
 
-        for x_val in op.get_debug_values(x):
-            assert False
+@change_flags(compute_test_value="off")
+def test_get_debug_values_no_debugger():
+    """Tests that `get_debug_values` returns `[]` when debugger is off."""
 
-    finally:
-        config.compute_test_value = prev_value
+    x = tt.vector()
+    assert op.get_debug_values(x) == []
 
 
+@change_flags(compute_test_value="ignore")
 def test_get_det_debug_values_ignore():
-    # get_debug_values should return [] when debugger is ignore
-    # and some values are missing
+    """Tests that `get_debug_values` returns `[]` when debugger is set to "ignore" and some values are missing."""
 
-    prev_value = config.compute_test_value
-    try:
-        config.compute_test_value = "ignore"
-
-        x = T.vector()
-
-        for x_val in op.get_debug_values(x):
-            assert False
-
-    finally:
-        config.compute_test_value = prev_value
+    x = tt.vector()
+    assert op.get_debug_values(x) == []
 
 
 def test_get_debug_values_success():
-    # tests that get_debug_value returns values when available
-    # (and the debugger is on)
+    """Tests that `get_debug_value` returns values when available (and the debugger is on)."""
 
-    prev_value = config.compute_test_value
     for mode in ["ignore", "warn", "raise"]:
-
-        try:
-            config.compute_test_value = mode
-
-            x = T.vector()
+        with change_flags(compute_test_value=mode):
+            x = tt.vector()
             x.tag.test_value = np.zeros((4,), dtype=config.floatX)
             y = np.zeros((5, 5))
 
@@ -348,54 +323,11 @@ def test_get_debug_values_success():
 
             assert iters == 1
 
-        finally:
-            config.compute_test_value = prev_value
-
 
+@change_flags(compute_test_value="raise")
 def test_get_debug_values_exc():
-    # tests that get_debug_value raises an exception when
-    # debugger is set to raise and a value is missing
-
-    prev_value = config.compute_test_value
-    try:
-        config.compute_test_value = "raise"
-
-        x = T.vector()
-
-        try:
-            for x_val in op.get_debug_values(x):
-                # this assert catches the case where we
-                # erroneously get a value returned
-                assert False
-            raised = False
-        except AttributeError:
-            raised = True
-
-        # this assert catches the case where we got []
-        # returned, and possibly issued a warning,
-        # rather than raising an exception
-        assert raised
+    """Tests that `get_debug_value` raises an exception when debugger is set to raise and a value is missing."""
 
-    finally:
-        config.compute_test_value = prev_value
-
-
-def test_debug_error_message():
-    # tests that debug_error_message raises an
-    # exception when it should.
-
-    prev_value = config.compute_test_value
-
-    for mode in ["ignore", "raise"]:
-
-        try:
-            config.compute_test_value = mode
-
-            try:
-                op.debug_error_message("msg")
-                raised = False
-            except ValueError:
-                raised = True
-            assert raised
-        finally:
-            config.compute_test_value = prev_value
+    with pytest.raises(AttributeError):
+        x = tt.vector()
+        assert op.get_debug_values(x) == []

tests/gof/test_op.py

@@ -7,9 +7,10 @@
 
 import tests.unittest_tools as utt
 
+from pickle import Unpickler
+
 from theano import config, function, tensor
 from theano.compat import PY3
-from theano.misc.pkl_utils import CompatUnpickler
 from theano.sandbox import multinomial
 from theano.sandbox.rng_mrg import MRG_RandomStreams as RandomStreams
 from theano.gpuarray.multinomial import (
@@ -384,6 +385,6 @@ def test_unpickle_legacy_op():
 
     if not PY3:
         with open(os.path.join(testfile_dir, fname), "r") as fp:
-            u = CompatUnpickler(fp)
+            u = Unpickler(fp)
             m = u.load()
             assert isinstance(m, GPUAChoiceFromUniform)

tests/gpuarray/test_multinomial.py

@@ -13,9 +13,9 @@
 
 import numpy as np
 
+from pickle import Unpickler
+
 from theano import config
-from theano.compat import PY3
-from theano.misc.pkl_utils import CompatUnpickler
 
 from theano.gpuarray.type import ContextNotDefined
 
@@ -37,10 +37,7 @@ def test_unpickle_gpuarray_as_numpy_ndarray_flag1():
         fname = "GpuArray.pkl"
 
         with open(os.path.join(testfile_dir, fname), "rb") as fp:
-            if PY3:
-                u = CompatUnpickler(fp, encoding="latin1")
-            else:
-                u = CompatUnpickler(fp)
+            u = Unpickler(fp, encoding="latin1")
             with pytest.raises((ImportError, ContextNotDefined)):
                 u.load()
     finally:
@@ -56,10 +53,7 @@ def test_unpickle_gpuarray_as_numpy_ndarray_flag2():
         fname = "GpuArray.pkl"
 
         with open(os.path.join(testfile_dir, fname), "rb") as fp:
-            if PY3:
-                u = CompatUnpickler(fp, encoding="latin1")
-            else:
-                u = CompatUnpickler(fp)
+            u = Unpickler(fp, encoding="latin1")
             try:
                 mat = u.load()
             except ImportError:

tests/gpuarray/test_pickle.py

@@ -5,10 +5,10 @@
 
 pygpu = pytest.importorskip("pygpu")
 
-from theano.compat import PY3
+from pickle import Unpickler
+
 from theano import config
 from theano.compile import DeepCopyOp, Rebroadcast, ViewOp
-from theano.misc.pkl_utils import CompatUnpickler
 from theano.gpuarray.type import GpuArrayType, gpuarray_shared_constructor
 
 from tests.gpuarray.config import test_ctx_name
@@ -122,10 +122,7 @@ def test_unpickle_gpuarray_as_numpy_ndarray_flag0():
         fname = "GpuArray.pkl"
 
         with open(os.path.join(testfile_dir, fname), "rb") as fp:
-            if PY3:
-                u = CompatUnpickler(fp, encoding="latin1")
-            else:
-                u = CompatUnpickler(fp)
+            u = Unpickler(fp, encoding="latin1")
             mat = u.load()
             assert isinstance(mat, pygpu.gpuarray.GpuArray)
             assert np.asarray(mat)[0] == -42.0