Fix inductor `sub` with symbolic integers. #108518

ysiraichi · 2023-09-04T13:35:09Z

Stack from ghstack (oldest at bottom):

-> Fix inductor sub with symbolic integers. #108518

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @ngimel @yf225 @chenyang78 @kadeng @muchulee8 @aakhundov

Fix: #108159 [ghstack-poisoned]

pytorch-bot · 2023-09-04T13:35:12Z

🔗 Helpful Links

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

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

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

Fix: #108159 ghstack-source-id: 6d86297a820863f9163c84f548f9b38d8e867e1f Pull Request resolved: #108518

Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Fix: #108159 ghstack-source-id: 8f0450feb7459eaafb086d5ab8f6fcba478606af Pull Request resolved: #108518

…ers." Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Fix: #108159 ghstack-source-id: 73fc8bf6c885044b3e101040402d5ba683a442d7 Pull Request resolved: #108518

Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Fix: #108159 ghstack-source-id: 5429ccab88bdc110381c7f040a98b39ddd9bb21d Pull Request resolved: #108518

peterbell10 · 2023-09-04T17:00:59Z

torch/_inductor/lowering.py

@@ -377,12 +377,23 @@ def inner(*inputs: List[TensorBox], alpha=None):
                inputs[-1] = mul(inputs[-1], alpha)
        else:
            assert alpha is None
+
+        # Get the first TensorBox/ExpandView input as the reference tensor for
+        # sizes, data types, and device.


promote_constants handles this for ir.Constant by wrapping it in an ExpandView:

pytorch/torch/_inductor/lowering.py

Lines 351 to 353 in 51c2e22

ExpandView.create(

ir.Constant(x, ex.get_dtype(), ex.get_device()), list(ex.get_size())

)

I think we could just do the same for IndexingConstant below:

pytorch/torch/_inductor/lowering.py

Line 356 in 51c2e22

out.append(IndexingConstant(x, ex.get_dtype(), ex.get_device()))

Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Fix: #108159 ghstack-source-id: 0a7d24b4bdb0c9280a8ce31d38138b7817e0cfdf Pull Request resolved: #108518

Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Fix: #108159 ghstack-source-id: 8cdbc02d862f247e9cbca293b5a5978569a18580 Pull Request resolved: #108518

peterbell10 · 2023-09-06T03:26:03Z

torch/_inductor/lowering.py

            out.append(
                ExpandView.create(
                    ir.Constant(x, ex.get_dtype(), ex.get_device()), list(ex.get_size())
                )
            )
-        elif isinstance(x, sympy.Expr):
-            out.append(IndexingConstant(x, ex.get_dtype(), ex.get_device()))


You still need to create an IndexingConstant node here, just wrap it in ExpandView.create.

Oh, I see. Thanks for the help, @peterbell10.

Trying to re-land #108518. [ghstack-poisoned]

Trying to re-land #108518. ghstack-source-id: c8cfe27f2a7291cf73d5d77dc9d2cc8498656ef4 Pull Request resolved: #108647

Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

Fix: #108159 ghstack-source-id: 8c7776bc84778e2f6543d1bcffd73c584b314738 Pull Request resolved: #108518

ysiraichi · 2023-09-06T16:28:37Z

@pytorchbot merge

pytorchmergebot · 2023-09-06T16:30:15Z

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team

Raised by workflow job

ysiraichi · 2023-09-06T20:02:18Z

@pytorchbot merge

peterbell10 · 2023-09-06T20:57:34Z

@pytorchbot merge

pytorchmergebot · 2023-09-06T21:01:27Z

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

Fix inductor sub with symbolic integers.

d42211c

Fix: #108159 [ghstack-poisoned]

ysiraichi added a commit that referenced this pull request Sep 4, 2023

Fix inductor sub with symbolic integers.

5ca980e

Fix: #108159 ghstack-source-id: 6d86297a820863f9163c84f548f9b38d8e867e1f Pull Request resolved: #108518

github-actions bot added module: inductor ciflow/inductor labels Sep 4, 2023

pytorchbot added the open source label Sep 4, 2023

Add comment. on "Fix inductor sub with symbolic integers."

cbd570d

Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

ysiraichi added a commit that referenced this pull request Sep 4, 2023

Fix inductor sub with symbolic integers.

d2d48e5

Fix: #108159 ghstack-source-id: 8f0450feb7459eaafb086d5ab8f6fcba478606af Pull Request resolved: #108518

ysiraichi added a commit that referenced this pull request Sep 4, 2023

Fix inductor sub with symbolic integers.

6a4e31d

Fix: #108159 ghstack-source-id: 73fc8bf6c885044b3e101040402d5ba683a442d7 Pull Request resolved: #108518

Fix lint issues. on "Fix inductor sub with symbolic integers."

3b21c33

Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

ysiraichi added a commit that referenced this pull request Sep 4, 2023

Fix inductor sub with symbolic integers.

e3fc736

Fix: #108159 ghstack-source-id: 5429ccab88bdc110381c7f040a98b39ddd9bb21d Pull Request resolved: #108518

peterbell10 reviewed Sep 4, 2023

View reviewed changes

Update on "Fix inductor sub with symbolic integers."

f16d945

Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

ysiraichi added a commit that referenced this pull request Sep 4, 2023

Fix inductor sub with symbolic integers.

e2a8984

Fix: #108159 ghstack-source-id: 0a7d24b4bdb0c9280a8ce31d38138b7817e0cfdf Pull Request resolved: #108518

ysiraichi marked this pull request as draft September 5, 2023 20:42

Update on "Fix inductor sub with symbolic integers."

9ed4fec

Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

ysiraichi added a commit that referenced this pull request Sep 5, 2023

Fix inductor sub with symbolic integers.

fb7fa17

Fix: #108159 ghstack-source-id: 8cdbc02d862f247e9cbca293b5a5978569a18580 Pull Request resolved: #108518

peterbell10 reviewed Sep 6, 2023

View reviewed changes

ysiraichi mentioned this pull request Sep 6, 2023

Re-land: Break graph on manual_seed. #108647

Closed

ysiraichi added a commit that referenced this pull request Sep 6, 2023

Re-land: Break graph on manual_seed.

7c5a6bc

Trying to re-land #108518. [ghstack-poisoned]

ysiraichi added a commit that referenced this pull request Sep 6, 2023

Re-land: Break graph on manual_seed.

ba988ac

Trying to re-land #108518. ghstack-source-id: c8cfe27f2a7291cf73d5d77dc9d2cc8498656ef4 Pull Request resolved: #108647

Update on "Fix inductor sub with symbolic integers."

0dbfb94

Fix: #108159 cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy ngimel yf225 chenyang78 kadeng muchulee8 aakhundov [ghstack-poisoned]

ysiraichi added a commit that referenced this pull request Sep 6, 2023

Fix inductor sub with symbolic integers.

5a49549

Fix: #108159 ghstack-source-id: 8c7776bc84778e2f6543d1bcffd73c584b314738 Pull Request resolved: #108518

peterbell10 approved these changes Sep 6, 2023

View reviewed changes

ysiraichi marked this pull request as ready for review September 6, 2023 16:28

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 6, 2023

pytorchmergebot added the merging label Sep 6, 2023

pytorchmergebot removed the merging label Sep 6, 2023

ysiraichi added the release notes: inductor label Sep 6, 2023

pytorchmergebot added the merging label Sep 6, 2023

pytorchmergebot added Merged and removed merging labels Sep 6, 2023

pytorchmergebot closed this in 089950b Sep 6, 2023

facebook-github-bot deleted the gh/ysiraichi/83/head branch September 10, 2023 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix inductor `sub` with symbolic integers. #108518

Fix inductor `sub` with symbolic integers. #108518

ysiraichi commented Sep 4, 2023 •

edited by pytorch-bot bot

pytorch-bot bot commented Sep 4, 2023 •

edited

peterbell10 Sep 4, 2023

peterbell10 Sep 6, 2023

ysiraichi Sep 6, 2023

ysiraichi commented Sep 6, 2023

pytorchmergebot commented Sep 6, 2023

ysiraichi commented Sep 6, 2023

peterbell10 commented Sep 6, 2023

pytorchmergebot commented Sep 6, 2023

	ExpandView.create(
	ir.Constant(x, ex.get_dtype(), ex.get_device()), list(ex.get_size())
	)

Fix inductor sub with symbolic integers. #108518

Fix inductor sub with symbolic integers. #108518

Conversation

ysiraichi commented Sep 4, 2023 • edited by pytorch-bot bot

pytorch-bot bot commented Sep 4, 2023 • edited

🔗 Helpful Links

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

✅ No Failures

peterbell10 Sep 4, 2023

Choose a reason for hiding this comment

peterbell10 Sep 6, 2023

Choose a reason for hiding this comment

ysiraichi Sep 6, 2023

Choose a reason for hiding this comment

ysiraichi commented Sep 6, 2023

pytorchmergebot commented Sep 6, 2023

Merge failed

ysiraichi commented Sep 6, 2023

peterbell10 commented Sep 6, 2023

pytorchmergebot commented Sep 6, 2023

Merge started

Fix inductor `sub` with symbolic integers. #108518

Fix inductor `sub` with symbolic integers. #108518

ysiraichi commented Sep 4, 2023 •

edited by pytorch-bot bot

pytorch-bot bot commented Sep 4, 2023 •

edited