Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

projector: Fix standalone vz_projector server. #6069

Merged
merged 2 commits into from
Nov 28, 2022

Conversation

bmd3k
Copy link
Contributor

@bmd3k bmd3k commented Nov 22, 2022

See: #6065

We document the ability to run the projector plugin in standalone mode:
https://github.com/tensorflow/tensorboard/blob/master/tensorboard/plugins/projector/README.md

But it doesn't work.

The first thing to fix is to get the ":standalone" target to be executable and bring up a working server:

  • Hard to know exactly where things went wrong. I have found evidence that this worked as late as September 2020. I suspect things went wrong in April 2021, when tensorboard_html_binary() was replaced by tb_combine_html().
  • In the case of vz_projector it seems we can just delete the current ":standalone" target and rename the ":standalone_lib" target to ":standalone". tb_combine_html() doesn't seem to be necessary for any part of the build. tf_web_library() produces an executable web server (Amazing! Who knew?).

The second thing to fix is to get it to render a working page.

  • As is, the page would wail with the error message "Cannot read properties of null (reading 'appendChild')". In this case, the null property is document.body.
  • Again, not really sure where things went wrong but this is probably also related to the changes in April 2021.
  • The fix here is to load the js binary in the body tag (much like we do for //tensorboard target).

The third thing to fix is to get a constant port of "6006".

  • As is, the web server would start up with random ports.
  • The relevant change here is test: fix concurrent web test execution #3047, where port is set to '0'. But the reason for that change is no longer relevant - the test infrastructure the change was supporting has all been deleted.
  • The fix is to change the port for the web server back to '6006'.

@bmd3k bmd3k requested a review from rileyajones November 22, 2022 21:55
@@ -99,7 +99,7 @@ def _tf_web_library(ctx):
)
params = struct(
label=str(ctx.label),
bind="localhost:0",
bind="localhost:6006",
Copy link
Contributor

Choose a reason for hiding this comment

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

You mention in your description that the webserver can start with "random ports" but the user can also specify the port with an environment variable. In the future cleanup it would be nice to have this respect the environment variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I may have missed that: What is the environment variable that users can specify?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed offline: We don't believe that we currently offer ability to set the port.

@bmd3k bmd3k merged commit bbc9e4f into tensorflow:master Nov 28, 2022
@nfelt
Copy link
Contributor

nfelt commented Nov 29, 2022

tf_web_library() produces an executable web server (Amazing! Who knew?).

Ah yes, a glorious jart-ism from bygone days. Even more amazing, but dutifully removed by William several years ago, was the huge, cryptic ANSI art image the server used to dump to the console on startup.

More seriously, in a post-Polymer world I expect we would want to avoid any further dependency on rules_closure, which would mean giving up the amazing hidden web server. But I suppose if that world somehow still includes a standalone mode for the projector, we could port it to use some other one-off server binary instead.

qihach64 pushed a commit to qihach64/tensorboard that referenced this pull request Dec 19, 2022
See: tensorflow#6065 

We document the ability to run the projector plugin in standalone mode:

https://github.com/tensorflow/tensorboard/blob/master/tensorboard/plugins/projector/README.md

But it doesn't work.

The first thing to fix is to get the ":standalone" target to be
executable and bring up a working server:
  * Hard to know exactly where things went wrong. I have found evidence
    that this worked as late as
    [September 2020](tensorflow#4158).
    I suspect things went wrong in
    [April 2021](https://github.com/tensorflow/tensorboard/pull/4906/files),
    when tensorboard_html_binary() was replaced by tb_combine_html().
  * In the case of vz_projector it seems we can just delete the current
    ":standalone" target and rename the ":standalone_lib" target to
    ":standalone". tb_combine_html() doesn't seem to be necessary for any
    part of the build. tf_web_library() produces an executable web server
    (Amazing! Who knew?).

The second thing to fix is to get it to render a working page.
  * As is, the page would wail with the error message "Cannot read
    properties of null (reading 'appendChild')". In this case, the null
    property is `document.body`.
  * Again, not really sure where things went wrong but this is probably
    also related to the changes in April 2021.
  * The fix here is to load the js binary in the `body` tag (much like we
    do for `//tensorboard` target).

The third thing to fix is to get a constant port of "6006".
  * As is, the web server would start up with random ports.
  * The relevant change here is
    tensorflow#3047, where port is
    set to '0'. But the reason for that change is no longer relevant - the
    test infrastructure the change was supporting has all been deleted.
  * The fix is to change the port for the web server back to '6006'.
yatbear pushed a commit to yatbear/tensorboard that referenced this pull request Mar 27, 2023
See: tensorflow#6065 

We document the ability to run the projector plugin in standalone mode:

https://github.com/tensorflow/tensorboard/blob/master/tensorboard/plugins/projector/README.md

But it doesn't work.

The first thing to fix is to get the ":standalone" target to be
executable and bring up a working server:
  * Hard to know exactly where things went wrong. I have found evidence
    that this worked as late as
    [September 2020](tensorflow#4158).
    I suspect things went wrong in
    [April 2021](https://github.com/tensorflow/tensorboard/pull/4906/files),
    when tensorboard_html_binary() was replaced by tb_combine_html().
  * In the case of vz_projector it seems we can just delete the current
    ":standalone" target and rename the ":standalone_lib" target to
    ":standalone". tb_combine_html() doesn't seem to be necessary for any
    part of the build. tf_web_library() produces an executable web server
    (Amazing! Who knew?).

The second thing to fix is to get it to render a working page.
  * As is, the page would wail with the error message "Cannot read
    properties of null (reading 'appendChild')". In this case, the null
    property is `document.body`.
  * Again, not really sure where things went wrong but this is probably
    also related to the changes in April 2021.
  * The fix here is to load the js binary in the `body` tag (much like we
    do for `//tensorboard` target).

The third thing to fix is to get a constant port of "6006".
  * As is, the web server would start up with random ports.
  * The relevant change here is
    tensorflow#3047, where port is
    set to '0'. But the reason for that change is no longer relevant - the
    test infrastructure the change was supporting has all been deleted.
  * The fix is to change the port for the web server back to '6006'.
dna2github pushed a commit to dna2fork/tensorboard that referenced this pull request May 1, 2023
See: tensorflow#6065 

We document the ability to run the projector plugin in standalone mode:

https://github.com/tensorflow/tensorboard/blob/master/tensorboard/plugins/projector/README.md

But it doesn't work.

The first thing to fix is to get the ":standalone" target to be
executable and bring up a working server:
  * Hard to know exactly where things went wrong. I have found evidence
    that this worked as late as
    [September 2020](tensorflow#4158).
    I suspect things went wrong in
    [April 2021](https://github.com/tensorflow/tensorboard/pull/4906/files),
    when tensorboard_html_binary() was replaced by tb_combine_html().
  * In the case of vz_projector it seems we can just delete the current
    ":standalone" target and rename the ":standalone_lib" target to
    ":standalone". tb_combine_html() doesn't seem to be necessary for any
    part of the build. tf_web_library() produces an executable web server
    (Amazing! Who knew?).

The second thing to fix is to get it to render a working page.
  * As is, the page would wail with the error message "Cannot read
    properties of null (reading 'appendChild')". In this case, the null
    property is `document.body`.
  * Again, not really sure where things went wrong but this is probably
    also related to the changes in April 2021.
  * The fix here is to load the js binary in the `body` tag (much like we
    do for `//tensorboard` target).

The third thing to fix is to get a constant port of "6006".
  * As is, the web server would start up with random ports.
  * The relevant change here is
    tensorflow#3047, where port is
    set to '0'. But the reason for that change is no longer relevant - the
    test infrastructure the change was supporting has all been deleted.
  * The fix is to change the port for the web server back to '6006'.
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