Introduce constrain_range; remove old expr_subs by ezyang · Pull Request #95063 · pytorch/pytorch

ezyang · 2023-02-17T14:25:00Z

Stack from ghstack (oldest at bottom):

-> Introduce constrain_range; remove old expr_subs #95063

This PR introduces a new constrain_range function which can be used to constrain the possible values a SymInt/SymFloat can take on. This knowledge can be then used to discharge potential guards (by running the range analysis, and then seeing if the guard must be true given the original range) without adding another guard.

The usage of ranges is very limited right now; ranges are only constrained when the user explicitly instructs the system so. However, we can also infer range constraints based on guards as well; this is left for future work.

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

Signed-off-by: Edward Z. Yang <ezyang@meta.com> [ghstack-poisoned]

pytorch-bot · 2023-02-17T14:25:03Z

🔗 Helpful Links

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

📄 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 5185e15:
💚 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: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]

This PR introduces a new `constrain_range` function which can be used to constrain the possible values a SymInt/SymFloat can take on. This knowledge can be then used to discharge potential guards (by running the range analysis, and then seeing if the guard must be true given the original range) without adding another guard. The usage of ranges is very limited right now; ranges are only constrained when the user explicitly instructs the system so. However, we can also infer range constraints based on guards as well; this is left for future work. Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]

Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: 403a7e1 Pull Request resolved: #95063

This PR introduces a new `constrain_range` function which can be used to constrain the possible values a SymInt/SymFloat can take on. This knowledge can be then used to discharge potential guards (by running the range analysis, and then seeing if the guard must be true given the original range) without adding another guard. The usage of ranges is very limited right now; ranges are only constrained when the user explicitly instructs the system so. However, we can also infer range constraints based on guards as well; this is left for future work. Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]

Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: 8e82782 Pull Request resolved: #95063

This PR introduces a new `constrain_range` function which can be used to constrain the possible values a SymInt/SymFloat can take on. This knowledge can be then used to discharge potential guards (by running the range analysis, and then seeing if the guard must be true given the original range) without adding another guard. The usage of ranges is very limited right now; ranges are only constrained when the user explicitly instructs the system so. However, we can also infer range constraints based on guards as well; this is left for future work. Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]

eellison

Nice. We could start producing a range for Symints like .nonzero() at some point.

torch/fx/experimental/symbolic_shapes.py

eellison · 2023-02-17T16:37:54Z

torch/utils/_sympy/interp.py

    if isinstance(expr, sympy.Integer):
        return analysis.constant(int(expr), torch.int64)
-    elif isinstance(expr, sympy.Float):
+    elif isinstance(expr, sympy.Number):


what caused this change ?

1/3 turns into a Rational which is not a float. There's something kind of fishy going on here... I am still feeling like I want to pass the sympy expressions directly to analysis.constant without casting it to float first.

eellison · 2023-02-17T16:43:00Z

torch/fx/experimental/symbolic_shapes.py

+        # are conservative: the int MUST fall in the range, but the
+        # range may contain ints which may not actually appear in
+        # practice
+        self.var_to_range: Dict["sympy.Symbol", ValueRanges] = {}


do we not have a similar range for SymFloats ? any reason to constrict ?

It just didn't seem necessary! Also, I view the range analysis on floats as a sort of necessary evil when users do naughty things like sqrt, and for the most part, I only really want to be reasoning about size variables.

eellison · 2023-02-17T17:18:29Z

torch/fx/experimental/symbolic_shapes.py

+    if not isinstance(a, SymInt):
+        assert min <= a <= max
+        return
+    if isinstance(a.node.expr, sympy.Integer):


any reason not to support Floats here ?

Same deal, not supporting constraints on floats for now

This PR introduces a new `constrain_range` function which can be used to constrain the possible values a SymInt/SymFloat can take on. This knowledge can be then used to discharge potential guards (by running the range analysis, and then seeing if the guard must be true given the original range) without adding another guard. The usage of ranges is very limited right now; ranges are only constrained when the user explicitly instructs the system so. However, we can also infer range constraints based on guards as well; this is left for future work. Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]

ezyang · 2023-02-20T21:31:08Z

oops this broke convit_base

jeanschmidt · 2023-02-21T10:41:54Z

@pytorchbot revert -m "Breaking internal builds, more details can be found: https://fburl.com/phabricator/fq5b6k8a" -c nosignal

pytorchmergebot · 2023-02-21T10:43:34Z

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

pytorchmergebot · 2023-02-21T10:43:44Z

@ezyang your PR has been successfully reverted.

This reverts commit 3711f7c. Reverted #95063 on behalf of https://github.com/jeanschmidt due to Breaking internal builds, more details can be found: https://fburl.com/phabricator/fq5b6k8a

This reverts commit 4e88547. [ghstack-poisoned]

…old expr_subs (#95063)"" This reverts commit 4e88547. [ghstack-poisoned]

…95063)"" This reverts commit 4e88547. [ghstack-poisoned]

…old expr_subs (#95063)"" This reverts commit 4e88547. [ghstack-poisoned]

…95063)"" This reverts commit 4e88547. [ghstack-poisoned]

…5209) This reverts commit 4e88547. Pull Request resolved: #95209 Approved by: https://github.com/albanD

…)" (#95209)" This reverts commit f7bf31f. Reverted #95209 on behalf of https://github.com/ezyang due to internal sympy is too old

…5209) This reverts commit 4e88547. Pull Request resolved: #95209 Approved by: https://github.com/albanD

This reverts commit 3711f7c. Reverted pytorch/pytorch#95063 on behalf of https://github.com/jeanschmidt due to Breaking internal builds, more details can be found: https://fburl.com/phabricator/fq5b6k8a

…rch#95063)" (pytorch#95209)" This reverts commit 3758559.

…bs (pytorch#95063)" (pytorch#95209)"" This reverts commit cf6e078.

…rch#95063)" (pytorch#95209)" This reverts commit f7bf31f.

…rch#95063)"" This reverts commit 4e88547.

This reverts commit 3711f7c.

This PR introduces a new `constrain_range` function which can be used to constrain the possible values a SymInt/SymFloat can take on. This knowledge can be then used to discharge potential guards (by running the range analysis, and then seeing if the guard must be true given the original range) without adding another guard. The usage of ranges is very limited right now; ranges are only constrained when the user explicitly instructs the system so. However, we can also infer range constraints based on guards as well; this is left for future work. Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#95063 Approved by: https://github.com/eellison

This reverts commit 3711f7c. Reverted pytorch#95063 on behalf of https://github.com/jeanschmidt due to Breaking internal builds, more details can be found: https://fburl.com/phabricator/fq5b6k8a

…" (pytorch#95209) This reverts commit 4e88547. Pull Request resolved: pytorch#95209 Approved by: https://github.com/albanD

…rch#95063)" (pytorch#95209)" This reverts commit f7bf31f. Reverted pytorch#95209 on behalf of https://github.com/ezyang due to internal sympy is too old

…" (pytorch#95209) This reverts commit 4e88547. Pull Request resolved: pytorch#95209 Approved by: https://github.com/albanD

Introduce constrain_range; remove old expr_subs

78eb068

Signed-off-by: Edward Z. Yang <ezyang@meta.com> [ghstack-poisoned]

ezyang mentioned this pull request Feb 17, 2023

Assert more invariants on ValueRanges #94906

Closed

pytorch-bot bot added the release notes: fx release notes category label Feb 17, 2023

This was referenced Feb 17, 2023

Add exhaustive testing to ValueRanges, fix bugs #94939

Closed

Add boolean/comparison operator support to ValueRanges #94944

Closed

Add torch.utils._sympy.interp #94985

Closed

github-actions bot requested review from Chillee, SherlockNoMad, albanD, antoniojkim, bdhirsh, jbschlosser, miladm, voznesenskym and wconstab February 17, 2023 14:25

Update on "Introduce constrain_range; remove old expr_subs"

69e1f86

Signed-off-by: Edward Z. Yang <ezyangmeta.com> [ghstack-poisoned]

ezyang requested review from eellison and nunoplopes February 17, 2023 14:25

ezyang added a commit that referenced this pull request Feb 17, 2023

Introduce constrain_range; remove old expr_subs

814ae86

Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: 403a7e1 Pull Request resolved: #95063

ezyang added a commit that referenced this pull request Feb 17, 2023

Introduce constrain_range; remove old expr_subs

bb9daca

Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: 8e82782 Pull Request resolved: #95063

albanD removed their request for review February 17, 2023 16:09

eellison approved these changes Feb 17, 2023

View reviewed changes

pytorchmergebot closed this in 3711f7c Feb 19, 2023

pytorchmergebot added the Reverted label Feb 21, 2023

ezyang added a commit that referenced this pull request Feb 21, 2023

Reland "Introduce constrain_range; remove old expr_subs (#95063)"

dd160a0

This reverts commit 4e88547. [ghstack-poisoned]

ezyang added a commit that referenced this pull request Feb 21, 2023

Update base for Update on "Reland "Introduce constrain_range; remove …

58c8dbb

…old expr_subs (#95063)"" This reverts commit 4e88547. [ghstack-poisoned]

ezyang added a commit that referenced this pull request Feb 21, 2023

Update on "Reland "Introduce constrain_range; remove old expr_subs (#…

00a3a83

…95063)"" This reverts commit 4e88547. [ghstack-poisoned]

ezyang added a commit that referenced this pull request Feb 21, 2023

Update base for Update on "Reland "Introduce constrain_range; remove …

d0592ae

…old expr_subs (#95063)"" This reverts commit 4e88547. [ghstack-poisoned]

ezyang added a commit that referenced this pull request Feb 21, 2023

Update on "Reland "Introduce constrain_range; remove old expr_subs (#…

ea452f2

…95063)"" This reverts commit 4e88547. [ghstack-poisoned]

pytorchmergebot pushed a commit that referenced this pull request Feb 21, 2023

Reland "Introduce constrain_range; remove old expr_subs (#95063)" (#9…

f7bf31f

…5209) This reverts commit 4e88547. Pull Request resolved: #95209 Approved by: https://github.com/albanD

pytorchmergebot pushed a commit that referenced this pull request Feb 22, 2023

Reland "Introduce constrain_range; remove old expr_subs (#95063)" (#9…

3758559

…5209) This reverts commit 4e88547. Pull Request resolved: #95209 Approved by: https://github.com/albanD

msaroufim mentioned this pull request Mar 3, 2023

Remove mention of dynamo.optimize() in docs #96002

Closed

pruthvistony added a commit to ROCm/pytorch that referenced this pull request May 2, 2023

Revert "Reland "Introduce constrain_range; remove old expr_subs (pyto…

67814f4

…rch#95063)" (pytorch#95209)" This reverts commit 3758559.

pruthvistony added a commit to ROCm/pytorch that referenced this pull request May 2, 2023

Revert "Revert "Reland "Introduce constrain_range; remove old expr_su…

7e4a6b8

…bs (pytorch#95063)" (pytorch#95209)"" This reverts commit cf6e078.

pruthvistony added a commit to ROCm/pytorch that referenced this pull request May 2, 2023

Revert "Reland "Introduce constrain_range; remove old expr_subs (pyto…

39b1607

…rch#95063)" (pytorch#95209)" This reverts commit f7bf31f.

pruthvistony added a commit to ROCm/pytorch that referenced this pull request May 2, 2023

Revert "Revert "Introduce constrain_range; remove old expr_subs (pyto…

018921d

…rch#95063)"" This reverts commit 4e88547.

pruthvistony added a commit to ROCm/pytorch that referenced this pull request May 2, 2023

Revert "Introduce constrain_range; remove old expr_subs (pytorch#95063)"

f160443

This reverts commit 3711f7c.

facebook-github-bot deleted the gh/ezyang/1826/head branch June 8, 2023 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce constrain_range; remove old expr_subs#95063

Introduce constrain_range; remove old expr_subs#95063
ezyang wants to merge 10 commits intogh/ezyang/1826/basefrom
gh/ezyang/1826/head

ezyang commented Feb 17, 2023 •

edited

Loading

Uh oh!

pytorch-bot bot commented Feb 17, 2023 •

edited

Loading

Uh oh!

eellison left a comment

Uh oh!

Uh oh!

eellison Feb 17, 2023

Uh oh!

ezyang Feb 17, 2023

Uh oh!

eellison Feb 17, 2023

Uh oh!

ezyang Feb 17, 2023

Uh oh!

eellison Feb 17, 2023

Uh oh!

ezyang Feb 17, 2023

Uh oh!

ezyang commented Feb 20, 2023

Uh oh!

jeanschmidt commented Feb 21, 2023

Uh oh!

pytorchmergebot commented Feb 21, 2023

Uh oh!

pytorchmergebot commented Feb 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ezyang commented Feb 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Feb 17, 2023 • 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/95063

✅ No Failures

Uh oh!

eellison left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eellison Feb 17, 2023

Choose a reason for hiding this comment

Uh oh!

ezyang Feb 17, 2023

Choose a reason for hiding this comment

Uh oh!

eellison Feb 17, 2023

Choose a reason for hiding this comment

Uh oh!

ezyang Feb 17, 2023

Choose a reason for hiding this comment

Uh oh!

eellison Feb 17, 2023

Choose a reason for hiding this comment

Uh oh!

ezyang Feb 17, 2023

Choose a reason for hiding this comment

Uh oh!

ezyang commented Feb 20, 2023

Uh oh!

jeanschmidt commented Feb 21, 2023

Uh oh!

pytorchmergebot commented Feb 21, 2023

Uh oh!

pytorchmergebot commented Feb 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ezyang commented Feb 17, 2023 •

edited

Loading

pytorch-bot bot commented Feb 17, 2023 •

edited

Loading