Make int unspecialization actually work by ezyang · Pull Request #95621 · pytorch/pytorch

ezyang · 2023-02-27T16:55:55Z

Stack from ghstack (oldest at bottom):

OK, so this PR used to be about reducing the number of constants we specialize on, but it turns out that unspecialization was ~essentially never used (because we still constant specialized way too aggressively) and I ended up having to fix a bunch of issues to actually get tests to pass. So this PR is now "make int unspecialization actually work". As part of this, I have to turn off unspecialization by default, as there are still latent bugs in inductor.

The general strategy is that an unspecialized int is represented as a SymInt. Representing it as a 0d tensor (which is what the code used to do) is untenable: (1) we often need unspecialized ints to participate in size computations, but we have no way of propagating sympy expressions through tensor compute, and (2) a lot of APIs work when passed SymInt, but not when passed a Tensor. However, I continue to represent Numpy scalars as Tensors, as they are rarely used for size computation and they have an explicit dtype, so they are more accurately modeled as 0d tensors.

I folded in the changes from rationalize specialize_int_float #95099 as I cannot represent unspecialized ints as SymInts without also turning on dynamic shapes. This also eliminates the necessity for test_unspec.py, as toggling specialization without dynamic shapes doesn't do anything. As dynamic shapes defaults to unspecializing, I just deleted this entirely; for the specialization case, I rely on regular static shape tests to catch it. (Hypothetically, we could also rerun all the tests with dynamic shapes, but WITH int/float specialization, but this seems... not that useful? I mean, I guess export wants it, but I'd kind of like our Source heuristic to improve enough that export doesn't have to toggle this either.)
Only 0/1 integers get specialized by default now
A hodgepodge of fixes. I'll comment on the PR about them.

Fixes #95469

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

cc @soumith @voznesenskym @yanboliang @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @desertfire

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

pytorch-bot · 2023-02-27T16:55:58Z

🔗 Helpful Links

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

📄 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 1142881:
💚 Looks good so far! There are no failures yet. 💚

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

Fixes #95469 Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

Fixes #95469 Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: c685677 Pull Request resolved: #95621

Fixes #95469 Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

Specifically: https://github.com/pytorch/pytorch/pull/95621/files/063e44147152f4dd7e51852cf8c679692bd9fd53#r1120306196 #95621 (comment) Signed-off-by: Edward Z. Yang <ezyang@meta.com> [ghstack-poisoned]

Specifically: https://github.com/pytorch/pytorch/pull/95621/files/063e44147152f4dd7e51852cf8c679692bd9fd53#r1120306196 #95621 (comment) Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: d094370 Pull Request resolved: #96043

Following CR at #95621 (comment) In fact, the block deleted is dead, because a MUST be a TensorVariable, and so it can never be a SymNodeVariable. Signed-off-by: Edward Z. Yang <ezyang@meta.com> [ghstack-poisoned]

Following CR at #95621 (comment) In fact, the block deleted is dead, because a MUST be a TensorVariable, and so it can never be a SymNodeVariable. Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: dce4fcd Pull Request resolved: #96044

OK, so this PR used to be about reducing the number of constants we specialize on, but it turns out that unspecialization was ~essentially never used (because we still constant specialized way too aggressively) and I ended up having to fix a bunch of issues to actually get tests to pass. So this PR is now "make int unspecialization actually work". As part of this, I have to turn off unspecialization by default, as there are still latent bugs in inductor. The general strategy is that an unspecialized int is represented as a SymInt. Representing it as a 0d tensor (which is what the code used to do) is untenable: (1) we often need unspecialized ints to participate in size computations, but we have no way of propagating sympy expressions through tensor compute, and (2) a lot of APIs work when passed SymInt, but not when passed a Tensor. However, I continue to represent Numpy scalars as Tensors, as they are rarely used for size computation and they have an explicit dtype, so they are more accurately modeled as 0d tensors. * I folded in the changes from pytorch/pytorch#95099 as I cannot represent unspecialized ints as SymInts without also turning on dynamic shapes. This also eliminates the necessity for test_unspec.py, as toggling specialization without dynamic shapes doesn't do anything. As dynamic shapes defaults to unspecializing, I just deleted this entirely; for the specialization case, I rely on regular static shape tests to catch it. (Hypothetically, we could also rerun all the tests with dynamic shapes, but WITH int/float specialization, but this seems... not that useful? I mean, I guess export wants it, but I'd kind of like our Source heuristic to improve enough that export doesn't have to toggle this either.) * Only 0/1 integers get specialized by default now * A hodgepodge of fixes. I'll comment on the PR about them. Fixes pytorch/pytorch#95469 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch/pytorch#95621 Approved by: https://github.com/jansel, https://github.com/Chillee

Specifically: https://github.com/pytorch/pytorch/pull/95621/files/063e44147152f4dd7e51852cf8c679692bd9fd53#r1120306196 #95621 (comment) Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc soumith voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

…eVariable" Following CR at #95621 (comment) In fact, the block deleted is dead, because a MUST be a TensorVariable, and so it can never be a SymNodeVariable. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc soumith voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

Following CR at #95621 (comment) In fact, the block deleted is dead, because a MUST be a TensorVariable, and so it can never be a SymNodeVariable. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc soumith voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

Specifically: https://github.com/pytorch/pytorch/pull/95621/files/063e44147152f4dd7e51852cf8c679692bd9fd53#r1120306196 #95621 (comment) Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: cd9a340 Pull Request resolved: #96043

Specifically: https://github.com/pytorch/pytorch/pull/95621/files/063e44147152f4dd7e51852cf8c679692bd9fd53#r1120306196 #95621 (comment) Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc soumith voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

Following CR at #95621 (comment) In fact, the block deleted is dead, because a MUST be a TensorVariable, and so it can never be a SymNodeVariable. Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: 6369791 Pull Request resolved: #96044

…eVariable" Following CR at #95621 (comment) In fact, the block deleted is dead, because a MUST be a TensorVariable, and so it can never be a SymNodeVariable. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc soumith voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

Following CR at #95621 (comment) In fact, the block deleted is dead, because a MUST be a TensorVariable, and so it can never be a SymNodeVariable. Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc soumith voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

Specifically: https://github.com/pytorch/pytorch/pull/95621/files/063e44147152f4dd7e51852cf8c679692bd9fd53#r1120306196 #95621 (comment) Signed-off-by: Edward Z. Yang <ezyangmeta.com> cc soumith voznesenskym penguinwu anijain2305 EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

Specifically: https://github.com/pytorch/pytorch/pull/95621/files/063e44147152f4dd7e51852cf8c679692bd9fd53#r1120306196 #95621 (comment) Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: bd22206 Pull Request resolved: #96043

Specifically: https://github.com/pytorch/pytorch/pull/95621/files/063e44147152f4dd7e51852cf8c679692bd9fd53#r1120306196 #95621 (comment) Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: #96043 Approved by: https://github.com/Chillee, https://github.com/albanD

Specifically: https://github.com/pytorch/pytorch/pull/95621/files/063e44147152f4dd7e51852cf8c679692bd9fd53#r1120306196 pytorch/pytorch#95621 (comment) Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch/pytorch#96043 Approved by: https://github.com/Chillee, https://github.com/albanD

OK, so this PR used to be about reducing the number of constants we specialize on, but it turns out that unspecialization was ~essentially never used (because we still constant specialized way too aggressively) and I ended up having to fix a bunch of issues to actually get tests to pass. So this PR is now "make int unspecialization actually work". As part of this, I have to turn off unspecialization by default, as there are still latent bugs in inductor. The general strategy is that an unspecialized int is represented as a SymInt. Representing it as a 0d tensor (which is what the code used to do) is untenable: (1) we often need unspecialized ints to participate in size computations, but we have no way of propagating sympy expressions through tensor compute, and (2) a lot of APIs work when passed SymInt, but not when passed a Tensor. However, I continue to represent Numpy scalars as Tensors, as they are rarely used for size computation and they have an explicit dtype, so they are more accurately modeled as 0d tensors. * I folded in the changes from pytorch#95099 as I cannot represent unspecialized ints as SymInts without also turning on dynamic shapes. This also eliminates the necessity for test_unspec.py, as toggling specialization without dynamic shapes doesn't do anything. As dynamic shapes defaults to unspecializing, I just deleted this entirely; for the specialization case, I rely on regular static shape tests to catch it. (Hypothetically, we could also rerun all the tests with dynamic shapes, but WITH int/float specialization, but this seems... not that useful? I mean, I guess export wants it, but I'd kind of like our Source heuristic to improve enough that export doesn't have to toggle this either.) * Only 0/1 integers get specialized by default now * A hodgepodge of fixes. I'll comment on the PR about them. Fixes pytorch#95469 Signed-off-by: Edward Z. Yang <ezyang@meta.com> Pull Request resolved: pytorch#95621 Approved by: https://github.com/jansel, https://github.com/Chillee

atalman · 2023-04-04T22:27:12Z

cc @guangy10 @ezyang

This issue is in the milestones : https://github.com/pytorch/pytorch/milestone/36?closed=1, if you want to see your fix included in this minor release. Please post it as a cherry-pick into the [v2.0.1] Release Tracker.

The deadline is April 14, 5PM PST.

Only issues that have ‘cherry-picks’ will be considered for the release.

Common FAQs:

Q1: Where can I find more information on the release process and terminology?

A: pytorch/RELEASE.md at master · pytorch/pytorch · GitHub

Q2: Am I guaranteed to be included in the cherry-pick if I do above?

A: No, it is not guaranteed, the Release Team will review all submissions against the listed criteria before making the final decision on what to include on 4/17.

Q3: When is 2.1 going to be released?

A: We do not have a formal date at this time but will update the community when we do. Our immediate focus is 2.0.1. Note that 1.12 was released on 6/28/22, 1.13 on 10/28/22 and 2.0 on 3/15/23.

Q4: I missed the 4/14 5PM PST deadline, is there any option to have an extension?

A: No, in order to meet our 4/28 goal, we must hold 4/14 as our deadline and will not accept any requests after the fact. We are over communicating the timelines and process with the community to avoid such issues.

Q5: Where should I double check to see if my issue is in the cherry pick tracker?

A: [v2.0.1] Release Tracker · Issue #97272 · pytorch/pytorch · GitHub

Q6: Where can I find the Release Compatibility Matrix for PyTorch?

A: pytorch/RELEASE.md at master · pytorch/pytorch · GitHub

Please contact OSS Releng team members if you have any questions/comments. Again we appreciate everyone’s time and commitment to the community, PyTorch and 2.0 and 2.01 releases!

Please refer to this post for more details: https://dev-discuss.pytorch.org/t/pytorch-release-2-0-1-important-information/1176

Reduce number of common integer constants

c0f4b90

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

ezyang mentioned this pull request Feb 27, 2023

Handle int/float arguments for cpp codegen in inductor #95533

Closed

github-actions bot added ciflow/inductor module: dynamo labels Feb 27, 2023

github-actions bot requested review from Chillee, SherlockNoMad, albanD, antoniojkim, bdhirsh, jbschlosser, miladm, voznesenskym and wconstab February 27, 2023 16:56

ezyang requested review from jansel and yanboliang February 27, 2023 16:59

jansel approved these changes Feb 27, 2023

View reviewed changes

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

Reduce number of common integer constants

255dce8

Fixes #95469 Signed-off-by: Edward Z. Yang <ezyangmeta.com> ghstack-source-id: c685677 Pull Request resolved: #95621

ezyang changed the title ~~Reduce number of common integer constants~~ Make int unspecialization actually work Feb 28, 2023

ezyang requested a review from jansel February 28, 2023 15:45

ezyang mentioned this pull request Mar 5, 2023

Extra CR comments from #95621 #96043

Closed

ezyang mentioned this pull request Mar 5, 2023

Simplify builtin handling of Tensor-SymNodeVariable #96044

Closed

ezyang mentioned this pull request Mar 5, 2023

OpenNMT doesn't work with dynamic torch.compile #93468

Closed

guangy10 mentioned this pull request Mar 15, 2023

Fail fast when dynamo attempts to add unspecialized int/float as additional graph inputs #96786

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make int unspecialization actually work#95621

Make int unspecialization actually work#95621
ezyang wants to merge 20 commits intogh/ezyang/1855/basefrom
gh/ezyang/1855/head

ezyang commented Feb 27, 2023 •

edited

Loading

Uh oh!

pytorch-bot bot commented Feb 27, 2023 •

edited

Loading

Uh oh!

atalman commented Apr 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

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

Uh oh!

Uh oh!

pytorch-bot bot commented Feb 27, 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/95621

✅ No Failures

Uh oh!

atalman commented Apr 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ezyang commented Feb 27, 2023 •

edited

Loading

pytorch-bot bot commented Feb 27, 2023 •

edited

Loading