Fix duplicate pattern error by eellison · Pull Request #139321 · pytorch/pytorch

eellison · 2024-10-30T21:53:07Z

Stack from ghstack (oldest at bottom):

-> Fix duplicate pattern error #139321

vllm had an error when we were incorrectly stating two patterns are duplicates. See, comment inline:

For a particular generated pattern repr, store all the equivalent graphs that used to generate them. Because we ignore certain patterns in searching, but not in matching, use the graph to distinguish if two equivalent searches are actually different.

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

…rch patterns but different match [ghstack-poisoned]

pytorch-bot · 2024-10-30T21:53:10Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

✅ No Failures

As of commit 6a86ef7 with merge base cf76c05 ():
💚 Looks good so far! There are no failures yet. 💚

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

…rch patterns but different match ghstack-source-id: b9efb7f Pull Request resolved: #139321

ProExpertProg · 2024-10-31T20:57:56Z

Any reason _seen_patterns is not a property of PatternMatcherPass? Running into some weird lifetime semantics because a pattern can only be registered once.

eellison · 2024-11-01T18:14:54Z

@ProExpertProg yes, agreed, they could be scoped to a pass. will update

vllm had an error when we were incorrectly stating two patterns are duplicates. See, comment inline: For a particular generated pattern repr, store all the equivalent graphs that used to generate them. Because we ignore certain patterns in searching, but not in matching, use the graph to distinguish if two equivalent searches are actually different. Element are first inserted as graphs and lazily converted to their str reprs when there is a duplicate. If the graph is not present we will error on duplicate. cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov [ghstack-poisoned]

…rch patterns but different match ghstack-source-id: 49a8bb8 Pull Request resolved: #139321

eellison · 2024-12-26T19:04:27Z

@pytorchbot rebase

pytorchmergebot · 2024-12-26T19:05:57Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

pytorchmergebot · 2024-12-26T19:06:03Z

Rebase failed due to Command git -C /home/runner/work/pytorch/pytorch rebase refs/remotes/origin/viable/strict gh/eellison/716/orig returned non-zero exit code 1

Rebasing (1/1)
Auto-merging test/inductor/test_pattern_matcher.py
Auto-merging torch/_inductor/pattern_matcher.py
CONFLICT (content): Merge conflict in torch/_inductor/pattern_matcher.py
error: could not apply 2071620ae60... Fix incorrect duplicate pattern on patterns which have equivalent search patterns but different match
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Could not apply 2071620ae60... Fix incorrect duplicate pattern on patterns which have equivalent search patterns but different match

Raised by https://github.com/pytorch/pytorch/actions/runs/12507427410

[ghstack-poisoned]

shunting314

So the PR change the duplication check from

check string representation of pattern
to
check string representation of the fx graph generating the pattern
?

My questions is, why two patterns with the same string representation could be generated by different fx graphs (in the vllm case). Due to Ignored() pattern?

[ghstack-poisoned]

eellison · 2024-12-26T20:32:46Z

@shunting314 yes, due to Ignored. The actual search graph is the same but the underlying pattern is different.

[ghstack-poisoned]

malfet · 2024-12-30T06:26:10Z

+            return x - 2
+
+        patterns = PatternMatcherPass()
+        torch.set_default_device("cuda")


Wouldn't that potentially affect other tests in a very weird ways? Shouldn't one better use with torch.device("cuda"): or just alloc next tensor on the device you want?

Hi, @eellison can you help to fix the issue #143974 before re-land? Just use GPU_TYPE instead of "cuda" as #143975 do.

@malfet was reusing repro and forgot to update, agreed. although having said that - there are a few tests which use this pattern.. so if it is harmful, we should fix them.

vllm had an error when we were incorrectly stating two patterns are duplicates. See, comment inline: For a particular generated pattern repr, store all the equivalent graphs that used to generate them. Because we ignore certain patterns in searching, but not in matching, use the graph to distinguish if two equivalent searches are actually different. Pull Request resolved: pytorch#139321 Approved by: https://github.com/shunting314

…de in (#143975)" This reverts commit 7c1c073. Reverted #143975 on behalf of https://github.com/jeanschmidt due to Need to revert in order to be able to revert #139321 feel free to merge it back once conflicts are cleared ([comment](#143975 (comment)))

…143975) test_pattern_matcher.py Fix #143974 Pull Request resolved: #143975 Approved by: https://github.com/malfet

jeanschmidt · 2024-12-31T17:41:14Z

@pytorchbot revert -m "breaking internal signals" -c ghfirst

pytorchmergebot · 2024-12-31T17:43:52Z

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot · 2024-12-31T17:44:04Z

@eellison your PR has been successfully reverted.

This reverts commit 9e8d84f. Reverted #139321 on behalf of https://github.com/jeanschmidt due to breaking internal signals ([comment](#139321 (comment)))

[ghstack-poisoned]

…rch patterns but different match ghstack-source-id: 3c883ef Pull Request resolved: #139321

eellison · 2025-01-06T01:57:35Z

@pytorchbot merge

pytorchmergebot · 2025-01-06T01:59:26Z

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

…device-bias hard code in" Re-land #143975. Fix "cuda" hard code in test_pattern_matcher.py introduced by #139321 Fix #143974 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov [ghstack-poisoned]

… code in" Re-land #143975. Fix "cuda" hard code in test_pattern_matcher.py introduced by #139321 Fix #143974 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov [ghstack-poisoned]

…device-bias hard code in" Re-land #143975. Fix "cuda" hard code in test_pattern_matcher.py introduced by #139321 Fix #143974 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov [ghstack-poisoned]

… code in" Re-land #143975. Fix "cuda" hard code in test_pattern_matcher.py introduced by #139321 Fix #143974 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov [ghstack-poisoned]

…144456) Re-land #143975. Fix "cuda" hard code in test_pattern_matcher.py introduced by #139321 Fix #143974 Pull Request resolved: #144456 Approved by: https://github.com/EikanWang, https://github.com/malfet, https://github.com/jansel ghstack dependencies: #144457

Fix incorrect duplicate pattern on patterns which have equivalent sea…

d8ed05e

…rch patterns but different match [ghstack-poisoned]

eellison added a commit that referenced this pull request Oct 30, 2024

Fix incorrect duplicate pattern on patterns which have equivalent sea…

e6be76c

…rch patterns but different match ghstack-source-id: b9efb7f Pull Request resolved: #139321

pytorch-bot Bot added ciflow/inductor module: inductor labels Oct 30, 2024

eellison changed the title ~~Fix incorrect duplicate pattern on patterns which have equivalent search patterns but different match~~ Fix duplicate pattern error for patterns with different match graphs Oct 30, 2024

eellison changed the title ~~Fix duplicate pattern error for patterns with different match graphs~~ Fix duplicate pattern error Oct 30, 2024

eellison added a commit that referenced this pull request Nov 13, 2024

Fix incorrect duplicate pattern on patterns which have equivalent sea…

2071620

…rch patterns but different match ghstack-source-id: 49a8bb8 Pull Request resolved: #139321

eellison added the topic: not user facing topic category label Dec 26, 2024

Update

9f22058

[ghstack-poisoned]

eellison mentioned this pull request Dec 26, 2024

Add a register_replacement to fix float8 delayed scaling kernel fusion issues #143464

Closed

eellison requested review from BoyuanFeng, jansel and shunting314 and removed request for BoyuanFeng, jansel and shunting314 December 26, 2024 19:56

shunting314 reviewed Dec 26, 2024

View reviewed changes

Comment thread torch/_inductor/pattern_matcher.py Outdated

Update

521ef66

[ghstack-poisoned]

Update

4b6705a

[ghstack-poisoned]

etaf mentioned this pull request Dec 30, 2024

[Break XPU] Hard code “cuda” in GPU test case cause failure on XPU. #143974

Closed

malfet reviewed Dec 30, 2024

View reviewed changes

jeanschmidt mentioned this pull request Dec 31, 2024

[Inductor UT] Generalize newly introduced device-bias hard code in #143975

Closed

pytorchmergebot referenced this pull request Dec 31, 2024

[Inductor UT] Generalize newly introduced device-bias hard code in (#…

7c1c073

…143975) test_pattern_matcher.py Fix #143974 Pull Request resolved: #143975 Approved by: https://github.com/malfet

pytorchmergebot added a commit that referenced this pull request Dec 31, 2024

Revert "Fix duplicate pattern error (#139321)"

a174ee2

This reverts commit 9e8d84f. Reverted #139321 on behalf of https://github.com/jeanschmidt due to breaking internal signals ([comment](#139321 (comment)))

pytorchmergebot added Reverted ci-no-td Do not run TD on this PR labels Dec 31, 2024

pytorchmergebot reopened this Dec 31, 2024

y-sq mentioned this pull request Jan 2, 2025

Add a register_replacement to fix float8 delayed scaling kernel fusion issues in torchao/float8 pytorch/ao#1469

Merged

Update

6a86ef7

[ghstack-poisoned]

eellison added a commit that referenced this pull request Jan 6, 2025

Fix incorrect duplicate pattern on patterns which have equivalent sea…

1e66f38

…rch patterns but different match ghstack-source-id: 3c883ef Pull Request resolved: #139321

pytorchmergebot added the merging label Jan 6, 2025

pytorchmergebot closed this in e1622dc Jan 6, 2025

pytorchmergebot removed the merging label Jan 6, 2025

etaf mentioned this pull request Jan 9, 2025

[Inductor UT] Generalize newly introduced device-bias hard code in #144456

Closed

github-actions Bot deleted the gh/eellison/716/head branch February 9, 2025 02:09

zou3519 mentioned this pull request Apr 30, 2025

[torch.compile] Add torch inductor pass for fusing silu_and_mul with subsequent scaled_fp8_quant operations vllm-project/vllm#10867

Merged

Conversation

eellison commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot Bot commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ No Failures

Uh oh!

ProExpertProg commented Oct 31, 2024

Uh oh!

eellison commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eellison commented Dec 26, 2024

Uh oh!

pytorchmergebot commented Dec 26, 2024

Uh oh!

pytorchmergebot commented Dec 26, 2024

Uh oh!

shunting314 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eellison commented Dec 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

malfet Dec 30, 2024

Choose a reason for hiding this comment

Uh oh!

etaf Jan 2, 2025

Choose a reason for hiding this comment

Uh oh!

eellison Jan 6, 2025

Choose a reason for hiding this comment

Uh oh!

jeanschmidt commented Dec 31, 2024

Uh oh!

pytorchmergebot commented Dec 31, 2024

Uh oh!

pytorchmergebot commented Dec 31, 2024

Uh oh!

eellison commented Jan 6, 2025

Uh oh!

pytorchmergebot commented Jan 6, 2025

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

eellison commented Oct 30, 2024 •

edited

Loading

pytorch-bot Bot commented Oct 30, 2024 •

edited

Loading

eellison commented Nov 1, 2024 •

edited

Loading

eellison commented Dec 26, 2024 •

edited

Loading