Extra CR comments from #95621 by ezyang · Pull Request #96043 · pytorch/pytorch

ezyang · 2023-03-05T00:38:42Z

Stack from ghstack (oldest at bottom):

-> Extra CR comments from #95621 #96043

Specifically:
https://github.com/pytorch/pytorch/pull/95621/files/063e44147152f4dd7e51852cf8c679692bd9fd53#r1120306196
#95621 (comment)

Signed-off-by: Edward Z. Yang [email protected]

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

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

pytorch-bot · 2023-03-05T00:38:44Z

🔗 Helpful Links

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

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

❌ 2 Failures

As of commit 6f6d308:

NEW FAILURES - The following jobs have failed:

cuda11.8-py3.10-gcc7-sm86 / test (inductor_timm_dynamic, 2, 2, linux.g5.4xlarge.nvidia.gpu) (gh)

BROKEN TRUNK - The following jobs failed but were present on the merge base 457396f:

👉 Rebase onto the `viable/strict` branch to avoid these failures

cuda11.8-py3.10-gcc7-sm86 / test (inductor, 1, 1, linux.g5.4xlarge.nvidia.gpu) (gh)

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

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

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: cd9a340 Pull Request resolved: #96043

ezyang · 2023-03-08T19:27:44Z

@pytorchbot merge

pytorchmergebot · 2023-03-08T19:30:07Z

Merge failed

Reason: This PR needs a 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.

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

albanD · 2023-03-08T20:04:37Z

torch/_dynamo/utils.py

-                ref = ref.item()
-            if is_numpy_int_type(res):
-                res = res.item()
+        if relax_numpy_equality and not (


Why the not here? You actually want to change what happens?

The logic you see here in the diff is not what the logic originally was. The ORIGINAL logic was

if relax_numpy_equality: ref = ref.item()

But if you passed in both ref and res and numpy things, you would error, because ref would get converted to int, but res would not. I sort of hacked this up, but the hack up was wrong.

With the new logic, I more faithfully represent the bias the old implementation had: only ref gets converted from numpy to int, but ONLY if res is not a numpy. I don't allow ref to NOT be numpy and res to be numpy, which seems reasonable to me, because we shouldn't hallucinate numpy integers in the compiled code when the original didn't have any.

Sounds good! Thanks for the explanation.

ezyang · 2023-03-08T22:30:16Z

@pytorchbot merge

pytorchmergebot · 2023-03-08T22:32:09Z

Merge failed

Reason: This PR needs a 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.

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

ezyang · 2023-03-09T22:08:21Z

@pytorchbot merge -r master

pytorchmergebot · 2023-03-09T22:10:04Z

@pytorchbot successfully started a rebase job. Check the current status here

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]

pytorchmergebot · 2023-03-09T22:10:22Z

Successfully rebased gh/ezyang/1876/orig onto refs/remotes/origin/master, please pull locally before adding more changes (for example, via ghstack checkout https://github.com/pytorch/pytorch/pull/96043)

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

pytorchmergebot · 2023-03-09T22:11:30Z

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

pytorchmergebot · 2023-03-10T00:58:51Z

Merge failed

Reason: 1 jobs have failed, first few of them are: inductor / cuda11.8-py3.10-gcc7-sm86 / test (inductor_timm_dynamic, 2, 2, linux.g5.4xlarge.nvidia.gpu)

Details for Dev Infra team

Raised by workflow job

ezyang · 2023-03-10T01:09:00Z

@pytorchbot merge -f "spurious failures"

pytorchmergebot · 2023-03-10T01:10:42Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

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

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

github-actions bot requested review from Chillee, SherlockNoMad, albanD, antoniojkim, bdhirsh, jbschlosser, miladm, voznesenskym and wconstab March 5, 2023 00:38

github-actions bot added ciflow/inductor module: dynamo labels Mar 5, 2023

ezyang mentioned this pull request Mar 5, 2023

Simplify builtin handling of Tensor-SymNodeVariable #96044

Closed

ezyang added the ciflow/trunk Trigger trunk jobs on your pull request label Mar 8, 2023

Chillee approved these changes Mar 8, 2023

View reviewed changes

ezyang closed this Mar 8, 2023

ezyang reopened this Mar 8, 2023

github-actions bot requested a review from Chillee March 8, 2023 19:28

albanD reviewed Mar 8, 2023

View reviewed changes

Chillee approved these changes Mar 8, 2023

View reviewed changes

albanD approved these changes Mar 8, 2023

View reviewed changes

ezyang added the topic: not user facing topic category label Mar 9, 2023

pytorchmergebot added the Merged label Mar 10, 2023

pytorchmergebot closed this in 384d3ec Mar 10, 2023

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

Conversation

ezyang commented Mar 5, 2023 • edited by pytorchmergebot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Mar 5, 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/96043

❌ 2 Failures

Uh oh!

ezyang commented Mar 8, 2023

Uh oh!

pytorchmergebot commented Mar 8, 2023

Merge failed

Uh oh!

albanD Mar 8, 2023

Choose a reason for hiding this comment

Uh oh!

ezyang Mar 8, 2023

Choose a reason for hiding this comment

Uh oh!

albanD Mar 8, 2023

Choose a reason for hiding this comment

Uh oh!

ezyang commented Mar 8, 2023

Uh oh!

pytorchmergebot commented Mar 8, 2023

Merge failed

Uh oh!

ezyang commented Mar 9, 2023

Uh oh!

pytorchmergebot commented Mar 9, 2023

Uh oh!

pytorchmergebot commented Mar 9, 2023

Uh oh!

pytorchmergebot commented Mar 9, 2023

Merge started

Uh oh!

pytorchmergebot commented Mar 10, 2023

Merge failed

Uh oh!

ezyang commented Mar 10, 2023

Uh oh!

pytorchmergebot commented Mar 10, 2023

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ezyang commented Mar 5, 2023 •

edited by pytorchmergebot

Loading

pytorch-bot bot commented Mar 5, 2023 •

edited

Loading