[inductor] Make serialized inductor patterns path configurable instead of using … #145243

kareemshaik80 · 2025-01-21T04:42:58Z

…fixed path in the inductor module

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

…fixed path in the inductor module Signed-off-by: kareem <[email protected]>

pytorch-bot · 2025-01-21T04:43:02Z

🔗 Helpful Links

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

📄 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 5b635b7 with merge base a4fdae5 ():
💚 Looks good so far! There are no failures yet. 💚

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

Signed-off-by: kareem <[email protected]>

kareemshaik80 · 2025-01-21T10:57:25Z

@pytorchbot release notes: inductor

pytorch-bot · 2025-01-21T10:57:28Z

❌ 🤖 pytorchbot command failed:

@pytorchbot: error: argument command: invalid choice: 'release' (choose from 'merge', 'revert', 'rebase', 'label', 'drci', 'cherry-pick', 'close')

usage: @pytorchbot [-h] {merge,revert,rebase,label,drci,cherry-pick,close} ...

Try @pytorchbot --help for more info.

kareemshaik80 · 2025-01-23T03:23:12Z

@pytorchbot label "release notes: inductor"

Valentine233 · 2025-01-23T05:12:07Z

One question is why we need other pattern paths instead of the default one? Can you elaborate on the issue we encounter?

kareemshaik80 · 2025-01-23T08:30:00Z

One question is why we need other pattern paths instead of the default one? Can you elaborate on the issue we encounter?

To reuse the same patteren matcher module for some other backend devices, and with different decmposition and generate custom patterens. This path is tightly coupled with the inductor module.

aorenste

Looks generally fine to me - just a couple small things that should either be fixed or discussed.

torch/_inductor/config.py

torch/_inductor/pattern_matcher.py

Signed-off-by: kareem <[email protected]>

aorenste · 2025-02-04T21:38:47Z

torch/_inductor/pattern_matcher.py

        return f"{file_template}{formatted_imports}"

-    if not SERIALIZED_PATTERN_PATH.is_dir():
+    if not TORCHINDUCTOR_SERIALIZED_PATTERN_PATH.is_dir():


Please revert renaming this variable. When I asked to rename the envvar I was just talking about the external-facing string (as you did above), not the internal name. We're already in inductor - no need to churn the codebase unnecessarily.

Signed-off-by: kareem <[email protected]>

kareemshaik80 · 2025-02-17T08:39:13Z

@aorenste can you pls review?

torch/_inductor/config.py

pralay-das · 2025-04-21T10:26:07Z

Hi @aorenste, could you review this PR again ?

torch/_inductor/config.py

Signed-off-by: kareem <[email protected]>

kareemshaik80 · 2025-04-24T04:26:36Z

@aorenste resolved review comments can you pls review and get this merged

aorenste

@kareemshaik80 You have not addressed all the feedback comments.

In particular you keep fixing one problem and reverting the fix for another:

The environment variable should be "TORCHINDUCTOR_SERIALIZED_PATTERN_PATH". In the latest version this is correct - torch/_inductor/config.py[133]
The variable SERIALIZED_PATTERN_PATH should not be renamed. This was fixed in a previous version but was re-broken in the latest version - torch/_inductor/pattern_matcher.py[1648]

* Added support for configurable serialized_path in inductor pattern_matcher upstream PR: pytorch#145243 small fix for upstream PR is also included here Reference: https://stackoverflow.com/questions/67631/how-can-i-import-a-module-dynamically-given-the-full-path Change-Id: I1a029de3c142bdb9eb1fe7268751e19376ce5c23 * Modified serialized_path Change-Id: I1a029de3c142bdb9eb1fe7268751e19376ce5c23 * Modified serialized_path after solved merge conflict Change-Id: I60459f6212c6432d003e2966be9a2bdd03047a40

github-actions · 2025-06-23T18:42:14Z

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

* Added support for configurable serialized_path in inductor pattern_matcher upstream PR: pytorch#145243 small fix for upstream PR is also included here Reference: https://stackoverflow.com/questions/67631/how-can-i-import-a-module-dynamically-given-the-full-path Change-Id: I1a029de3c142bdb9eb1fe7268751e19376ce5c23 * Modified serialized_path Change-Id: I1a029de3c142bdb9eb1fe7268751e19376ce5c23 * Modified serialized_path after solved merge conflict Change-Id: I60459f6212c6432d003e2966be9a2bdd03047a40

Make serialized inductor patterns path configurable instead of using …

88c9901

…fixed path in the inductor module Signed-off-by: kareem <[email protected]>

pytorch-bot bot added the module: inductor label Jan 21, 2025

pytorchbot added the open source label Jan 21, 2025

fix module loading issues

ba43a10

Signed-off-by: kareem <[email protected]>

kareemshaik80 changed the title ~~Make serialized inductor patterns path configurable instead of using …~~ [inductor] Make serialized inductor patterns path configurable instead of using … Jan 21, 2025

mikaylagawarecki requested a review from eellison January 23, 2025 01:15

mikaylagawarecki added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jan 23, 2025

pytorch-bot bot added the release notes: inductor label Jan 23, 2025

eellison requested a review from aorenste January 27, 2025 18:31

aorenste requested changes Jan 27, 2025

View reviewed changes

torch/_inductor/config.py Outdated Show resolved Hide resolved

torch/_inductor/pattern_matcher.py Show resolved Hide resolved

eellison removed their request for review January 27, 2025 19:41

kareemshaik80 and others added 2 commits January 30, 2025 08:26

Merge branch 'pytorch:main' into expose_inductor_patt_path

841d2d5

reslove review comments and rebase.

beeaecd

Signed-off-by: kareem <[email protected]>

kareemshaik80 requested a review from aorenste January 30, 2025 03:15

aorenste approved these changes Jan 30, 2025

View reviewed changes

kareemshaik80 and others added 2 commits February 3, 2025 21:35

Merge branch 'pytorch:main' into expose_inductor_patt_path

d6c52e7

fix lint errors

5c80e38

Signed-off-by: kareem <[email protected]>

kareemshaik80 requested a review from aorenste February 4, 2025 02:54

aorenste requested changes Feb 4, 2025

View reviewed changes

kareemshaik80 and others added 3 commits February 11, 2025 08:22

Merge branch 'pytorch:main' into expose_inductor_patt_path

572e072

fix review comments

335560b

Signed-off-by: kareem <[email protected]>

Merge branch 'pytorch:main' into expose_inductor_patt_path

379202f

kareemshaik80 requested a review from aorenste February 12, 2025 08:01

aorenste requested changes Feb 18, 2025

View reviewed changes

torch/_inductor/config.py Outdated Show resolved Hide resolved

Merge branch 'pytorch:main' into expose_inductor_patt_path

e9f6a36

aorenste requested changes Apr 21, 2025

View reviewed changes

torch/_inductor/config.py Outdated Show resolved Hide resolved

kareemshaik80 and others added 2 commits April 22, 2025 08:37

Merge branch 'pytorch:main' into expose_inductor_patt_path

c02cfa9

resolve review comments

5b635b7

Signed-off-by: kareem <[email protected]>

kareemshaik80 requested a review from aorenste April 23, 2025 04:59

aorenste requested changes Apr 24, 2025

View reviewed changes

github-actions bot added the Stale label Jun 23, 2025

github-actions bot closed this Jul 23, 2025

[inductor] Make serialized inductor patterns path configurable instead of using … #145243

[inductor] Make serialized inductor patterns path configurable instead of using … #145243

Uh oh!

Conversation

kareemshaik80 commented Jan 21, 2025 • edited by pytorch-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Jan 21, 2025 • 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/145243

✅ No Failures

Uh oh!

kareemshaik80 commented Jan 21, 2025

Uh oh!

pytorch-bot bot commented Jan 21, 2025

Uh oh!

kareemshaik80 commented Jan 23, 2025

Uh oh!

Valentine233 commented Jan 23, 2025

Uh oh!

kareemshaik80 commented Jan 23, 2025

Uh oh!

aorenste left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aorenste Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

kareemshaik80 commented Feb 17, 2025

Uh oh!

Uh oh!

pralay-das commented Apr 21, 2025

Uh oh!

Uh oh!

kareemshaik80 commented Apr 24, 2025

Uh oh!

aorenste left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kareemshaik80 commented Jan 21, 2025 •

edited by pytorch-bot bot

Loading

pytorch-bot bot commented Jan 21, 2025 •

edited

Loading