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

graph: color large graphs by 'none' instead of 'structure' by default (4) #4742

Merged
merged 3 commits into from
Mar 9, 2021

Conversation

psybuzz
Copy link
Contributor

@psybuzz psybuzz commented Mar 6, 2021

The "Color by structure" mode can be very expensive to compute,
often consistently freezing TensorBoard for minutes with some large
graphs. This introduces a new "Color by none", which does not color
groups by structure.

Opening TensorBoard's Graphs with a small graph will still default to
"structure" coloring, but when loading a large graph, it will
automatically switch to "none", requiring the user to opt-in by clicking
on the radio button.

The condition for a large graph is "10K+ nodes && 10K+ edges", but
this can be tuned. These were determined by examining several
graphs manually:

(# nodes, # edges) Time computing templates (sec) Time to load (after) Notes
(12, 19) 0.004 <3s
(125, 144) 0.007 <3s
(233, 410) 0.013 <3s
(361, 394) 0.011s <3s Resnet8
(601, 956) 0.013s <3s Mnist
(603, 955) 0.016s <3s Recommender
(6K, 9K) 0.4s <3s Cifar
(35K, 1,968K) 3.6s 24s
(160K, 660K) unknown (2m+) 1m45s
(224K, 229K) 102s 23s

Screenshot
image

Diffbase: #4741
Followup: none

@google-cla google-cla bot added the cla: yes label Mar 6, 2021
@psybuzz psybuzz changed the base branch from master to pz-colorbynone3 March 6, 2021 01:07
@psybuzz psybuzz force-pushed the pz-colorbynone4 branch 2 times, most recently from 11b34c7 to 53d6f16 Compare March 6, 2021 02:14
@psybuzz psybuzz marked this pull request as ready for review March 6, 2021 02:15
@psybuzz psybuzz requested a review from stephanwlee March 6, 2021 02:15
@stephanwlee
Copy link
Contributor

Thanks for attaching the table. It was insightful.

Comment on lines 34 to 37
const MAX_GRAPH_SIZE_TO_COMPUTE_TEMPLATES = {
maxNodeCount: 10000,
maxEdgeCount: 10000,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

style thing: often, consts invert the casing :P

i.e.,

const MaxGraphSizeToComputeTemplates = {
  MAX_NODE_COUNT: 10000,
  MAX_Edge_COUNT: 10000,
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I'll remember for next time!

Copy link
Contributor Author

@psybuzz psybuzz left a comment

Choose a reason for hiding this comment

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

Thanks for the reviews!

Comment on lines 34 to 37
const MAX_GRAPH_SIZE_TO_COMPUTE_TEMPLATES = {
maxNodeCount: 10000,
maxEdgeCount: 10000,
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I'll remember for next time!

Base automatically changed from pz-colorbynone3 to master March 9, 2021 18:48
@psybuzz psybuzz merged commit 0ef26f3 into master Mar 9, 2021
@psybuzz psybuzz deleted the pz-colorbynone4 branch March 9, 2021 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants