Skip to content

Conversation

@brandonwillard
Copy link
Contributor

The TFlowOpName string type was an attempt to "smooth out" some of the name-uniqueness constraints in TF (e.g. so that equality could exist between a meta object with the name "value:0" and a recreated base object with an automatically generated name like "blah/value_1:0"). While sometimes convenient, this isn't the right approach, since it causes problems when the ignored name distinctions are actually intended to correspond to distinct base objects.

@brandonwillard brandonwillard added bug Something isn't working important Features and issues that need to be addressed ASAP labels Oct 14, 2019
@brandonwillard brandonwillard self-assigned this Oct 14, 2019
@brandonwillard
Copy link
Contributor Author

Working on this has brought up something that needs addressing: the connection between meta objects and their names is not one-to-one with the TF graphs in which meta objects create their corresponding reified base objects.

For example, consider the following:

import tensorflow as tf

from tensorflow.python.eager.context import graph_mode

from symbolic_pymc.tensorflow.meta import mt, TFlowMetaNodeDef, TFlowMetaOp


with graph_mode(), tf.Graph().as_default() as test_graph:

    add_tf = tf.add(1, 1)

    assert add_tf.name == 'Add:0'

with graph_mode(), test_graph.as_default():

    # This could be a meta version of `add_tf.op.node_def`
    node_def_mt = TFlowMetaNodeDef('Add', 'Add', {})

    assert mt(add_tf.op.node_def) == node_def_mt

    # This could be a meta version of `add_tf.op`
    inputs_mt = mt(add_tf.op.inputs)
    test_op_mt = TFlowMetaOp(mt.Add, node_def_mt, inputs_mt)

    assert mt(add_tf.op) == test_op_mt

In this example, we've created a TF Add node, add_tf, and created—by hand—a meta NodeDef and meta Operation that could be meta versions of add_tf's constituent Operation and NodeDef base objects.

Now, when we reify the meta Operation, test_op_mt, we find that the base object it produces does not correspond exactly to the underlying meta object, because its name values are different.

with graph_mode(), test_graph.as_default():

    test_op_tf = test_op_mt.reify()

    # The reified, base/TF `Operation` object has a different name!
    assert test_op_mt.name != test_op_tf.name

This discrepancy is due to the fact that test_graph already has a node with the name Add (i.e. the original add_tf's Operation).

One solution: we could check for elements with the desired/specified meta object names in the current TF graph and verify that the proposed meta objects are in-line with those—and, say, raise exceptions when they aren't. At the very least, such a process could save some effort (re)constructing TF objects that already exist and/or better imputation of unspecified information in meta objects (e.g. dtypes and shapes).

Also, TFlowMetaNodeDef needs a custom reify method; the base class method currently does not work for this type.

@brandonwillard brandonwillard merged commit a68a2a9 into pymc-devs:master Oct 14, 2019
@brandonwillard brandonwillard deleted the remove-tflowopname branch October 14, 2019 22:43
@brandonwillard brandonwillard added meta graph This issue involves the meta graph objects TensorFlow This issue involves the TensorFlow backend labels Mar 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working important Features and issues that need to be addressed ASAP meta graph This issue involves the meta graph objects TensorFlow This issue involves the TensorFlow backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant