Escape curly brackets in FxGraphDrawer _typename #83604

SungMinCho · 2022-08-17T16:49:45Z

Summary:
Encountered Error: bad label format from dot (i.e. graphviz) when benchmarking models that have dict-like structure.

The root cause was that curly brackets were not properly escaped, like this example P522499127 (unescaped curly brackets in target= string)

This diff insert the fix in FxGraphDrawer, since many of these graph generation codes rely on that class.

(Modified summary before exporting to GitHub PR)

Test Plan:

CUDA_VISIBLE_DEVICES=7 buck run mode/opt -c python.package_style=inplace //hpc/new/models/feed/benchmark:feed_lower_benchmark -- --model-name={INSERT IFR QE MODEL NAME HERE} --batch-iter 100 --batch-size 768 --num-gpu 1 --lower-presets {INSERT ITS PRESET}

Will not encounter dot errors after this diff.

(Modified test plan before exporting to GitHub PR)

Reviewed By: yinghai

Differential Revision: D38758827

facebook-github-bot · 2022-08-17T16:49:52Z

🔗 Helpful links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/83604
✖️ Python docs build was skipped
✖️ C++ docs build was skipped
❓Need help or want to give feedback on the CI? Visit our office hours
↩️ [fb-only] Re-run with SSH instructions

❌ 2 New Failures, 1 Pending

As of commit 3db7cff (more details on the Dr. CI page):

Expand to see more

2/2 failures introduced in this PR

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages

pull / linux-bionic-py3.7-clang9 / test (dynamo, 2, 2, linux.2xlarge) (1/1)

Step: "Test" (full log | diagnosis details)

2022-08-31T07:58:30.7079525Z AssertionError: to....* op returned non-Tensor int call_method data_ptr

2022-08-31T07:58:30.6998909Z     self.assertRaises(IndexError, lambda: reference[0.0, :, 0.0])
2022-08-31T07:58:30.6999225Z 
2022-08-31T07:58:30.6999370Z Set torchdynamo.config.verbose=True for more information
2022-08-31T07:58:30.6999589Z ==========
2022-08-31T07:58:30.7012315Z ok (1.983s)
2022-08-31T07:58:30.7077457Z   test_index_getitem_copy_bools_slices_cpu (__main__.TestIndexingCPU) ... torchdynamo.convert_frame: [ERROR] WON'T CONVERT test_index_getitem_copy_bools_slices test_indexing.py line 1003 
2022-08-31T07:58:30.7078096Z due to: 
2022-08-31T07:58:30.7078342Z Traceback (most recent call last):
2022-08-31T07:58:30.7078639Z   File "/var/lib/jenkins/torchdynamo/torchdynamo/variables/tensor.py", line 260, in create
2022-08-31T07:58:30.7079105Z     ), f"torch.* op returned non-Tensor {typestr(example_value)} {proxy.node.op} {proxy.node.target}"
2022-08-31T07:58:30.7079525Z AssertionError: torch.* op returned non-Tensor int call_method data_ptr
2022-08-31T07:58:30.7079697Z 
2022-08-31T07:58:30.7079772Z from user code:
2022-08-31T07:58:30.7080035Z    File "test_indexing.py", line 1010, in test_index_getitem_copy_bools_slices
2022-08-31T07:58:30.7080340Z     self.assertNotEqual(a.data_ptr(), a[True].data_ptr())
2022-08-31T07:58:30.7080499Z 
2022-08-31T07:58:30.7080625Z Set torchdynamo.config.verbose=True for more information
2022-08-31T07:58:30.7080853Z ==========
2022-08-31T07:58:30.7092008Z ok (0.007s)
2022-08-31T07:58:30.8456057Z   test_index_put_accumulate_duplicate_indices_cpu (__main__.TestIndexingCPU) ... ok (0.136s)
2022-08-31T07:58:30.9284098Z   test_index_put_accumulate_expanded_values_cpu (__main__.TestIndexingCPU) ... ok (0.082s)

🕵️‍♀️ 1 failure not recognized by patterns:

The following CI failures may be due to changes from the PR

Job	Step
^build	^Unknown

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

facebook-github-bot · 2022-08-17T16:50:37Z

This pull request was exported from Phabricator. Differential Revision: D38758827

jamesr66a · 2022-08-24T18:55:24Z

torch/fx/passes/graph_drawer.py


-            return _get_qualified_name(target)
+            # Escape "{" and "}" to prevent dot files like P522499127


This paste is not accessible to OSS users. Please move it to e.g. a github gist

Updated it to github gist!

facebook-github-bot · 2022-08-25T01:31:43Z

This pull request was exported from Phabricator. Differential Revision: D38758827

facebook-github-bot · 2022-08-25T01:40:21Z

This pull request was exported from Phabricator. Differential Revision: D38758827

facebook-github-bot · 2022-08-28T19:58:27Z

This pull request was exported from Phabricator. Differential Revision: D38758827

Summary: Pull Request resolved: pytorch#83604 Encountered `Error: bad label format` from dot (i.e. graphviz) when benchmarking models that have dict-like structure. The root cause was that curly brackets were not properly escaped, like this example P522499127 (unescaped curly brackets in target= string) This diff insert the fix in FxGraphDrawer, since many of these graph generation codes rely on that class. (Modified summary before exporting to GitHub PR) Test Plan: ``` CUDA_VISIBLE_DEVICES=7 buck run mode/opt -c python.package_style=inplace //hpc/new/models/feed/benchmark:feed_lower_benchmark -- --model-name={INSERT IFR QE MODEL NAME HERE} --batch-iter 100 --batch-size 768 --num-gpu 1 --lower-presets {INSERT ITS PRESET} ``` Will not encounter dot errors after this diff. (Modified test plan before exporting to GitHub PR) Reviewed By: yinghai, jianyuh Differential Revision: D38758827 fbshipit-source-id: d5d1563181dce2fec446b545cf41cf4ef40a03b8

facebook-github-bot · 2022-08-30T20:09:49Z

This pull request was exported from Phabricator. Differential Revision: D38758827

SungMinCho · 2022-08-31T15:09:50Z

@pytorchbot merge

pytorchmergebot · 2022-08-31T15:11:23Z

@pytorchbot successfully started a merge job. Check the current status here.
The merge job was triggered without a flag. This means that your change will be merged once all checks on your PR have passed (ETA: 0-4 Hours). If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

pytorchmergebot · 2022-08-31T15:11:25Z

Merge failed

Reason: View failures on hud. Refusing to merge as mandatory check(s) pull failed for rule superuser.

Details for Dev Infra team

Raised by workflow job

SungMinCho · 2022-08-31T15:12:24Z

@pytorchbot merge --force

pytorch-bot · 2022-08-31T15:12:28Z

❌ 🤖 pytorchbot command failed:

@pytorchbot merge: error: argument -f/--force: expected one argument

usage: @pytorchbot merge [-g | -f MESSAGE | -l]

Try @pytorchbot --help for more info.

SungMinCho · 2022-08-31T15:13:35Z

@pytorchbot merge --force "force merging, permitted by Sergii"

pytorchmergebot · 2022-08-31T15:15:17Z

@pytorchbot successfully started a merge job. Check the current status here.
The merge job was triggered with the force (-f) flag. This means your change will be merged immediately, bypassing any CI checks (ETA: 1-5 minutes). If this is not the intended behavior, feel free to use some of the other merge options in the wiki.
Please reach out to the PyTorch DevX Team with feedback or questions!

github-actions · 2022-08-31T15:16:05Z

Hey @SungMinCho.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

Summary: Encountered `Error: bad label format` from dot (i.e. graphviz) when benchmarking models that have dict-like structure. The root cause was that curly brackets were not properly escaped, like this example P522499127 (unescaped curly brackets in target= string) This diff insert the fix in FxGraphDrawer, since many of these graph generation codes rely on that class. (Modified summary before exporting to GitHub PR) Pull Request resolved: #83604 Approved by: https://github.com/yinghai, https://github.com/jianyuh Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/bf67589915de07a6b8756a685de9abbd90ec2dfa Test plan from GitHub: ``` CUDA_VISIBLE_DEVICES=7 buck run mode/opt -c python.package_style=inplace //hpc/new/models/feed/benchmark:feed_lower_benchmark -- --model-name={INSERT IFR QE MODEL NAME HERE} --batch-iter 100 --batch-size 768 --num-gpu 1 --lower-presets {INSERT ITS PRESET} ``` Will not encounter dot errors after this diff. (Modified test plan before exporting to GitHub PR) Reviewed By: yinghai, mehtanirav, jianyuh Differential Revision: D38758827 Pulled By: SungMinCho fbshipit-source-id: f33f372b454563ad9656dd7850ef591e58d5f46c

facebook-github-bot added the cla signed label Aug 17, 2022

facebook-github-bot added fx fb-exported labels Aug 17, 2022

yinghai approved these changes Aug 20, 2022

View reviewed changes

jianyuh approved these changes Aug 22, 2022

View reviewed changes

jamesr66a reviewed Aug 24, 2022

View reviewed changes

SungMinCho force-pushed the export-D38758827 branch from c494bfa to c06781a Compare August 25, 2022 01:31

SungMinCho force-pushed the export-D38758827 branch from c06781a to 677b5f5 Compare August 25, 2022 01:40

SungMinCho force-pushed the export-D38758827 branch from 677b5f5 to 0a1abff Compare August 28, 2022 19:58

SungMinCho force-pushed the export-D38758827 branch from 0a1abff to 3db7cff Compare August 30, 2022 20:09

pytorchmergebot added the Merged label Aug 31, 2022

pytorchmergebot closed this in bf67589 Aug 31, 2022

SungMinCho added release notes: fx release notes category topic: bug fixes topic category labels Aug 31, 2022


		return _get_qualified_name(target)
		# Escape "{" and "}" to prevent dot files like P522499127

Escape curly brackets in FxGraphDrawer _typename #83604

Escape curly brackets in FxGraphDrawer _typename #83604

Uh oh!

Conversation

SungMinCho commented Aug 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Aug 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful links

❌ 2 New Failures, 1 Pending

🕵️ 1 new failure recognized by patterns

pull / linux-bionic-py3.7-clang9 / test (dynamo, 2, 2, linux.2xlarge) (1/1)

🕵️‍♀️ 1 failure not recognized by patterns:

Uh oh!

facebook-github-bot commented Aug 17, 2022

Uh oh!

jamesr66a Aug 24, 2022

Choose a reason for hiding this comment

Uh oh!

SungMinCho Aug 25, 2022

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 25, 2022

Uh oh!

facebook-github-bot commented Aug 25, 2022

Uh oh!

facebook-github-bot commented Aug 28, 2022

Uh oh!

facebook-github-bot commented Aug 30, 2022

Uh oh!

SungMinCho commented Aug 31, 2022

Uh oh!

pytorchmergebot commented Aug 31, 2022

Uh oh!

pytorchmergebot commented Aug 31, 2022

Merge failed

Uh oh!

SungMinCho commented Aug 31, 2022

Uh oh!

pytorch-bot bot commented Aug 31, 2022

Uh oh!

SungMinCho commented Aug 31, 2022

Uh oh!

pytorchmergebot commented Aug 31, 2022

Uh oh!

github-actions bot commented Aug 31, 2022

Uh oh!

Uh oh!

SungMinCho commented Aug 17, 2022 •

edited

Loading

facebook-github-bot commented Aug 17, 2022 •

edited

Loading