Description
This issue tracks getting rid of tensorboard.compat.tf
, which was a shim mechanism inserted for expediency as a centralized way to swap out TF dependencies throughout TensorBoard with a partial stub implementation sufficient to use TensorBoard without TF (as part of #1453).
This was never meant to be long-term functionality; the intention was to eliminate it on a case-by-case basis, either by changing our code to either use the stub all the time (e.g. in cases where we were simply copying a small utility from TensorFlow), or by defining an abstract interface for our code to use and then having both TF and non-TF implementations of that interface (e.g. for filesystem support, where we prefer using TF when available since it supports more filesystems that the stub does).
Unfortunately, this shim not only stuck around, I also allowed it to grow new uses in the summary APIs:
-
Add TF 2.0 summary writing APIs for scalar/text/image/audio/histogram #1696 implemented the v2 summary APIs, which used new logic in
tensorboard.compat
to find TF 2 if available or raise ImportError otherwise (since the summary APIs needed real TF APIs and couldn't fall back to the TF stub, this wasn't actually an optional dependency). Because the v2 APIs were pulled into the same package namespace (tensorboard.summary
) as the existing v1 APIs, and at that point we still had to remain compatible with versions of TF that didn't include a v2 API at all, the v2 summary APIs were made to invoke thetensorboard.compat
TF 2 import logic at the time they were called rather than at import time. -
Make tensorboard.compat.{tf,tf2} lazily loaded #1781 added lazy-loading to
tensorboard.compat.{tf,tf2}
to attempt to avoid circular dependency issues that arose when importing the v2 summary APIs within TensorFlow. (At this point adding the automatic lazy-loading meant that we could actually have removed the on-demand importing in the v2 summary APIs from the previous PR, but it was left in place.)
Here's a breakdown of how I think we can fix this situation:
-
Change the summary APIs to all express explicit BUILD dependencies on TensorFlow and bypass
tensorboard.compat.{tf,tf2}
entirely.-
The
_tf/summary/__init__.py
logic to reexport TF summary already does have a direct dependency at both BUILD and import time, so it won't be affected by this change. -
The v1 summary ops also have a direct dependency already at BUILD time. At runtime, they do
import tensorflow.compat.v1 as tf
when called; I think this could be changed to import time. I think the only reason that wouldn't work is if these APIs are loaded implicitly during TF's import of thetf.summary
TB-provided symbols and thecompat.v1
module is not yet available at that point; we could mitigate this by deferring the dereferencing ofcompat.v1
to occur at runtime rather than import time. -
The v2 summary ops use the indirect
tensorboard.compat
deps. I think these can change to just use an explicit BUILD dep and explicitimport tensorflow.compat.v2 as tf
. The BUILD dep should be okay now that we're using the component package approach (in Add summary/_tf/ package for providing tf.summary API component #1847), since that component package that depends on the v2 summary ops already has a direct BUILD dep on TF. The runtime dep should be okay for the analogous reason (the component package already imports TF), subject to the same caveat as for the v1 summary ops that it's possible dereferencingcompat.v2
will have to occur at runtime rather than import time.Note that this does mean that it won't be possible to use the v2 non-TF summary ops (i.e. the numpy-only versions) without pulling in a TF build and runtime dependency. I think that's acceptable for now; if we want versions of those APIs that avoid TF deps entirely they probably need to live in a separate python package from
tensorboard.summary
anyway, and we'd need to rethink the overall package layout to ensure that they don't depend on anything that might in turn pick up a TF dep, so it's a larger task overall. -
We should also replace the
tensorboard.util.lazy_tensor_creator
usage oftensorboard.compat
to get the TF 2 API in the same vein as the v2 summary ops above, since those are the only places that use this utility.
-
-
Eliminate usage of
//tensorboard/compat:no_tensorflow
-
Remove recently added usage in
uploader_test.py
that seems spurious? -
Get rid of usage for the
X_plugin_notf
tests, where it serves the function of making sure plugins aren't using TensorFlow even it's available as an import (because the test code itself uses it), but this doesn't really guarantee anything because it only works if plugins were accessing TF viatensorboard.compat.tf
in the first place. The better way to do this would be to remove the TF deps from the test code as described in Prune unnecessary TensorFlow dependencies from plugins and tests #2007 and then run these tests in CI without TF installed at all, which would ensure they really don't have any dep on TF.The only plugin that makes real use of this is the projector (which has partial functionality without TF), which can be solved as described below.
-
-
Eliminate usage of
//tensorboard/compat:tensorflow
-
Change the custom scalars plugin to remove a single usage for
tf.compat.as_bytes
-
Replace BUILD deps for mesh plugin
:demo_utils
and:demo_utils_test
with direct TF deps since that's what they actually use -
Change the projector plugin to have a plugin loader that checks if TF is available, and if so injects the TF dependency into the plugin as a constructor parameter, so that it can be cleanly tested in both TF and no-TF modes
-
Migrate all file-I/O-related uses to have that dependency injected as either real
tf.io.gfile
or the stub'sgfile
reimplementation so that they can be cleanly tested dependency-wise
-