Skip to content

Conversation

orionr
Copy link
Contributor

@orionr orionr commented Feb 1, 2019

  • Motivation for features / changes

Continuation of #1663

In order to support TensorBoard for PyTorch we should likely be able to pip install tensorboard (and equivalents) without TensorFlow installed on the machine. Some functionality won't be available in this case, but it should function.

  • Technical description of changes
  1. Update standard tensorboard target in BUILD to not require TF.
  2. Remove tensorboard-notf target from BUILD.
  • Screenshots of UI changes

N/A

  • Detailed steps to verify changes work correctly (as executed by you)
conda create --name notf python=2.7
conda activate notf
pip install --upgrade pip
pip install scipy
conda install protobuf
conda install absl-py
conda install numpy
bazel build tensorboard --verbose_failures
./bazel-bin/tensorboard/tensorboard --logdir ~/local/sample_runs
  • Alternate designs / implementations considered

Separate build target tensorboard-notf

@orionr orionr force-pushed the allow-no-tensorflow branch from d698735 to dce06e1 Compare February 5, 2019 16:53
@orionr orionr changed the title [RFC] Standard TensorBoard build handles no TensorFlow Standard TensorBoard build handles no TensorFlow Feb 13, 2019
@orionr orionr force-pushed the allow-no-tensorflow branch 2 times, most recently from 9366cb3 to 50cfaef Compare February 13, 2019 17:10
@orionr
Copy link
Contributor Author

orionr commented Feb 13, 2019

@nfelt, I explored more advanced ways to disable plugins based on TensorFlow being installed, but I hit a big blocker in that even just importing the plugins (beholder, interactive inference, etc) doesn't work well. I've gone through and changed import tensorflow as tf to from tensorboard.compat import tf, etc. but there are even grpc references. At this point my guess is the large number of changes required (and stubbing even more code in compat) wouldn't be of interest to you. Let me know and thanks.

@orionr
Copy link
Contributor Author

orionr commented Feb 13, 2019

Putting up those changes just so you can see the direction I was going.

@lanpa
Copy link
Contributor

lanpa commented Feb 25, 2019

Hi, @orionr I build the code with the steps provided above with additional pip uninstall tensorflow; bazel clean (using latested d2b8fbb) and got the following error:

Traceback (most recent call last):
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/main.py", line 46, in <module>
    from tensorboard import default
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/default.py", line 45, in <module>
    from tensorboard.plugins.interactive_inference import interactive_inference_plugin
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/plugins/interactive_inference/interactive_inference_plugin.py", line 37, in <module>
    from tensorboard.plugins.interactive_inference.utils import inference_utils
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/plugins/interactive_inference/utils/inference_utils.py", line 29, in <module>
    from tensorboard.plugins.interactive_inference.utils import platform_utils
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/plugins/interactive_inference/utils/platform_utils.py", line 74, in <module>
    example_class=tf.train.Example):
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/lazy.py", line 62, in __getattr__
    return getattr(load_once(self), attr_name)
AttributeError: 'module' object has no attribute 'train'

without using the conda environment, I got similar issue:

Traceback (most recent call last):
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/main.py", line 46, in <module>
    from tensorboard import default
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/default.py", line 37, in <module>
    from tensorboard.plugins.beholder import beholder_plugin
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/plugins/beholder/__init__.py", line 15, in <module>
    from tensorboard.plugins.beholder.beholder import Beholder
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/plugins/beholder/beholder.py", line 199, in <module>
    class BeholderHook(tf.estimator.SessionRunHook):
  File "/Users/dexter/git/tensorboard_orion/bazel-bin/tensorboard/tensorboard.runfiles/org_tensorflow_tensorboard/tensorboard/lazy.py", line 62, in __getattr__
    return getattr(load_once(self), attr_name)
AttributeError: module 'tensorflow' has no attribute 'estimator'

What did I missed?

@orionr
Copy link
Contributor Author

orionr commented Feb 25, 2019

So the last checkin on this pull request (d2b8fbb) is causing that problem, but it's expected. If you remove that (with git rebase -i master or other) you should be able to run normally. On this PR, though, I wanted to show @nfelt that we unfortunately need to do some code check outside the plugins rather than within them since even including them via python errors out without making a ton of changes.

@orionr orionr force-pushed the allow-no-tensorflow branch from d2b8fbb to fd807be Compare February 25, 2019 18:59
@orionr
Copy link
Contributor Author

orionr commented Feb 25, 2019

And make that fd807be with the rebase.

@orionr orionr force-pushed the allow-no-tensorflow branch from fd807be to 9bfef42 Compare March 16, 2019 15:06
@orionr orionr force-pushed the allow-no-tensorflow branch from 9bfef42 to b777974 Compare March 19, 2019 15:37
@orionr
Copy link
Contributor Author

orionr commented Mar 19, 2019

Waiting on feedback from @nfelt on the last diff on this branch before fixing Travis CI.

Nick, is this what you were thinking about? Unfortunately I needed to create loaders for each plugin since I need to load the plugin itself after the TF check and this would get complicated with a shared class. You might have a better approach, though. Thanks.

Copy link
Contributor

@nfelt nfelt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this is exactly what I had in mind - thanks for making the changes! One loader per plugin is fine.

Copy link
Contributor

@nfelt nfelt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@nfelt nfelt merged commit cc25644 into tensorflow:master Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants