scalars: don’t multiplex fetches in Colab #4191
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: In #4050, we added multiplexing to scalar chart fetches for performance. This works nicely in local TensorBoard and public Colab, but the POST request machinery isn’t yet supported in the Google-internal version of Colab, so #4050 caused a regression there. This patch conditionally rolls back the change for Colab only. This includes rolling it back for public Colab, where it worked fine. We hope that this isn’t too big of a problem, since we expect that most Colab users are inspecting datasets with few runs (generated from within Colab) rather than massive hyperparameter sweeps. We also hope that we can simply revert this patch once the Google-internal behavior is fixed. Jupyter environments are unaffected (and still work). We used to have a global `window.TENSORBOARD_ENV` that listed whether we were in Colab, but we removed that in #2798 because we thought that it was no longer necessary. Since in that PR we switched to create an iframe rather than manually linking and loading the HTML (much nicer, to be sure), we now plumb this information via a query parameter. This also has the advantage that it’s easy to test the Colab codepaths by simply adding that query parameter to a normal TensorBoard instance. Test Plan: First, test that normal TensorBoard (`bazel run //tensorboard`) still works with multiplexing, and that adding `?tensorboardColab=true` causes it to send multiple GET requests instead. Then, build the Pip package (`bazel run //tensorboard/pip_package:extract_pip_packages`), upload it to public Colab, and install it into the runtime. Verify that the scalar charts still work there, albeit without multiplexing. Finally, cherry-pick these changes into google3 via a test sync and follow the test plan at <http://cl/333398676> to verify the fix. wchargin-branch: scalars-nomux-colab wchargin-source: 453503f1dea196985e889be483c2a7675cc87aa1
nfelt
approved these changes
Sep 24, 2020
bmd3k
added a commit
to bmd3k/tensorboard
that referenced
this pull request
Oct 5, 2020
Read the 'tensorboardColab' environment variable as an indicator of whether tensorboard is running in a colab environment. If it is true then send GET requests instead of POST requests. This check replaces an old colab-related check that stopped functioning. The new 'tensorboardColab' mechanism was introduced in tensorflow#4191.
bmd3k
added a commit
that referenced
this pull request
Oct 6, 2020
…4218) * Motivation for features / changes Hparams dashboard does not work in Google-internal colab environments because they do not support POST requests. * Technical description of changes Reuse mechanism introduced in #4191 that provides a 'tensorboardColab' query parameter in the URL to indicate a colab environment. If the value for the parameter is true then the Hparams dashboard will send GET requests. Otherwise it sends POST requests. There already existed code that purported to determine if the hparams dashboard was running in a colab environment but this code has not worked for some time, possibly since #2798. The code in this PR simply replaces that code. The plumbing to pipe this through other layers had already been implemented.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
In #4050, we added multiplexing to scalar chart fetches for performance.
This works nicely in local TensorBoard and public Colab, but the POST
request machinery isn’t yet supported in the Google-internal version of
Colab, so #4050 caused a regression there. This patch conditionally
rolls back the change for Colab only. This includes rolling it back for
public Colab, where it worked fine. We hope that this isn’t too big of a
problem, since we expect that most Colab users are inspecting datasets
with few runs (generated from within Colab) rather than massive
hyperparameter sweeps. We also hope that we can simply revert this patch
once the Google-internal behavior is fixed.
Jupyter environments are unaffected (and still work).
We used to have a global
window.TENSORBOARD_ENVthat listed whether wewere in Colab, but we removed that in #2798 because we thought that it
was no longer necessary. Since in that PR we switched to create an
iframe rather than manually linking and loading the HTML (much nicer, to
be sure), we now plumb this information via a query parameter. This also
has the advantage that it’s easy to test the Colab codepaths by simply
adding that query parameter to a normal TensorBoard instance.
Fixes #4174.
Test Plan:
First, test that normal TensorBoard (
bazel run //tensorboard) stillworks with multiplexing, and that adding
?tensorboardColab=truecausesit to send multiple GET requests instead. Then, build the Pip package
(
bazel run //tensorboard/pip_package:extract_pip_packages), upload itto public Colab, and install it into the runtime. Verify that the scalar
charts still work there, albeit without multiplexing. Finally,
cherry-pick these changes into google3 via a test sync and follow the
test plan at http://cl/333398676 to verify the fix.
wchargin-branch: scalars-nomux-colab