Skip to content

[inductor] add a config to specify the shape attribute for the generated svg graphs #114811

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

Closed
wants to merge 1 commit into from

Conversation

chenyang78
Copy link
Contributor

@chenyang78 chenyang78 commented Nov 29, 2023

Stack from ghstack (oldest at bottom):

We draw our fx graphs with the "record" shape attribute by default.
Sometimes, when the graph is very complex, we may hit dot errors like below:
"flat edge between adjacent nodes one of which has a record shape -
replace records with HTML-like labels"
and thus fail to generate a graph. So, let's give the user an option
to specify the shape attribute for the dot graph. For example, passing
INDUCTOR_DOT_GRAPH_SHAPE_SVG = "none" would let us generate HTML-like lables
to workaround the above failure.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @kadeng @muchulee8 @aakhundov @ColinPeppler

…ted svg graphs

We draw our fx graphs with the "record" shape attribute by default.
Sometimes, when the graph is very complex, we may hit dot errors like below:
  "flat edge between adjacent nodes one of which has a record shape -
   replace records with HTML-like labels"
and thus fail to generate a graph. So, let's give the user an option
to specify the shape attribute for the dot graph. For example, passing
INDUCTOR_DOT_GRAPH_SHAPE_SVG = "none" would let us generate HTML-like lables
to workaround the above failure.

[ghstack-poisoned]
@chenyang78 chenyang78 requested a review from weifengpy November 29, 2023 23:57
@pytorch-bot pytorch-bot bot added the release notes: fx release notes category label Nov 29, 2023
@chenyang78 chenyang78 requested a review from Chillee November 29, 2023 23:57
Copy link

pytorch-bot bot commented Nov 29, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/114811

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 778b8f0 with merge base 0ced55e (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@chenyang78 chenyang78 requested a review from mlazos November 29, 2023 23:58
chenyang78 added a commit that referenced this pull request Nov 29, 2023
…ted svg graphs

We draw our fx graphs with the "record" shape attribute by default.
Sometimes, when the graph is very complex, we may hit dot errors like below:
  "flat edge between adjacent nodes one of which has a record shape -
   replace records with HTML-like labels"
and thus fail to generate a graph. So, let's give the user an option
to specify the shape attribute for the dot graph. For example, passing
INDUCTOR_DOT_GRAPH_SHAPE_SVG = "none" would let us generate HTML-like lables
to workaround the above failure.

ghstack-source-id: e287acd
Pull Request resolved: #114811
Copy link
Contributor

@weifengpy weifengpy 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 fix. I met the same error before and shape=record seem to complain a lot when drawing siblings. there are some CIs in progress but I assume it's good to land if CI passed.

there are some subclass in fbcode inherits FxGraphDrawer. since we are adding a new param, you might need to be keep an eye on the diff train if it breaks fbcode CI. I did a similar fix before D49749670)

@chenyang78
Copy link
Contributor Author

thanks for the fix. I met the same error before and shape=record seem to complain a lot when drawing siblings. there are some CIs in progress but I assume it's good to land if CI passed.

there are some subclass in fbcode inherits FxGraphDrawer. since we are adding a new param, you might need to be keep an eye on the diff train if it breaks fbcode CI. I did a similar fix before D49749670)

Thanks. Will keep an eye on those internal cases.

@chenyang78 chenyang78 added the topic: not user facing topic category label Nov 30, 2023
@chenyang78
Copy link
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 30, 2023
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@facebook-github-bot facebook-github-bot deleted the gh/chenyang78/5/head branch December 3, 2023 15:27
dmenig pushed a commit to dmenig/pytorch that referenced this pull request Dec 21, 2023
…ted svg graphs (pytorch#114811)

We draw our fx graphs with the "record" shape attribute by default.
Sometimes, when the graph is very complex, we may hit dot errors like below:
  "flat edge between adjacent nodes one of which has a record shape -
   replace records with HTML-like labels"
and thus fail to generate a graph. So, let's give the user an option
to specify the shape attribute for the dot graph. For example, passing
INDUCTOR_DOT_GRAPH_SHAPE_SVG = "none" would let us generate HTML-like lables
to workaround the above failure.

Pull Request resolved: pytorch#114811
Approved by: https://github.com/weifengpy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: inductor release notes: fx release notes category topic: not user facing topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants