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: 50bb181cf53418b172ff45389036d2cd55086880 Pull Request resolved: #127693

torch/fx/experimental/symbolic_shapes.py

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

[ghstack-poisoned]

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

[ghstack-poisoned]

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

[ghstack-poisoned]

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

[ghstack-poisoned]

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

[ghstack-poisoned]

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

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

torch/_export/passes/add_runtime_assertions_for_constraints_pass.py

        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.

Summary: 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 pytorch/pytorch#127396 Signed-off-by: Edward Z. Yang <ezyang@meta.com> X-link: pytorch/pytorch#127693 Approved by: https://github.com/lezcano ghstack dependencies: #126905 bypass-github-export-checks pushed new commits to forward fix error Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/9cab5987bdeb66df8efbc581b3469bfe300e168c Differential Revision: D58394154 Pulled By: clee2000

clee2000 · 2024-06-11T23:34:31Z

@pytorchbot revert -m "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" -c ghfirst

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

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: 50bb181cf53418b172ff45389036d2cd55086880 Pull Request resolved: #127693

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

lezcano reviewed Jun 3, 2024

View reviewed changes

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

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

Introduce int_oo

50514ce

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

Update

0de6920

[ghstack-poisoned]

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

Introduce int_oo

dda9a82

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

Update

c4991b1

[ghstack-poisoned]

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

Introduce int_oo

951ae1f

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

Update

2c2f463

[ghstack-poisoned]

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

Introduce int_oo

50bd608

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

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: ef3063b7f97002dde7e1effbb91961d4997d193d 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 added a commit that referenced this pull request Jun 4, 2024

Introduce int_oo

0152d9b

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce int_oo #127693

Introduce int_oo #127693

ezyang commented Jun 1, 2024 •

edited

pytorch-bot bot commented Jun 1, 2024 •

edited

ezyang Jun 4, 2024

avikchaudhuri Jun 6, 2024

clee2000 commented Jun 11, 2024

pytorchmergebot commented Jun 11, 2024

pytorchmergebot commented Jun 11, 2024

ezyang commented Jun 13, 2024

pytorchmergebot commented Jun 13, 2024

Introduce int_oo #127693

Introduce int_oo #127693

Conversation

ezyang commented Jun 1, 2024 • edited

pytorch-bot bot commented Jun 1, 2024 • edited

🔗 Helpful Links

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

✅ You can merge normally! (6 Unrelated Failures)

ezyang Jun 4, 2024

Choose a reason for hiding this comment

avikchaudhuri Jun 6, 2024

Choose a reason for hiding this comment

clee2000 commented Jun 11, 2024

pytorchmergebot commented Jun 11, 2024

pytorchmergebot commented Jun 11, 2024

ezyang commented Jun 13, 2024

pytorchmergebot commented Jun 13, 2024

Merge started

ezyang commented Jun 1, 2024 •

edited

pytorch-bot bot commented Jun 1, 2024 •

edited