profile: disable dashboard inside Colab #1914

wchargin · 2019-02-28T00:59:48Z

Summary:
The profile dashboard now shows a “disabled” message on the frontend
when we detect that we’re running in a Colab notebook. See #1913.

This commit does not cause the profile dashboard to be marked inactive
by the main TensorBoard UI. We might want to do that, but it’s less
straightforward: currently, the backend is responsible for telling the
frontend which plugins are active, and the backend can’t know whether
we’re running in Colab. As a quick hack, we could add special-case logic
in tf-tensorboard.html to check for the profile plugin specifically.
As a longer-term solution, we could let the tf_tensorboard.Dashboard
interface specify an isActive callback, defaulting to () => true.

The implementation includes a small refactor to the profile dashboard to
make it easier to introduce a new state.

Test Plan:
Evaluate !!(window.TENSORBOARD_ENV || {}).IN_COLAB in both standalone
TensorBoard and Colab %tensorboard, and note that each evaluates to
the appropriate boolean.

Run TensorBoard locally with the profile plugin’s demo data, and note
that its behavior is unchanged. To trigger the loading state, it
suffices to load the dashboard, then throttle your network to “Slow 3G”,
then change the selected tool. To trigger the “no data” state, relaunch
TensorBoard pointing at a different log directory.

Then, test the Colab behavior by either pointing TensorBoard at a log
directory that does not contain profile data, then executing

window.TENSORBOARD_ENV = window.TENSORBOARD_ENV || {};
window.TENSORBOARD_ENV["IN_COLAB"] = true;

in the main TensorBoard pane before switching to the inactive profile
dashboard. Note that this is the same setup JavaScript as is injected
into the Colab output frame by notebook.py.

wchargin-branch: profile-disable-colab

Summary: The profile dashboard now shows a “disabled” message on the frontend when we detect that we’re running in a Colab notebook. See #1913. This commit does _not_ cause the profile dashboard to be marked inactive by the main TensorBoard UI. We might want to do that, but it’s less straightforward: currently, the backend is responsible for telling the frontend which plugins are active, and the backend can’t know whether we’re running in Colab. As a quick hack, we could add special-case logic in `tf-tensorboard.html` to check for the profile plugin specifically. As a longer-term solution, we could let the `tf_tensorboard.Dashboard` interface specify an `isActive` callback, defaulting to `() => true`. ![Screenshot of the “Profiling isn’t yet supported in Colab” message][s] [s]: https://user-images.githubusercontent.com/4317806/53533696-a6497d80-3ab0-11e9-935d-56e857a687a8.png The implementation includes a small refactor to the profile dashboard to make it easier to introduce a new state. Test Plan: Evaluate `!!(window.TENSORBOARD_ENV || {}).IN_COLAB` in both standalone TensorBoard and Colab `%tensorboard`, and note that each evaluates to the appropriate boolean. Run TensorBoard locally with the profile plugin’s demo data, and note that its behavior is unchanged. To trigger the loading state, it suffices to load the dashboard, then throttle your network to “Slow 3G”, then change the selected tool. To trigger the “no data” state, relaunch TensorBoard pointing at a different log directory. Then, test the Colab behavior by either pointing TensorBoard at a log directory that does not contain profile data, then executing ```js window.TENSORBOARD_ENV = window.TENSORBOARD_ENV || {}; window.TENSORBOARD_ENV["IN_COLAB"] = true; ``` in the main TensorBoard pane before switching to the inactive profile dashboard. Note that this is the same setup JavaScript as is injected into the Colab output frame by `notebook.py`. wchargin-branch: profile-disable-colab

nfelt · 2019-02-28T01:40:28Z

tensorboard/plugins/profile/tf_profile_dashboard/tf-profile-dashboard.html

+        return "IN_COLAB";
+      if (dataNotFound)
+        return "DATA_NOT_FOUND";
+      if (!progress || progress.value < 100)


I think we can now remove _isNotComplete() since its logic was inlined here and it's not called anywhere else

Good point; done.

wchargin-branch: profile-disable-colab

GalOshri · 2019-02-28T02:47:00Z

nit on the error message: "Profiling isn't yet supported in Colab" sounds less natural to me than "Profiling isn't supported in Colab yet".

Relevant question: I think the profiling (creating the right logs) is supported, right? It's just displaying them in the dashboard that isn't.

Additionally, does this affect only Colab or also Jupyter?

wchargin · 2019-02-28T04:34:03Z

nit on the error message: "Profiling isn't yet supported in Colab"
sounds less natural to me than "Profiling isn't supported in Colab
yet".

Sure; can change.

Relevant question: I think the profiling (creating the right logs) is
supported, right? It's just displaying them in the dashboard that
isn't.

Correct; we generate the logs (though the autoprofiler, triggered from
TensorBoard, of course can’t run if TensorBoard can’t show the
profiler). I think that the wording is fine, because the profiling
experience is certainly broken given that you can’t consume what you
produce. But I’d accept a PR to change it.

Additionally, does this affect only Colab or also Jupyter?

Colab only. Jupyter puts all of TensorBoard in an iframe with direct
network access; another iframe within that iframe is fine because there
is no service worker proxy to contend with. (This also means that
hparams should work fine with POSTs.)

wchargin-branch: profile-disable-colab

wchargin · 2019-02-28T04:44:02Z

Proof of life for Jupyter:

stephanwlee · 2019-02-28T01:30:37Z

tensorboard/plugins/profile/tf_profile_dashboard/tf-profile-dashboard.html

@@ -43,13 +43,23 @@

 <dom-module id="tf-profile-dashboard">
 <template>
-<template is="dom-if" if="[[_isNotComplete(progress)]]">
+<template is="dom-if" if="[[_isState(_topLevelState, 'IN_COLAB')]]">
+  <div style="max-width: 540px; margin: 80px auto 0 auto;">


:( prefer not to use inline style.

This is copy-pasted from below…which is indeed a reasonable argument for
making it a class. :-)

stephanwlee · 2019-02-28T04:47:48Z

tensorboard/plugins/profile/tf_profile_dashboard/tf-profile-dashboard.html

+      _topLevelState: {
+        type: String,
+        computed: '_computeTopLevelState(_inColab, _dataNotFound, progress)',
+        readOnly: true,


computed is readOnly. https://polymer-library.polymer-project.org/1.0/docs/devguide/properties

Good to know; thanks.

stephanwlee · 2019-02-28T04:48:52Z

tensorboard/plugins/profile/tf_profile_dashboard/tf-profile-dashboard.html

@@ -235,6 +274,17 @@ <h3>No profile data was found.</h3>
        type: Array,
        observer: '_activeHostsChanged'
      },
+      _inColab: {
+        type: Boolean,
+        value: () => !!(window.TENSORBOARD_ENV || {}).IN_COLAB,


Instead of !!, use Boolean((window.TENSORBOARD_ENV || {}).IN_COLAB) though I understand both with ease.

http://go/tsstyle explicitly says that !! is okay; jsstyle has no mention.

Summary: Not only is opening in new tabs a better user experience, it’s necessary in notebook contexts so that we don’t try to load the GitHub docs in an iframe, which would fail because GitHub sets `frame-ancestors 'none'`. (When I tested #1914 in Colab, I instinctively Ctrl-clicked the link, thus not hitting this issue.) Test Plan: Tested that in both Jupyter and Colab, clicking on a help link before this change yields a white frame with a “Refused to display…” console error, while after this change it opens the appropriate link in a new tab. Checked statically that each link has `rel="noopener" target="_blank"`: ```shell $ <./tensorboard/plugins/profile/tf_profile_dashboard/tf-profile-dashboard.html tee \ > >(grep 'href=' | grep -Fcv '<link') \ > >(grep -Fc 'rel="noopener"') \ > >(grep -Fc 'target="_blank"') \ > >/dev/null 6 6 6 ``` wchargin-branch: profile-help-in-new-tab

Summary: The profile dashboard had an existing bug that was revealed by #1914: the loading progress bar never actually finishes. Prior to #1914, you could actually see the loading bar if you made your window narrow enough: ![Screenshot of a zombie progress bar](https://user-images.githubusercontent.com/4317806/53674700-95317580-3c44-11e9-924f-82f6dc8d5f18.png) This was because the prior version of the dashboard rendered the main tool irrespective of whether the data had actually finished loading. As of #1914, the “loading” and “active” states are mutually exclusive, so instead of just having a zombie progress bar we actually block the view. The cause of the bug is a promise that never resolves. (The executor provided to the `Promise` constructor’s return value is ignored; only the `resolve` and `reject` callbacks can fulfill a promise.) This was not caught while testing #1914 because it cannot be reproduced with the demo data present at that commit; with that more complete set of data, there are other code paths that clobber the progress value to 100% (which is really a separate issue, but that’s not important right now), such as `_maybeUpdateData`. Test Plan: Regenerate profile plugin demo data if you haven’t done so since #1942, then launch TensorBoard: ``` $ rm -rf /tmp/profile_demo/ $ bazel run //tensorboard/plugins/profile:profile_demo $ bazel run //tensorboard -- --logdir /tmp/profile_demo ``` The trace viewer should be displayed (prior to this commit, it wasn’t). wchargin-branch: profile-broken-promises

Summary: The profile dashboard now shows a “disabled” message on the frontend when we detect that we’re running in a Colab notebook. See tensorflow#1913. This commit does _not_ cause the profile dashboard to be marked inactive by the main TensorBoard UI. We might want to do that, but it’s less straightforward: currently, the backend is responsible for telling the frontend which plugins are active, and the backend can’t know whether we’re running in Colab. As a quick hack, we could add special-case logic in `tf-tensorboard.html` to check for the profile plugin specifically. As a longer-term solution, we could let the `tf_tensorboard.Dashboard` interface specify an `isActive` callback, defaulting to `() => true`. ![Screenshot of the “Profiling isn’t yet supported in Colab” message][s] [s]: https://user-images.githubusercontent.com/4317806/53541886-b83b1880-3ad0-11e9-9501-299f5e671d7a.png The implementation includes a small refactor to the profile dashboard to make it easier to introduce a new state. Test Plan: Evaluate `!!(window.TENSORBOARD_ENV || {}).IN_COLAB` in both standalone TensorBoard and Colab `%tensorboard`, and note that each evaluates to the appropriate boolean. Run TensorBoard locally with the profile plugin’s demo data, and note that its behavior is unchanged. To trigger the loading state, it suffices to load the dashboard, then throttle your network to “Slow 3G”, then change the selected tool. To trigger the “no data” state, relaunch TensorBoard pointing at a different log directory. Then, test the Colab behavior by either pointing TensorBoard at a log directory that does not contain profile data, then executing ```js window.TENSORBOARD_ENV = window.TENSORBOARD_ENV || {}; window.TENSORBOARD_ENV["IN_COLAB"] = true; ``` in the main TensorBoard pane before switching to the inactive profile dashboard. Note that this is the same setup JavaScript as is injected into the Colab output frame by `notebook.py`. wchargin-branch: profile-disable-colab

Summary: Not only is opening in new tabs a better user experience, it’s necessary in notebook contexts so that we don’t try to load the GitHub docs in an iframe, which would fail because GitHub sets `frame-ancestors 'none'`. (When I tested tensorflow#1914 in Colab, I instinctively Ctrl-clicked the link, thus not hitting this issue.) Test Plan: Tested that in both Jupyter and Colab, clicking on a help link before this change yields a white frame with a “Refused to display…” console error, while after this change it opens the appropriate link in a new tab. Checked statically that each link has `rel="noopener" target="_blank"`: ```shell $ <./tensorboard/plugins/profile/tf_profile_dashboard/tf-profile-dashboard.html tee \ > >(grep 'href=' | grep -Fcv '<link') \ > >(grep -Fc 'rel="noopener"') \ > >(grep -Fc 'target="_blank"') \ > >/dev/null 6 6 6 ``` wchargin-branch: profile-help-in-new-tab

Summary: The profile dashboard had an existing bug that was revealed by tensorflow#1914: the loading progress bar never actually finishes. Prior to tensorflow#1914, you could actually see the loading bar if you made your window narrow enough: ![Screenshot of a zombie progress bar](https://user-images.githubusercontent.com/4317806/53674700-95317580-3c44-11e9-924f-82f6dc8d5f18.png) This was because the prior version of the dashboard rendered the main tool irrespective of whether the data had actually finished loading. As of tensorflow#1914, the “loading” and “active” states are mutually exclusive, so instead of just having a zombie progress bar we actually block the view. The cause of the bug is a promise that never resolves. (The executor provided to the `Promise` constructor’s return value is ignored; only the `resolve` and `reject` callbacks can fulfill a promise.) This was not caught while testing tensorflow#1914 because it cannot be reproduced with the demo data present at that commit; with that more complete set of data, there are other code paths that clobber the progress value to 100% (which is really a separate issue, but that’s not important right now), such as `_maybeUpdateData`. Test Plan: Regenerate profile plugin demo data if you haven’t done so since tensorflow#1942, then launch TensorBoard: ``` $ rm -rf /tmp/profile_demo/ $ bazel run //tensorboard/plugins/profile:profile_demo $ bazel run //tensorboard -- --logdir /tmp/profile_demo ``` The trace viewer should be displayed (prior to this commit, it wasn’t). wchargin-branch: profile-broken-promises

Summary: The profile dashboard now shows a “disabled” message on the frontend when we detect that we’re running in a Colab notebook. See #1913. This commit does _not_ cause the profile dashboard to be marked inactive by the main TensorBoard UI. We might want to do that, but it’s less straightforward: currently, the backend is responsible for telling the frontend which plugins are active, and the backend can’t know whether we’re running in Colab. As a quick hack, we could add special-case logic in `tf-tensorboard.html` to check for the profile plugin specifically. As a longer-term solution, we could let the `tf_tensorboard.Dashboard` interface specify an `isActive` callback, defaulting to `() => true`. ![Screenshot of the “Profiling isn’t yet supported in Colab” message][s] [s]: https://user-images.githubusercontent.com/4317806/53541886-b83b1880-3ad0-11e9-9501-299f5e671d7a.png The implementation includes a small refactor to the profile dashboard to make it easier to introduce a new state. Test Plan: Evaluate `!!(window.TENSORBOARD_ENV || {}).IN_COLAB` in both standalone TensorBoard and Colab `%tensorboard`, and note that each evaluates to the appropriate boolean. Run TensorBoard locally with the profile plugin’s demo data, and note that its behavior is unchanged. To trigger the loading state, it suffices to load the dashboard, then throttle your network to “Slow 3G”, then change the selected tool. To trigger the “no data” state, relaunch TensorBoard pointing at a different log directory. Then, test the Colab behavior by either pointing TensorBoard at a log directory that does not contain profile data, then executing ```js window.TENSORBOARD_ENV = window.TENSORBOARD_ENV || {}; window.TENSORBOARD_ENV["IN_COLAB"] = true; ``` in the main TensorBoard pane before switching to the inactive profile dashboard. Note that this is the same setup JavaScript as is injected into the Colab output frame by `notebook.py`. wchargin-branch: profile-disable-colab

Summary: Not only is opening in new tabs a better user experience, it’s necessary in notebook contexts so that we don’t try to load the GitHub docs in an iframe, which would fail because GitHub sets `frame-ancestors 'none'`. (When I tested #1914 in Colab, I instinctively Ctrl-clicked the link, thus not hitting this issue.) Test Plan: Tested that in both Jupyter and Colab, clicking on a help link before this change yields a white frame with a “Refused to display…” console error, while after this change it opens the appropriate link in a new tab. Checked statically that each link has `rel="noopener" target="_blank"`: ```shell $ <./tensorboard/plugins/profile/tf_profile_dashboard/tf-profile-dashboard.html tee \ > >(grep 'href=' | grep -Fcv '<link') \ > >(grep -Fc 'rel="noopener"') \ > >(grep -Fc 'target="_blank"') \ > >/dev/null 6 6 6 ``` wchargin-branch: profile-help-in-new-tab

Summary: The profile dashboard had an existing bug that was revealed by #1914: the loading progress bar never actually finishes. Prior to #1914, you could actually see the loading bar if you made your window narrow enough: ![Screenshot of a zombie progress bar](https://user-images.githubusercontent.com/4317806/53674700-95317580-3c44-11e9-924f-82f6dc8d5f18.png) This was because the prior version of the dashboard rendered the main tool irrespective of whether the data had actually finished loading. As of #1914, the “loading” and “active” states are mutually exclusive, so instead of just having a zombie progress bar we actually block the view. The cause of the bug is a promise that never resolves. (The executor provided to the `Promise` constructor’s return value is ignored; only the `resolve` and `reject` callbacks can fulfill a promise.) This was not caught while testing #1914 because it cannot be reproduced with the demo data present at that commit; with that more complete set of data, there are other code paths that clobber the progress value to 100% (which is really a separate issue, but that’s not important right now), such as `_maybeUpdateData`. Test Plan: Regenerate profile plugin demo data if you haven’t done so since #1942, then launch TensorBoard: ``` $ rm -rf /tmp/profile_demo/ $ bazel run //tensorboard/plugins/profile:profile_demo $ bazel run //tensorboard -- --logdir /tmp/profile_demo ``` The trace viewer should be displayed (prior to this commit, it wasn’t). wchargin-branch: profile-broken-promises

wchargin requested review from stephanwlee and GalOshri February 28, 2019 00:59

nfelt approved these changes Feb 28, 2019

View reviewed changes

review: remove now-unused _isNotComplete

0001611

wchargin-branch: profile-disable-colab

wchargin added type:bug plugin:profile labels Feb 28, 2019

review: permute “isn’t (yet→) supported (→yet)”

9e28928

wchargin-branch: profile-disable-colab

wchargin merged commit 5101522 into master Feb 28, 2019

wchargin deleted the wchargin-profile-disable-colab branch February 28, 2019 04:47

stephanwlee reviewed Feb 28, 2019

View reviewed changes

wchargin mentioned this pull request Feb 28, 2019

profile/dynamic plugins should work in Colab #1913

Closed

wchargin mentioned this pull request Feb 28, 2019

profile: make help links open in new tabs #1922

Merged

wchargin mentioned this pull request Mar 2, 2019

profile: fix loading progress behavior #1945

Merged

stephanwlee mentioned this pull request Mar 5, 2019

1.13.1 cherry-pick tracking #1906

Closed

wchargin mentioned this pull request Mar 5, 2019

Release 1.13.1 #1963

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

profile: disable dashboard inside Colab #1914

profile: disable dashboard inside Colab #1914

wchargin commented Feb 28, 2019 •

edited

Loading

nfelt Feb 28, 2019

wchargin Feb 28, 2019

GalOshri commented Feb 28, 2019

wchargin commented Feb 28, 2019

wchargin commented Feb 28, 2019

stephanwlee Feb 28, 2019

wchargin Feb 28, 2019

stephanwlee Feb 28, 2019

wchargin Feb 28, 2019

stephanwlee Feb 28, 2019

wchargin Feb 28, 2019

wchargin Feb 28, 2019

profile: disable dashboard inside Colab #1914

profile: disable dashboard inside Colab #1914

Conversation

wchargin commented Feb 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GalOshri commented Feb 28, 2019

wchargin commented Feb 28, 2019

wchargin commented Feb 28, 2019

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

wchargin commented Feb 28, 2019 •

edited

Loading