TensorFlow Name Conflicts During Meta Object Reification

[brandonwillard]

An issue arises when reifying meta tensors that are intended to be new to a TF graph, because a tensor with an unspecified name value is currently set—by default—to its OpDefs name. This issue should be easily fixed by using the standard TF means of generating a unique name. I'll put in a fix ASAP.

Originally posted by @brandonwillard in https://github.com/pymc-devs/symbolic-pymc/issues/75#issuecomment-560988964

[brandonwillard]

Here's a representative, minimal example of this problem:

import tensorflow as tf

from tensorflow.python.eager.context import graph_mode

from unification import unify, reify, var

from symbolic_pymc.tensorflow.meta import mt

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

    # Divide two constant integers
    first_div_mt = mt(1) / mt(2)

    # Create a TF graph for that division
    first_div_tf = first_div_mt.reify()

    # Create a division "pattern" with variable/unknown dividends and name
    # parameter
    div_lv = mt.realdiv(var('b'), var('c'), name=var('name'))

    # Unify the "pattern" with the TF graph and get the logic variable
    # replacements map/dict
    s = unify(first_div_tf, div_lv)

    # Change the dividends in the substitution map so that the following
    # reification will produce a new new graph
    s[var('b')] = 1
    s[var('b')] = 3

    div_mt = reify(div_lv, s)

    # Attempting to create this TF graph within the same parent graph
    # (i.e. `test_graph`) as `first_div_tf` will throw a
    # `MetaReificationError`, because it would create two graphs with the same
    # name (not possible/expected in TF).
    div_mt.reify()

Before #90, we would allow this, which would result in meta objects with unequal (in name) base objects.

This previous name inconsistency would also lead to unexpected object creation when meta objects were intended to correspond to existing base objects, but some step along the way unintentionally—say—altered a meta object property and reasonably broke the correspondence. In this case, the added name consistency check serves as a more direct means of addressing these scenarios.

The example above is not necessarily inconsistent, though. The problem in that example is, however, easily remedied. Here are two basic approaches:

with graph_mode(), test_graph.as_default():
    # Reify an equivalent "pattern" that uses the same logic variables, save
    # the name, e.g.
    div_mt = reify(mt.realdiv(var('b'), var('c')), s)

    # No problem here
    div_mt.reify()

with graph_mode(), test_graph.as_default():
    # Manually change the name value in the replacements map, e.g.
    s[var('name')] = "new_name"
    div_mt = reify(div_lv, s)

    # No problem here
    div_mt.reify()

Of the two, the first one is most "compatible" with miniKanren, since it can be implemented without resorting to a custom, low-level goal that (non-relationally) manipulates the state/replacement mappings.

Still, it seems like the original example should work in the same way that specifying an existing tensor name does in the TF API (i.e. it creates the next unique name suffixed with an underscore and number).

One way to do this would involve a succinct means of specifying that the names in the "pattern" graph, div_lv, are not a strict assignment.

[brandonwillard]

I've removed the bug label, because I can't find an actual error caused by this behavior. It's really just "unintuitive" at the user-level and will likely trip people up.