Remove TFLowOpName

Author: brandonwillard

symbolic_pymc/tensorflow/meta.py

@@ -8,7 +8,7 @@
 
 from inspect import Parameter, Signature
 
-from collections import OrderedDict, UserString
+from collections import OrderedDict
 from collections.abc import Sequence
 
 from functools import partial
@@ -144,51 +144,6 @@ def get_op_info(cls, opdef):
 op_def_lib = MetaOpDefLibrary()
 
 
-class TFlowOpName(UserString):
-    """A wrapper for Tensor names.
-
-    TF `Operation` names, and the variables that result from them, cannot be
-    compared directly due to their uniqueness (within a TF namespace/scope).
-
-    This wrapper class ignores those TF distinctions during string comparison.
-    """
-
-    def __init__(self, s):
-        super().__init__(s)
-
-        if isinstance(s, type(self)):
-            self._scope_op = s._scope_op
-            self._in_idx = s._in_idx
-            self._scope = s._scope
-            self._op_name = s._op_name
-            self._unique_name = s._unique_name
-        else:
-            self._scope_op, _, self._in_idx = self.data.partition(":")
-            self._scope, _, self._op_name = self._scope_op.rpartition("/")
-            self._unique_name = self._op_name.split("_", 1)[0]
-
-    def __eq__(self, other):
-        if self is other:
-            return True
-
-        if isinstance(other, str):
-            return self.data == other or self._unique_name == other
-
-        if type(self) != type(other):
-            return False
-
-        return (
-            self._unique_name.lower() == other._unique_name.lower()
-            and self._in_idx == other._in_idx
-        )
-
-    def __hash__(self):
-        return hash((self._unique_name, self._in_idx))
-
-
-_metatize.add((TFlowOpName,), lambda x: x)
-
-
 def _metatize_tf_object(obj):
     try:
         obj = tf.convert_to_tensor(obj)
@@ -406,7 +361,7 @@ def __init__(self, op, name, attr, obj=None):
         super().__init__(obj=obj)
         self.op = metatize(op)
         assert name is not None
-        self.name = name if isvar(name) else TFlowOpName(name)
+        self.name = name if isvar(name) else str(name)
 
         if not isvar(attr):
             # We want to limit the attributes we'll consider to those that show
@@ -549,7 +504,7 @@ def name(self):
         if isvar(self.node_def):
             self._name = var()
         else:
-            self._name = TFlowOpName(self.node_def.name)
+            self._name = str(self.node_def.name)
 
         return self._name
 

tests/tensorflow/test_meta.py

@@ -1,3 +1,9 @@
+"""
+If you're debugging/running tests manually, it might help to simply
+disable eager execution entirely:
+
+    > tf.compat.v1.disable_eager_execution()
+"""
 import pytest
 import numpy as np
 
@@ -14,36 +20,13 @@
                                            TFlowMetaOp,
                                            TFlowMetaOpDef,
                                            TFlowMetaNodeDef,
-                                           TFlowOpName,
                                            MetaOpDefLibrary,
                                            mt)
 
 from tests.tensorflow import run_in_graph_mode
 from tests.tensorflow.utils import assert_ops_equal
 
 
-@pytest.mark.usefixtures("run_with_tensorflow")
-def test_op_names():
-    """Make sure equality is flexible for `Operation`/`OpDef` names."""
-    # Against a string, only the distinct operator-name part matters
-    assert TFlowOpName('add_1') == 'add'
-    assert TFlowOpName('blah/add_1:0') == 'add'
-    assert TFlowOpName('blah/add_1:0') != 'add_1'
-    assert TFlowOpName('blah/add_1:0') != 'add:0'
-    # Unless it's the whole thing
-    assert TFlowOpName('blah/add_1:0') == 'blah/add_1:0'
-
-    # Ignore namespaces
-    assert TFlowOpName('blah/add_1:0') == TFlowOpName('add:0')
-    assert TFlowOpName('blah/add_1:0') == TFlowOpName('agh/add_1:0')
-    # and "unique" operator names (for the same operator "type")
-    assert TFlowOpName('blah/add_1:0') == TFlowOpName('add_2:0')
-    # but not output numbers
-    assert TFlowOpName('blah/add_1:0') != TFlowOpName('blah/add:1')
-
-    assert isinstance(mt(TFlowOpName('blah/add_1:0')), TFlowOpName)
-
-
 @pytest.mark.usefixtures("run_with_tensorflow")
 def test_meta_helper():
     """Make sure the helper/namespace emulator can find `OpDef`s and create their meta objects."""
@@ -79,14 +62,69 @@ def test_meta_eager():
 
 @pytest.mark.usefixtures("run_with_tensorflow")
 @run_in_graph_mode
-def test_meta_create():
+def test_meta_basic():
+
+    var_mt = TFlowMetaTensor(var(), var(), var())
+    # It should generate a logic variable for the name and use from here on.
+    var_name = var_mt.name
+    assert isvar(var_name)
+    assert var_mt.name is var_name
+    # Same for a tensor shape
+    var_shape = var_mt.shape
+    assert isinstance(var_shape, TFlowMetaTensorShape)
+    assert isvar(var_shape.dims)
+
+    # This essentially logic-variabled tensor should not reify; it should
+    # create a distinct/new meta object that's either equal to the original
+    # meta object or partially reified.
+    assert var_mt.reify() == var_mt
+
+    # This operator is reifiable
+    # NOTE: Const objects are automatically created for the constant inputs, so
+    # we need to do this in a new graph to make sure that their auto-generated
+    # names are consistent throughout runs.
+    with tf.Graph().as_default() as test_graph:
+        test_op = TFlowMetaOp(mt.Add, TFlowMetaNodeDef('Add', 'Add', {}), [1, 0])
+
+        # This tensor has an "unknown"/logic variable output index and dtype, but,
+        # since the operator fully specifies it, reification should still work.
+        var_mt = TFlowMetaTensor(test_op, var(), var())
+
+        # This should be partially reified
+        var_tf = var_mt.reify()
+
+        assert isinstance(var_tf, tf.Tensor)
+
+        # These shouldn't be equal, since `var_mt` has logic variables for
+        # output index and dtype.  (They should be unifiable, though.)
+        assert mt(var_tf) != var_mt
+
+        # NOTE: The operator name specified by the meta NodeDef *can* be
+        # different from the reified TF tensor (e.g. when meta objects are
+        # created/reified within a graph already using the NodeDef-specified
+        # name).
+        #
+        # TODO: We could search for existing TF objects in the current graph by
+        # name and raise exceptions when the desired meta information and name
+        # do not correspond--effectively making the meta object impossible to
+        # reify in said graph.
+
+    # Next, we convert an existing TF object into a meta object
+    # and make sure everything corresponds between the two.
     N = 100
     X = np.vstack([np.random.randn(N), np.ones(N)]).T
+
     X_tf = tf.convert_to_tensor(X)
-    X_mt = mt(X)
+
+    with tf.Graph().as_default() as test_graph:
+        X_mt = mt(X)
 
     assert isinstance(X_mt, TFlowMetaTensor)
-    assert X_mt.op.obj.name.startswith('Const')
+    assert X_mt.op.obj.name == 'Const'
+    assert not hasattr(X_mt, '_name')
+    assert X_mt.name == 'Const:0'
+    assert X_mt._name == 'Const:0'
+
     # Make sure `reify` returns the cached base object.
     assert X_mt.reify() is X_mt.obj
     assert isinstance(X_mt.reify(), tf.Tensor)
@@ -373,7 +411,12 @@ def test_nodedef():
     # `ytest_mt.inputs` should have two `.attr` values that are Python
     # primitives (i.e. int and bool); these shouldn't get metatized and break
     # our ability to reconstruct the object from its rator + rands.
-    assert y_test_mt == y_test_mt.op.op_def(*y_test_mt.inputs)
+    y_test_new_mt = y_test_mt.op.op_def(*y_test_mt.inputs)
+
+    # We're changing this simply so we can use ==
+    y_test_new_mt.op.node_def.name = 'y'
+
+    assert y_test_mt == y_test_new_mt
 
 
 @pytest.mark.usefixtures("run_with_tensorflow")

tests/tensorflow/test_unify.py

@@ -4,7 +4,7 @@
 
 from unification import unify, reify, var
 
-from symbolic_pymc.tensorflow.meta import (TFlowOpName, mt, TFlowMetaTensorShape, TFlowOpName)
+from symbolic_pymc.tensorflow.meta import (mt, TFlowMetaTensorShape)
 from symbolic_pymc.etuple import (ExpressionTuple, etuple, etuplize)
 
 from tests.tensorflow import run_in_graph_mode
@@ -15,8 +15,6 @@
 @run_in_graph_mode
 def test_etuple_term():
 
-    str_type = TFlowOpName("blah")
-    assert etuplize(str_type, return_bad_args=True) == str_type
     assert etuplize("blah", return_bad_args=True) == "blah"
 
     a = tf.compat.v1.placeholder(tf.float64, name='a')
@@ -42,13 +40,13 @@ def test_etuple_term():
     assert test_e[2] is a_mt.op.node_def.attr['shape']
 
     test_e._eval_obj = ExpressionTuple.null
-    a_evaled = test_e.eval_obj
+    with tf.Graph().as_default():
+        a_evaled = test_e.eval_obj
     assert all([a == b for a, b in zip(a_evaled.rands(), a_mt.rands())])
 
     a_reified = a_evaled.reify()
     assert isinstance(a_reified, tf.Tensor)
     assert a_reified.shape.dims is None
-    assert TFlowOpName(a_reified.name) == TFlowOpName(a.name)
 
     e2 = mt.add(a, b)
     e2_et = etuplize(e2)
@@ -75,8 +73,10 @@ def test_basic_unify_reify():
     test_base_res = test_reify_res.reify()
     assert isinstance(test_base_res, tf.Tensor)
 
-    expected_res = tf.add(tf.constant(1, dtype=tf.float64),
-                          tf.constant(2, dtype=tf.float64) * a)
+    with tf.Graph().as_default():
+        a = tf.compat.v1.placeholder(tf.float64, name='a')
+        expected_res = tf.add(tf.constant(1, dtype=tf.float64),
+                              tf.multiply(tf.constant(2, dtype=tf.float64), a))
     assert_ops_equal(test_base_res, expected_res)
 
     # Simply make sure that unification succeeds