Dynamic shape guards in C++ #139899

isuruf · 2024-11-06T17:12:01Z

Stack from ghstack (oldest at bottom):

cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10 @ezyang @SherlockNoMad @EikanWang @wenzhe-nrv @voznesenskym @penguinwu @Guobing-Chen @zhuhaozhe @blzheng @jiayisunx @chenyang78 @kadeng @chauhang @amjames

[ghstack-poisoned]

pytorch-bot · 2024-11-06T17:12:04Z

🔗 Helpful Links

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

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

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

ghstack-source-id: ef9fab3 Pull Request resolved: #139899

isuruf · 2024-11-06T17:18:02Z

cc @anijain2305

[ghstack-poisoned]

ghstack-source-id: 454ac7a Pull Request resolved: #139899

[ghstack-poisoned]

ghstack-source-id: 0708511 Pull Request resolved: #139899

[ghstack-poisoned]

ghstack-source-id: fb14b80 Pull Request resolved: #139899

[ghstack-poisoned]

ghstack-source-id: 3f5bcbb Pull Request resolved: #139899

ezyang · 2024-11-07T16:58:14Z

torch/_dynamo/guards.py

+            import sympy
+
+            from torch._inductor.codecache import CppCodeCache
+            from torch._inductor.codegen.cpp_utils import cexpr


Sigh, layering violation lol

ezyang · 2024-11-07T16:59:56Z

torch/fx/experimental/symbolic_shapes.py

-    ) -> Tuple[List[str], List[str]]:  # regular, verbose
+    ) -> Tuple[
+        List[str], List[str], List[Tuple[sympy.Expr, Dict[str, List[Source]]]]
+    ]:  # regular, verbose, sympy


Re the changes here, would it make more sense to just directly do C++ rendering here directly? This prevents you from having to invent one-off sympy functions for special concepts we don't normally have in sympy. In general sympy IR is not a good IR for driving C++ codegen, IMO.

anijain2305 · 2024-11-09T22:40:29Z

torch/csrc/dynamo/guards.cpp

+    if (!THPVariable_CheckExact(obj) && !THPVariable_Check(obj)) {
+      return false;
+    }


I think we should be able to skip this check. Because in the guard tree, this accessor will only be called from a GuardManager for a tensor.

Just a ping on this.

torch/csrc/dynamo/guards.cpp

anijain2305 · 2024-11-09T22:55:05Z

I have done a quick non-rigorous review. The flow looks good to me. A few things that will help

Maybe we should move the code sitting in inductor for sympy C++ translator to symbolic shapes first in a separate PR.
Can you write some comments on the IndexManager feeding into Symbolic Shape Guard? A simple example directly in the cpp file, that shows the translated code and how IndexedManager helps you construct the arguments to the translated function, is good enough.
We should keep the flag off for now. You can turn it on in tests. Atleast not turn it on in this PR itself, because the reverts could be painful then.

anijain2305 · 2024-11-09T22:55:38Z

cc @jansel as well

[ghstack-poisoned]

pytorchmergebot · 2025-01-22T05:22:37Z

Merge failed

Reason: Command git -C /home/runner/work/pytorch/pytorch cherry-pick -x 106221246a400da5a6c4e8c01b9d31e14fedf3ab returned non-zero exit code 1

Auto-merging torch/_dynamo/guards.py
Auto-merging torch/fx/experimental/symbolic_shapes.py
CONFLICT (content): Merge conflict in torch/fx/experimental/symbolic_shapes.py
error: could not apply 106221246a... Add a langs option for symbolic shape guards
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"

Details for Dev Infra team

Raised by workflow job

[ghstack-poisoned]

isuruf · 2025-01-22T14:50:00Z

@pytorchbot merge

pytorchmergebot · 2025-01-22T14:52:28Z

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

Update

b67cb5f

[ghstack-poisoned]

pytorch-bot bot added ciflow/inductor module: dynamo release notes: fx release notes category labels Nov 6, 2024

isuruf added a commit that referenced this pull request Nov 6, 2024

Dynamic shape guards in C++

aed0e0e

ghstack-source-id: ef9fab3 Pull Request resolved: #139899

facebook-github-bot added the fx label Nov 6, 2024

pytorchbot added the open source label Nov 6, 2024

Update

c4075a0

[ghstack-poisoned]

isuruf added a commit that referenced this pull request Nov 6, 2024

Dynamic shape guards in C++

49671d2

ghstack-source-id: 454ac7a Pull Request resolved: #139899

Update

7759bb1

[ghstack-poisoned]

isuruf added a commit that referenced this pull request Nov 6, 2024

Dynamic shape guards in C++

1e08ea5

ghstack-source-id: 0708511 Pull Request resolved: #139899

Update

40efb46

[ghstack-poisoned]

isuruf added a commit that referenced this pull request Nov 7, 2024

Dynamic shape guards in C++

50878f4

ghstack-source-id: fb14b80 Pull Request resolved: #139899

Update

3a9cbd4

[ghstack-poisoned]

isuruf added a commit that referenced this pull request Nov 7, 2024

Dynamic shape guards in C++

c43426c

ghstack-source-id: 3f5bcbb Pull Request resolved: #139899

ezyang reviewed Nov 7, 2024

View reviewed changes

ezyang requested review from anijain2305, bobrenjc93 and laithsakka November 7, 2024 16:58

ezyang reviewed Nov 7, 2024

View reviewed changes

anijain2305 reviewed Nov 9, 2024

View reviewed changes

torch/csrc/dynamo/guards.cpp Show resolved Hide resolved

anijain2305 reviewed Nov 9, 2024

View reviewed changes

torch/csrc/dynamo/guards.cpp Show resolved Hide resolved

anijain2305 requested a review from jansel November 9, 2024 22:55

Update

d1187a8

[ghstack-poisoned]

isuruf mentioned this pull request Nov 13, 2024

Move Sympy printers to torch/utils/_sympy/printers.py #140597

Closed

pytorchmergebot removed the merging label Jan 22, 2025

Update

60032c9

[ghstack-poisoned]

pytorch-bot bot temporarily deployed to upload-benchmark-results January 22, 2025 06:26 Inactive

Update

0ab1d58

[ghstack-poisoned]

pytorch-bot bot temporarily deployed to upload-benchmark-results January 22, 2025 09:08 Inactive

Update

f7e4bb0

[ghstack-poisoned]

pytorch-bot bot temporarily deployed to upload-benchmark-results January 22, 2025 11:05 Inactive

pytorchmergebot added the merging label Jan 22, 2025

pytorchmergebot added the Merged label Jan 22, 2025

pytorchmergebot closed this in 0efa843 Jan 22, 2025

pytorchmergebot removed the merging label Jan 22, 2025

anijain2305 mentioned this pull request Jan 29, 2025

"AssertionError: Guard check failed" on PyTorch nightlies after 2025-01-22 when running torchao tests #145733

Closed

isuruf mentioned this pull request Feb 14, 2025

dict_tag optimization leads to wrong results with relational guards #147212

Open

github-actions bot deleted the gh/isuruf/103/head branch February 22, 2025 02:03

Dynamic shape guards in C++ #139899

Dynamic shape guards in C++ #139899

Uh oh!

Conversation

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

Uh oh!

Uh oh!

pytorch-bot bot commented Nov 6, 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/139899

✅ No Failures

Uh oh!

isuruf commented Nov 6, 2024

Uh oh!

ezyang Nov 7, 2024

Choose a reason for hiding this comment

Uh oh!

ezyang Nov 7, 2024

Choose a reason for hiding this comment

Uh oh!

anijain2305 Nov 9, 2024

Choose a reason for hiding this comment

Uh oh!

anijain2305 Nov 20, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

anijain2305 commented Nov 9, 2024

Uh oh!

anijain2305 commented Nov 9, 2024

Uh oh!

pytorchmergebot commented Jan 22, 2025

Merge failed

Uh oh!

isuruf commented Jan 22, 2025

Uh oh!

pytorchmergebot commented Jan 22, 2025

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

isuruf commented Nov 6, 2024 •

edited

Loading

pytorch-bot bot commented Nov 6, 2024 •

edited

Loading