-
Notifications
You must be signed in to change notification settings - Fork 695
NXP backend: Switch to the default ExecuTorch graph visualization. #14297
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
NXP backend: Switch to the default ExecuTorch graph visualization. #14297
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/14297
Note: Links to docs will display an error until the docs builds have been completed. ⏳ No Failures, 118 PendingAs of commit 1c79d1b with merge base 82a2324 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@pytorchbot label "module: nxp" "release notes: nxp" |
287a4eb to
c1a4c8b
Compare
|
The failing linting checks are related to imports from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chiming in with some comments, hope that's ok.
devtools/visualization/model_explorer_styles/cluster_highlight_style.json
Show resolved
Hide resolved
6a4ccae to
e16565a
Compare
Thank you for the review @Erik-Lundell . Do you have any information regarding the MYPY import warnings (the failing lintrunner)? |
|
Thanks for your adjustments! I was probably not too clear what I meant with callback, I meant that the function takes a callback(Node) -> namespace, that makes it possible to use for any type of clustering. You can then include a new function that does the particular clustering you are interested in. But my main concerns are fixed, so I'll leave the rest of the review to someone who can actually approve. Regarding mypy, I don't know, sorry. Maybe just add ignore markers? |
@Gasoonjia pinging you in case you missed the comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MartinPavella, Implementation and concept wise looks good to me.
What I recommend:
- Integrate it as example usage into nxp's
aot_neutron_examplewhere on the CLI argument e.g.--visualize=show|storethe example would either show the visualization of the model or store it to a json file based on the input model name. This will give everyone an example use and context for the reviewers, especially @Gasoonjia as code owner. - In the example store/visualize the exported quantized graph (shows the QDQ cluster visualization) and final exported program with NeutronBackend node. Optionally you can also visualize/store the output of the partitioner, although it is a bit more difficult as the CLI argument must be propagated from example to the NeutronPartitioner (e.g. via
neutron_compile_spec, although bit hacky). Or ExecuTorch'sto_edge_transform_and_lowerand/orto_edgemust be extended to support:- either propagation of visualize argument, then the visualization can be done on the level of ExecuTorch for all the backends. Or
- the aforementioned routines would return the partitioned graph .
- Document the usage in a new
mdindocs\sources\showing how to use thevisualize_with_cluster, opening a model explorer, using the predefined color scheme. - There is no documentation available how to use the visualization at all. The
docs/source/devtools-overview.mdmentions the "visualization - Comming soon" is outdated, as the visualization is already here for some time :-). You spent time in investigation, so feel free to add short guideline how to use it, this will give you a prerequisity for the documentation ofvisualize_with_clusterand fill the gap with missing visualization documentation. - Move the color coding style definition (
model_explorer_cluster_style.json) to a subfolder. This way, there will be a foundation for other backends to add their style definition to visualize their features.
@digantdesai , for the context, at our early development days we mentioned our visualization tool, based on .dot files, and discussed using the ExecuTorch visualizer instead. Martin worked out a proposal. Feel free to comment on the PR or my suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides small adjustments and documentation it looks fine.
e16565a to
5ea9a0c
Compare
79edf73 to
b71ea03
Compare
Thank you for the suggestions. I have implemented the changes except for the visualization of the partitioning. I didn't want to make such a large change to the entire pipeline within this PR. |
0cdfb63 to
6b0cb03
Compare
cdc1277 to
ee6de30
Compare
07eb96f to
6986599
Compare
|
@Erik-Lundell, @mergennachin the PR has been updated recently to better document the Model Explorer visualization. Please feel free to review the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have no comments here. Nice work!
6986599 to
88a464e
Compare
| ./devtools/install_requirements.sh | ||
| ``` | ||
|
|
||
| ## Visualize a model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this page looks great, thank you
https://docs-preview.pytorch.org/pytorch/executorch/14297/visualize.html
devtools/visualization/model_explorer_styles/cluster_highlight_style.json
Outdated
Show resolved
Hide resolved
4ea6dbd to
5a52a1f
Compare
|
The failing checks appear unrelated. Merging the PR. |
…cases. Add style .json file, which can be loaded in the Model Explorer GUI, and it changes the visualization to highlight the clusters and partitions in different colors.
5a52a1f to
1c79d1b
Compare
| @@ -0,0 +1,144 @@ | |||
| # Visualize a Model using ModelExplorer | |||
|
|
|||
| The [visualization_utils.py](../../devtools/visualization/visualization_utils.py) contains functions for | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: this doesn't point to the code but becomes a download link.

Summary
This PR updates the existing ExecuTorch graph visualization tool to suit NXP use-cases, and removes the old custom visualizer.
Test plan
N/A
cc @robert-kalmar @roman-janik-nxp @StrycekSimon @jirioc