Enable WIT usage as a jupyter extension #1662

jameswex · 2018-12-05T19:24:44Z

Motivation for features / changes

This PR enables the What-If Tool to work as a Jupyter extension, while not affecting its use in TensorBoard as well, which many customers have asked for.

Technical description of changes

The code for the jupyter extension sits in the witwidget directory under the top-level plugin directory. The jupyter extension can be installed as its own pip package named witwidget and is built using a bazel command and shell script just like the main tensorboard pip package. There are added instructions for this mode in the WIT top-level readme and the WIT_from_scratch tutorial jupyter notebook.

wit.py and wit.js contain the special code for using WIT in a jupyter extension, using the ipywidget framework with traitlets. Most of the rest of the new files are necessary boilerplate from https://github.com/jupyter-widgets/widget-cookiecutter

Screenshots of UI changes

Screenshot showing invocation and display of WIT inside jupyter notebook.

Detailed steps to verify changes work correctly (as executed by you)

Ran tensorboard with WIT on existing models to ensure no functionality loss in TensorBoard.

Built pip package and installed it, ran WIT inside of notebook as per the updated tutorial notebook and tested all of its features.

Alternate designs / implementations considered

Colab implementation of extension will be in separate PR. Colab doesn't allow for ipywidget use so the implementation must be different unfortunately. This follows the same patterns as the TensorFlow Model Analysis jupyter/colab extensions.

stephanwlee

Hi @jameswex, thanks a lot for opening the PR.
Just curious, since setup.py runs/generates the static file, you don't need to check those files in, right?
It is just required at the pip package generation time for the data_files?

jameswex · 2018-12-06T19:01:06Z

I think you are right that static/extension.js, static/index.js and static/index.js.map don't need to be checked in as they are created by the pip setup. This is my first pip package so I'm not very familiar with it, so its possible there is some trimming down that can be done. I believe the wit_jupyter.html needs to stay as part of the submission, unless the setup.py can be altered to build it from the appopriate bazel run and copy it to the right place for packaging. Is that also doable in the setup.py?

stephanwlee · 2018-12-06T19:04:55Z

James, would it be possible to run the bazel build tensorboard/plugins/interactive_inference/tf_interactive_inference_dashboard:wit_jupyter in the setup.py? You may need to find the file in the bazel-out directory but it seems cleaner than checking in transpiled code.

nfelt · 2018-12-06T19:49:30Z

Another option re: the generated code might be checking in a script that first generates those intermediate files in a temporary location, and then invokes setup.py to build the pip package using those files.

This is basically what we do for the main pip package in build_pip_package.sh - we build the python code as data dependencies to a sh_binary script, and then the script pulls the python files out of the bazel runfiles directory into a new directory and generates the pip package from there.

tensorboard/tensorboard/pip_package/BUILD

Lines 8 to 30 in 0da00e7

    
           # rm -rf /tmp/tensorboard 
        
           # bazel run //tensorboard/pip_package:build_pip_package 
        
           # pip install -U /tmp/tensorboard/*py2*.pip 
        
           sh_binary( 
        
               name = "build_pip_package", 
        
               srcs = ["build_pip_package.sh"], 
        
               data = [ 
        
                   "LICENSE", 
        
                   "MANIFEST.in", 
        
                   "README.rst", 
        
                   "setup.cfg", 
        
                   "setup.py", 
        
                   "//tensorboard",  # Main tensorboard binary and everything it uses 
        
                   "//tensorboard:lib",  # User-facing overall TensorBoard API 
        
                   "//tensorboard:version",  # Version module (read by setup.py) 
        
                   "//tensorboard/plugins/beholder",  # User-facing beholder API 
        
                   "//tensorboard/plugins/projector",  # User-facing projector API 
        
               ], 
        
               tags = [ 
        
                   "local", 
        
                   "manual", 
        
               ], 
        
           )

tensorboard/tensorboard/pip_package/build_pip_package.sh

Lines 75 to 116 in 0da00e7

    
           cp -LR "${RUNFILES}/org_tensorflow_tensorboard/tensorboard" . 
        
           mv -f "tensorboard/pip_package/LICENSE" . 
        
           mv -f "tensorboard/pip_package/MANIFEST.in" . 
        
           mv -f "tensorboard/pip_package/README.rst" . 
        
           mv -f "tensorboard/pip_package/setup.cfg" . 
        
           mv -f "tensorboard/pip_package/setup.py" . 
        
           rm -rf tensorboard/pip_package 
        
           rm -f tensorboard/tensorboard              # bazel py_binary sh wrapper 
        
           chmod -x LICENSE                           # bazel symlinks confuse cp 
        
           find . -name __init__.py | xargs chmod -x  # which goes for all genfiles 
        
           mkdir -p tensorboard/_vendor 
        
           touch tensorboard/_vendor/__init__.py 
        
           cp -LR "${RUNFILES}/org_html5lib/html5lib" tensorboard/_vendor 
        
           cp -LR "${RUNFILES}/org_mozilla_bleach/bleach" tensorboard/_vendor 
        
           # Vendor tensorflow-serving-api because it depends directly on TensorFlow. 
        
           # TODO(nickfelt): de-vendor if they're able to relax that dependency. 
        
           cp -LR "${RUNFILES}/org_tensorflow_serving_api/tensorflow_serving" tensorboard/_vendor 
        
           chmod -R u+w,go+r . 
        
           find tensorboard -name \*.py | 
        
             xargs $sedi -e ' 
        
               s/^import html5lib$/from tensorboard._vendor import html5lib/ 
        
               s/^from html5lib/from tensorboard._vendor.html5lib/ 
        
               s/^import bleach$/from tensorboard._vendor import bleach/ 
        
               s/^from bleach/from tensorboard._vendor.bleach/ 
        
               s/from tensorflow_serving/from tensorboard._vendor.tensorflow_serving/ 
        
             ' 
        
           virtualenv venv 
        
           export VIRTUAL_ENV=venv 
        
           export PATH="$PWD/venv/bin:${PATH}" 
        
           unset PYTHON_HOME 
        
           # Require wheel for bdist_wheel command, and setuptools 36.2.0+ so that 
        
           # env markers are handled (https://github.com/pypa/setuptools/pull/1081) 
        
           pip install -qU wheel 'setuptools>=36.2.0' 
        
           python setup.py bdist_wheel --python-tag py2 >/dev/null 
        
           python setup.py bdist_wheel --python-tag py3 >/dev/null

jameswex · 2018-12-06T22:17:43Z

I added a new command to setup.py that builds wit_jupyter and moves it to the static dir, so the static dir has been removed from the repo.

jameswex · 2018-12-28T18:52:11Z

@stephanwlee @tolga-b This PR now only contains the necessary code to use WIT as a jupyter extension (including building the pip package for the extension). The colab extension code will be reviewed as a separate PR after this is submitted.

Can be tested locally if desired by building the pip package whl files as per the new bazel rule in pip_package directory then using 'pip install [path to built whl file] and running the jupyter nbextension commands as per the updated plugin documentation, then running through the WIT_from_scratch.ipynb notebook which has been updated to show this usage.

stephanwlee · 2018-12-28T18:53:51Z

tensorboard/plugins/interactive_inference/README.md

+
+## How do I use it in a Jupyter notebook?
+First, install and enable WIT for Jupyter through the following commands:
+> pip install witwidget


Seems to read better like this

\```sh pip install witwidget jupyter nbextension install --py --symlink --sys-prefix witwidget jupyter nbextension enable --py --sys-prefix witwidget \```

(omit the backslash before backticks in code)

Drive-by markdown tip: @stephanwlee, you can write

Seems to read better like this ``````markdown ```sh pip install witwidget jupyter nbextension install --py --symlink --sys-prefix witwidget jupyter nbextension enable --py --sys-prefix witwidget ``` ``````

which will render like this:

Seems to read better like this

```sh pip install witwidget jupyter nbextension install --py --symlink --sys-prefix witwidget jupyter nbextension enable --py --sys-prefix witwidget ```

In general, you can start code fences or inline code spans with any
number of backticks, and close them with the matching number of
backticks.

stephanwlee · 2018-12-28T18:55:32Z

tensorboard/plugins/interactive_inference/witwidget/notebook/colab/__init__.py

@@ -0,0 +1,13 @@
+# Copyright 2018 The TensorFlow Authors. All Rights Reserved.


omit this file for now

stephanwlee · 2018-12-28T18:58:17Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/lib/wit.js

+// python backend.
+var WITView = widgets.DOMWidgetView.extend({
+    render: function() {
+        loadVulcanizedTemplate();


Is this a synchronous operation? I forgot whether adding a script tag to the head is synchronous. Anyways, depending on the answer, it may impact L42

i'm seeing this code function correctly as-is when i test locally. should i be instead deferring to some point in the future to be more defensive? The setTimeouts (now requestAnimationFrames) in my code may be accomplishing this for me already.

I have tried this. Not sure of <link> differs too much from <script> but it sounds like it is worth doing some investigation to invoke L26: http://jsfiddle.net/b726ykhc/

Thanks for bringing this up. Fixed this all with change to wait for onload of the import before creating the WIT polymer element. Now there are no more setTimeouts OR requestAnimationFrame calls. Much simpler logic.

stephanwlee · 2018-12-28T19:00:14Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/lib/wit.js

+
+        // Add listeners for changes from WIT Polymer element. Passes changes
+        // along to python.
+        this.view_.addEventListener('infer-examples', e => {


would you be factor out view creation to its own method?

stephanwlee · 2018-12-28T19:01:59Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/lib/wit.js

+          this.mutantFeature = e.detail.feature_name;
+          this.touch();
+        });
+        setTimeout(()=> {


prefer to use requestAnimationFrame unless it has to happen in nextTick.

stephanwlee · 2018-12-28T19:02:56Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/lib/wit.js

+    // Callback functions for when changes made on python side.
+    examples_changed: function() {
+      const examples = this.model.get('examples');
+      if (!this.view_.updateExampleContents_) {


It should not read the private property of the view. Can this be refactored?

made this method public as it is needed to be called here.

i added this because i noticed that in practice when the element is created, it doesn't initially have its methods defined (must have to do with how polymer elements are created?)

stephanwlee · 2018-12-28T19:03:52Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/lib/wit.js

+    examples_changed: function() {
+      const examples = this.model.get('examples');
+      if (!this.view_.updateExampleContents_) {
+        setTimeout(() => this.examples_changed(), 100);


If above if statement is truthy, we can get stuck in infinite loop. Also, since examples_changed can get triggered multiple times, consider clearing the timeout before doing this.

switched to requestAnimationFrame. i added this because i noticed that in practice when the element is created, it doesn't initially have its methods defined (must have to do with how polymer elements are created?), so we need to wait until the method exists before calling it.

stephanwlee · 2018-12-28T19:04:14Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/lib/wit.js

+    },
+    eligible_features_changed: function() {
+      const features = this.model.get('eligible_features');
+      this.view_.partialDepPlotEligibleFeatures = features


nit: missing trailing semi-colon (for consistency)

stephanwlee · 2018-12-28T19:06:09Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/lib/wit.js

+    },
+    config_changed: function() {
+      const config = this.model.get('config');
+      if ('inference_address' in config) {


Curious: is config guaranteed to be non-null or non-undefined?

added protection against null

stephanwlee · 2018-12-28T19:08:05Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/lib/wit.js

+    },
+    sprite_changed: function() {
+      if (!this.view_.updateSprite_) {
+        setTimeout(() => this.sprite_changed(), 100);


consider clearing timeout before setting a new one.

switched to requestAnimationFrame

stephanwlee · 2019-01-02T16:29:02Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/lib/index.js

+limitations under the License.
+==============================================================================*/
+
+__webpack_public_path__ =


Is this currently being exported as a global variable? If so, can we be a little bit more explicit with (global || window).__webpack_public_path__? Or, pull it out to own module and exporting the value that wit.js requires would be more readable

used window.

stephanwlee · 2019-01-02T16:32:07Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/webpack.config.js

+        output: {
+            filename: 'index.js',
+            path: path.resolve(__dirname, 'static'),
+            libraryTarget: 'amd'


any thoughts on setting the public path here and remove https://github.com/tensorflow/tensorboard/pull/1662/files#diff-56459f8bc62645373c45f3cb38734e04R16?

doesn't matter to me where it is set, we don't expect it to be updated in future PRs.

stephanwlee · 2019-01-02T16:53:56Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/wit.py

+  """WIT widget for Jupyter."""
+  _view_name = Unicode('WITView').tag(sync=True)
+  _view_module = Unicode('wit-widget').tag(sync=True)
+  _view_module_version = Unicode('^0.1.5').tag(sync=True)


It seems like the version number is encoded in three different places (version.py for pip package gen, package.json, and here). Can we do anything to make it more maintainable? If not, we might need to update the RELEASE.md

looking back at TFMA's use, only the version.py is important for the package. The package.json doesn't need a version as it isn't published and the view module versions can be kept static at 0.1.0 - they do not need to match the pip version and don't need to ever be updated as this is the only wit widget to be pip installed for a jupyter kernel. made appropriate changes.

stephanwlee · 2019-01-02T16:55:12Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/wit.py

+
+  def __init__(self, config_builder, height=1000):
+    super(WitWidget, self).__init__(layout=Layout(height='%ipx' % height))
+    tf.logging.set_verbosity(tf.logging.WARN)


please note that this is going away in tf 2.0.

this plugin and widget version of it will need tf 2.0 updates, but i imagine outside of this PR

stephanwlee · 2019-01-02T17:01:23Z

tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js/lib/wit.js

+      this.view_.hasSprite = true;
+      this.view_.localAtlasUrl = spriteUrl;
+      this.view_.updateSprite();
+  },


nit: indentation is off

tolga-b

Thank you James, just some high-level comments for now:

Run Inference button in jupyter notebook always stays on.
I get the following deprecation warning under the widget in jupyter:
WARNING:tensorflow: .../python2.7/site-packages/tensorflow_estimator/python/estimator/util.py:104: make_initializable_iterator (from tensorflow.python.data.ops.dataset_ops) is deprecated and will be removed in a future version. Instructions for updating: Use for ... in dataset:to iterate over a dataset. If usingtf.estimator, return the Datasetobject directly from your input function. As a last resort, you can usetf.compat.v1.data.make_initializable_iterator(dataset).

tolga-b · 2019-01-02T20:54:16Z

tensorboard/plugins/interactive_inference/witwidget/pip_package/build_pip_package.sh

+fi
+
+set -x
+command -v virtualenv >/dev/null


this line fails silently when virtualenv is not installed (does not warn that it is not installed, just says build completed successfully).

tolga-b · 2019-01-02T21:07:51Z

In jupyter mode PDP and Global PDP panels do not seem to always open the top plot as it was in tensorboard mode. For example in WIT_From_Scratch notebook, if you open PDP panel then close and open it again, all PDPs are closed and one needs to click on "Age" again for the plot to be computed.

jameswex · 2019-01-03T20:26:11Z

Thanks Tolga. Fixed the warning with a change in the example ipython notebook and fixed the inference button issue with a fix in the WitConfigBuilder object handling when a label vocab isn't set.

nfelt

Pip package scripting LGTM.

nfelt · 2019-01-03T21:53:56Z

tensorboard/plugins/interactive_inference/witwidget/pip_package/BUILD

+        "//tensorboard/plugins/interactive_inference/tf_interactive_inference_dashboard:wit_jupyter", # For HTML payload
+        "//tensorboard/plugins/interactive_inference/witwidget/notebook/jupyter/js:js_files",
+        "//tensorboard/plugins/interactive_inference/utils:inference_utils",  # Utility method that visualization code uses.
+        "//tensorboard/plugins/interactive_inference/witwidget",  # Version module (read by setup.py)


Stale comment? The line below is the version module

nfelt · 2019-01-03T22:01:29Z

tensorboard/plugins/interactive_inference/witwidget/pip_package/setup.py

+REQUIRED_PACKAGES = [
+    'ipywidgets>=7.0.0',
+    'jupyter>=1.0,<2',
+    'tensorflow>=1.12.0',


FWIW - TensorBoard doesn't express a direct dep on TF because if this dep won't be satisfied by e.g. the tensorflow-gpu or tf-nightly packages, as pip isn't smart enough to realize that they're all providing the tensorflow package. If the user already has one of those packages installed, then installing witwidget will cause tensorflow to be installed and clobber the existing package, which can result in a bit of a mess.

I guess tensorflow-serving-api already depends on TF directly, though (it partially gets around this by shipping a -gpu version that depends on tensorflow-gpu instead, apparently), so you're kind of stuck. Just something to be aware of at least.

hmm not ideal. thanks for explaining this.

nfelt · 2019-01-03T22:02:47Z

tensorboard/plugins/interactive_inference/witwidget/pip_package/setup.py

+      'Programming Language :: Python :: 3',
+      'Programming Language :: Python :: 3.3',
+      'Programming Language :: Python :: 3.4',
+      'Programming Language :: Python :: 3.5',


Also include 3.6? Not that it makes much functional difference AFAIK.

tensorboard/plugins/interactive_inference/README.md

jameswex and others added 11 commits March 19, 2018 19:38

Add flags to TBContext

d84a0eb

Add flags to TBContext in application

ab0e9cf

Add flags to tbcontext

e72cbcf

gitignore changes

3803ab7

Merge branch 'master' of https://github.com/tensorflow/tensorboard

286df31

Merge remote-tracking branch 'upstream/master'

22cfe3e

wit changes for jupyter

cd67388

clean up unwanted files

b176cf0

cleanup

cf8c835

fixes

143aedd

doc updates:

64b7cfa

jameswex requested a review from nfelt December 5, 2018 19:24

stephanwlee reviewed Dec 6, 2018

View reviewed changes

have setup.py build wit_jupyter.html

4e8d6f9

jameswex added 12 commits December 6, 2018 17:18

Merge remote-tracking branch 'upstream/master'

f68bc7e

Merge branch 'master' into wit-jup

b6b077b

licenses

8674db7

pip fixes

e970604

refactor

990b125

move file

c0439a9

cleanup

c1d83dc

remove old dist files

6fc13e1

cleanup

047ecd1

fix colab height

be46e3d

update

15e5338

Merge remote-tracking branch 'upstream/master'

64e3c0a

jameswex added 8 commits December 28, 2018 11:34

Merge remote-tracking branch 'upstream/master'

f183091

Merge branch 'master' into wit-jup

6c9bcc1

remove gitignore change

187cb8e

resolve merge error

7f09168

Merge remote-tracking branch 'upstream/master'

e73aa1e

Merge branch 'master' into wit-jup

070e912

review comments

cae1f19

remove colab for now

0b94fef

jameswex changed the title ~~Proof-of-concept WIT as a jupyter extension~~ Enable WIT usage as a jupyter extension Dec 28, 2018

jameswex added 2 commits December 28, 2018 13:41

update license

e8588f1

spacing fix

ba96419

stephanwlee reviewed Dec 28, 2018

View reviewed changes

jameswex added 3 commits December 28, 2018 14:35

review comments

29c04a8

review more

c5af9f5

fix async issues

7cdb5e6

stephanwlee reviewed Jan 2, 2019

View reviewed changes

version and review updates

38225a5

tolga-b reviewed Jan 2, 2019

View reviewed changes

tolga fixes

000e580

update homepage link for pip package:

b8e5cf9

nfelt reviewed Jan 3, 2019

View reviewed changes

stephanwlee reviewed Jan 3, 2019

View reviewed changes

tensorboard/plugins/interactive_inference/README.md Show resolved Hide resolved

tolga-b approved these changes Jan 4, 2019

View reviewed changes

review comments:

122b2f6

stephanwlee approved these changes Jan 4, 2019

View reviewed changes

jameswex merged commit 7713137 into tensorflow:master Jan 4, 2019

jameswex deleted the wit-jup branch January 4, 2019 17:47

		@@ -0,0 +1,13 @@
		# Copyright 2018 The TensorFlow Authors. All Rights Reserved.

Enable WIT usage as a jupyter extension #1662

Enable WIT usage as a jupyter extension #1662

Conversation

jameswex commented Dec 5, 2018 • edited Loading

stephanwlee left a comment

Choose a reason for hiding this comment

jameswex commented Dec 6, 2018

stephanwlee commented Dec 6, 2018

nfelt commented Dec 6, 2018 • edited Loading

jameswex commented Dec 6, 2018

jameswex commented Dec 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tolga-b left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tolga-b commented Jan 2, 2019

jameswex commented Jan 3, 2019

nfelt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameswex commented Dec 5, 2018 •

edited

Loading

nfelt commented Dec 6, 2018 •

edited

Loading