Introduce int_oo #127693

ezyang · 2024-06-01T20:44:13Z

Stack from ghstack (oldest at bottom):

-> Introduce int_oo #127693

In a previous life, we used sympy.oo to represent the lower/upper bounds of integer ranges. Later, we changed this to be sys.maxsize - 1 for a few reasons: (1) sometimes we do tests on a value being exactly sys.maxsize, and we wanted to avoid a data dependent guard in this case, (2) sympy.oo corresponds to floating point infinity, so you get incorrect types for value ranges with oo, and (3) you can do slightly better reasoning if you assume that input sizes fall within representable 64-bit integer range.

After working in the sys.maxsize regime for a bit, I've concluded that this was actually a bad idea. Specifically, the problem is that you end up with sys.maxsize in your upper bound, and then whenever you do any sort of size-increasing computation like size * 2, you end up with 2 * sys.maxsize, and you end up doing a ton of arbitrary precision int computation that is totally unnecessary. A symbolic bound is better.

But especially after #126905, we can't go back to using sympy.oo, because that advertises that it's not an integer, and now your ValueRanges is typed incorrectly. So what do we do? We define a new numeric constant int_oo, which is like sympy.oo but it advertises is_integer. test/test_sympy_utils.py describes some basic properties of the number, and torch/utils/_sympy/numbers.py has the actual implementation.

The rest of the changes of the PR are working out the implications of this change. I'll give more commentary as inline comments.

Fixes #127396

Signed-off-by: Edward Z. Yang ezyang@meta.com

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

[ghstack-poisoned]

pytorch-bot · 2024-06-01T20:44:16Z

🔗 Helpful Links

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

📄 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.

✅ You can merge normally! (6 Unrelated Failures)

As of commit 0b36b03 with merge base f681e36 ():

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

inductor / cuda12.1-py3.10-gcc9-sm86 / test (aot_inductor_torchbench, 1, 2, linux.g5.4xlarge.nvidia.gpu) (gh) (similar failure)
DALLE2_pytorch
inductor / cuda12.1-py3.10-gcc9-sm86 / test (dynamic_inductor_torchbench, 1, 2, linux.g5.4xlarge.nvidia.gpu) (gh) (similar failure)
DALLE2_pytorch
inductor / cuda12.1-py3.10-gcc9-sm86 / test (inductor_torchbench, 1, 2, linux.g5.4xlarge.nvidia.gpu) (gh) (similar failure)
DALLE2_pytorch
inductor-periodic / cuda12.1-py3.10-gcc9-sm86-periodic-dynamo-benchmarks / test (dynamic_aot_eager_torchbench, 1, 2, linux.g5.4xlarge.nvidia.gpu) (gh) (similar failure)
DALLE2_pytorch
inductor-periodic / cuda12.1-py3.10-gcc9-sm86-periodic-dynamo-benchmarks / test (dynamo_eager_torchbench, 1, 2, linux.g5.4xlarge.nvidia.gpu) (gh) (similar failure)
DALLE2_pytorch
pull / linux-jammy-py3.8-gcc11 / test (distributed, 2, 2, linux.2xlarge) (gh) (similar failure)
Process completed with exit code 1.

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

Signed-off-by: Edward Z. Yang <ezyang@meta.com> ghstack-source-id: 50bb181 Pull Request resolved: #127693

torch/fx/experimental/symbolic_shapes.py

[ghstack-poisoned]

Signed-off-by: Edward Z. Yang <ezyang@meta.com> ghstack-source-id: ef3063b Pull Request resolved: #127693

[ghstack-poisoned]

ezyang · 2024-06-04T11:52:32Z

torch/_export/passes/add_runtime_assertions_for_constraints_pass.py

+    if val in (sympy.oo, int_oo):
        return math.inf
-    if val == -sympy.oo:
+    if val in (-sympy.oo, -int_oo):


@angelayi @avikchaudhuri This is a bit suspicious, because math.inf is not an integer. But I guess that's what your code used to do? Another option is when you have bounds like this is to return None.

Yeah None would be better.

pytorchmergebot · 2024-06-11T23:36:00Z

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

This reverts commit 9cab598. Reverted #127693 on behalf of https://github.com/clee2000 due to sorry executorch CI is a bit weird regarding pins, I'll make a chat with mergen with the choices of what to do and how it'll affect executorch CI, reverting for now to prevent more divergences in the meantime ([comment](#127693 (comment)))

pytorchmergebot · 2024-06-11T23:36:11Z

@ezyang your PR has been successfully reverted.

Summary: This is needed for pytorch/pytorch#127693 . This code is written so it is compatible before and after this PR. Differential Revision: D58465158

Summary: Pull Request resolved: #3947 This is needed for pytorch/pytorch#127693 . This code is written so it is compatible before and after this PR. Differential Revision: D58465158

Summary: This is needed for pytorch/pytorch#127693 . This code is written so it is compatible before and after this PR. Reviewed By: mergennachin Differential Revision: D58465158

Summary: Pull Request resolved: #3947 This is needed for pytorch/pytorch#127693 . This code is written so it is compatible before and after this PR. Reviewed By: mergennachin Differential Revision: D58465158

Summary: Pull Request resolved: #3947 This is needed for pytorch/pytorch#127693 . This code is written so it is compatible before and after this PR. Reviewed By: mergennachin, clee2000 Differential Revision: D58465158 fbshipit-source-id: ca0f2a79eb07e78ff2887f78eb62ff38eeea3ede

ezyang · 2024-06-13T04:05:58Z

@pytorchbot merge

pytorchmergebot · 2024-06-13T04:07:48Z

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

merge at will After #125968 and #127693 landrace Pull Request resolved: #128587 Approved by: https://github.com/huydhn

In a previous life, we used sympy.oo to represent the lower/upper bounds of integer ranges. Later, we changed this to be sys.maxsize - 1 for a few reasons: (1) sometimes we do tests on a value being exactly sys.maxsize, and we wanted to avoid a data dependent guard in this case, (2) sympy.oo corresponds to floating point infinity, so you get incorrect types for value ranges with oo, and (3) you can do slightly better reasoning if you assume that input sizes fall within representable 64-bit integer range. After working in the sys.maxsize regime for a bit, I've concluded that this was actually a bad idea. Specifically, the problem is that you end up with sys.maxsize in your upper bound, and then whenever you do any sort of size-increasing computation like size * 2, you end up with 2 * sys.maxsize, and you end up doing a ton of arbitrary precision int computation that is totally unnecessary. A symbolic bound is better. But especially after pytorch#126905, we can't go back to using sympy.oo, because that advertises that it's not an integer, and now your ValueRanges is typed incorrectly. So what do we do? We define a new numeric constant `int_oo`, which is like `sympy.oo` but it advertises `is_integer`. **test/test_sympy_utils.py** describes some basic properties of the number, and **torch/utils/_sympy/numbers.py** has the actual implementation. The rest of the changes of the PR are working out the implications of this change. I'll give more commentary as inline comments. Fixes pytorch#127396 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#127693 Approved by: https://github.com/lezcano ghstack dependencies: pytorch#126905

This reverts commit 9cab598. Reverted pytorch#127693 on behalf of https://github.com/clee2000 due to sorry executorch CI is a bit weird regarding pins, I'll make a chat with mergen with the choices of what to do and how it'll affect executorch CI, reverting for now to prevent more divergences in the meantime ([comment](pytorch#127693 (comment)))

In a previous life, we used sympy.oo to represent the lower/upper bounds of integer ranges. Later, we changed this to be sys.maxsize - 1 for a few reasons: (1) sometimes we do tests on a value being exactly sys.maxsize, and we wanted to avoid a data dependent guard in this case, (2) sympy.oo corresponds to floating point infinity, so you get incorrect types for value ranges with oo, and (3) you can do slightly better reasoning if you assume that input sizes fall within representable 64-bit integer range. After working in the sys.maxsize regime for a bit, I've concluded that this was actually a bad idea. Specifically, the problem is that you end up with sys.maxsize in your upper bound, and then whenever you do any sort of size-increasing computation like size * 2, you end up with 2 * sys.maxsize, and you end up doing a ton of arbitrary precision int computation that is totally unnecessary. A symbolic bound is better. But especially after pytorch#126905, we can't go back to using sympy.oo, because that advertises that it's not an integer, and now your ValueRanges is typed incorrectly. So what do we do? We define a new numeric constant `int_oo`, which is like `sympy.oo` but it advertises `is_integer`. **test/test_sympy_utils.py** describes some basic properties of the number, and **torch/utils/_sympy/numbers.py** has the actual implementation. The rest of the changes of the PR are working out the implications of this change. I'll give more commentary as inline comments. Fixes pytorch#127396 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#127693 Approved by: https://github.com/lezcano ghstack dependencies: pytorch#126905

merge at will After pytorch#125968 and pytorch#127693 landrace Pull Request resolved: pytorch#128587 Approved by: https://github.com/huydhn

This reverts commit 9cab598. Reverted pytorch#127693 on behalf of https://github.com/clee2000 due to sorry executorch CI is a bit weird regarding pins, I'll make a chat with mergen with the choices of what to do and how it'll affect executorch CI, reverting for now to prevent more divergences in the meantime ([comment](pytorch#127693 (comment)))

In a previous life, we used sympy.oo to represent the lower/upper bounds of integer ranges. Later, we changed this to be sys.maxsize - 1 for a few reasons: (1) sometimes we do tests on a value being exactly sys.maxsize, and we wanted to avoid a data dependent guard in this case, (2) sympy.oo corresponds to floating point infinity, so you get incorrect types for value ranges with oo, and (3) you can do slightly better reasoning if you assume that input sizes fall within representable 64-bit integer range. After working in the sys.maxsize regime for a bit, I've concluded that this was actually a bad idea. Specifically, the problem is that you end up with sys.maxsize in your upper bound, and then whenever you do any sort of size-increasing computation like size * 2, you end up with 2 * sys.maxsize, and you end up doing a ton of arbitrary precision int computation that is totally unnecessary. A symbolic bound is better. But especially after pytorch#126905, we can't go back to using sympy.oo, because that advertises that it's not an integer, and now your ValueRanges is typed incorrectly. So what do we do? We define a new numeric constant `int_oo`, which is like `sympy.oo` but it advertises `is_integer`. **test/test_sympy_utils.py** describes some basic properties of the number, and **torch/utils/_sympy/numbers.py** has the actual implementation. The rest of the changes of the PR are working out the implications of this change. I'll give more commentary as inline comments. Fixes pytorch#127396 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#127693 Approved by: https://github.com/lezcano ghstack dependencies: pytorch#126905

merge at will After pytorch#125968 and pytorch#127693 landrace Pull Request resolved: pytorch#128587 Approved by: https://github.com/huydhn

bhack · 2024-10-15T20:46:51Z

@ezyang With last nightly I was going to get spammed by

site-packages/torch/utils/_sympy/interp.py:159] [0/8] failed while executing int_truediv([MetaProxy(scalar_tensor_default_7), MetaProxy(scalar_tensor_default_7)])

Update

ec5af1a

[ghstack-poisoned]

ezyang mentioned this pull request May 31, 2024

Complete revamp of float/promotion sympy handling #126905

Closed

pytorch-bot bot added ciflow/inductor module: cpu CPU specific problem (e.g., perf, algorithm) release notes: fx release notes category labels Jun 1, 2024

ezyang added a commit that referenced this pull request Jun 1, 2024

Introduce int_oo

a857c4b

Signed-off-by: Edward Z. Yang <ezyang@meta.com> ghstack-source-id: 50bb181 Pull Request resolved: #127693

github-actions bot requested review from SherlockNoMad, albanD, antoniojkim, bdhirsh and miladm June 1, 2024 20:44

lezcano reviewed Jun 3, 2024

View reviewed changes

torch/fx/experimental/symbolic_shapes.py Outdated Show resolved Hide resolved

Update

0de6920

[ghstack-poisoned]

Update

c4991b1

[ghstack-poisoned]

Update

2c2f463

[ghstack-poisoned]

Update

8ffb4c6

[ghstack-poisoned]

ezyang added a commit that referenced this pull request Jun 3, 2024

Introduce int_oo

8ca6dd8

Signed-off-by: Edward Z. Yang <ezyang@meta.com> ghstack-source-id: ef3063b Pull Request resolved: #127693

Update

89c2124

[ghstack-poisoned]

ezyang requested review from avikchaudhuri, gmagogsfm, tugsbayasgalan and zhxchen17 as code owners June 4, 2024 11:39

pytorch-bot bot added the module: dynamo label Jun 4, 2024

ezyang commented Jun 4, 2024

View reviewed changes

pytorchmergebot added the Reverted label Jun 11, 2024

pytorchmergebot reopened this Jun 11, 2024

huydhn mentioned this pull request Jun 12, 2024

Apply isFailureFromPrevMergeCommit logic to all similar failures pytorch/test-infra#5323

Merged

ezyang mentioned this pull request Jun 12, 2024

FC preparation for int_oo in PyTorch pytorch/executorch#3947

Closed

pytorchmergebot added the merging label Jun 13, 2024

pytorchmergebot closed this in 2229884 Jun 13, 2024

pytorchmergebot removed the merging label Jun 13, 2024

clee2000 mentioned this pull request Jun 13, 2024

Forward fix lint #128587

Closed

pytorchmergebot pushed a commit that referenced this pull request Jun 13, 2024

Forward fix lint (#128587)

518c9e6

merge at will After #125968 and #127693 landrace Pull Request resolved: #128587 Approved by: https://github.com/huydhn

TharinduRusira pushed a commit to TharinduRusira/pytorch that referenced this pull request Jun 14, 2024

Forward fix lint (pytorch#128587)

b7c7d08

merge at will After pytorch#125968 and pytorch#127693 landrace Pull Request resolved: pytorch#128587 Approved by: https://github.com/huydhn

ignaciobartol pushed a commit to ignaciobartol/pytorch that referenced this pull request Jun 14, 2024

Forward fix lint (pytorch#128587)

9ae392e

merge at will After pytorch#125968 and pytorch#127693 landrace Pull Request resolved: pytorch#128587 Approved by: https://github.com/huydhn

ezyang mentioned this pull request Jun 24, 2024

[inductor][cpu]transformers models static/dynamic quant performance/accuracy crash in 2024-06-17 nightly release #128933

Closed

github-actions bot deleted the gh/ezyang/2796/head branch July 14, 2024 02:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Introduce int_oo #127693

Introduce int_oo #127693

Uh oh!

ezyang commented Jun 1, 2024 •

edited

Loading

Uh oh!

pytorch-bot bot commented Jun 1, 2024 •

edited

Loading

Uh oh!

Uh oh!

ezyang Jun 4, 2024

Uh oh!

avikchaudhuri Jun 6, 2024

Uh oh!

pytorchmergebot commented Jun 11, 2024

Uh oh!

pytorchmergebot commented Jun 11, 2024

Uh oh!

ezyang commented Jun 13, 2024

Uh oh!

pytorchmergebot commented Jun 13, 2024

Uh oh!

bhack commented Oct 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Introduce int_oo #127693

Introduce int_oo #127693

Uh oh!

Conversation

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

Uh oh!

Uh oh!

pytorch-bot bot commented Jun 1, 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/127693

✅ You can merge normally! (6 Unrelated Failures)

Uh oh!

Uh oh!

ezyang Jun 4, 2024

Choose a reason for hiding this comment

Uh oh!

avikchaudhuri Jun 6, 2024

Choose a reason for hiding this comment

Uh oh!

pytorchmergebot commented Jun 11, 2024

Uh oh!

pytorchmergebot commented Jun 11, 2024

Uh oh!

ezyang commented Jun 13, 2024

Uh oh!

pytorchmergebot commented Jun 13, 2024

Merge started

Uh oh!

bhack commented Oct 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ezyang commented Jun 1, 2024 •

edited

Loading

pytorch-bot bot commented Jun 1, 2024 •

edited

Loading