Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ZeroDivisionError when unfolding a zero-dimension tensor in compile mode #113259

Conversation

Copy link

linux-foundation-easycla bot commented Nov 8, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link

pytorch-bot bot commented Nov 8, 2023

🔗 Helpful Links

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

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 009cbea with merge base 6e73ae2 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@lamroger lamroger marked this pull request as ready for review November 8, 2023 16:33
@lamroger
Copy link
Contributor Author

lamroger commented Nov 8, 2023

@pytorchbot label "release notes: inductor"

x = torch.rand([1,0], dtype=torch.float32)

y = forward(x)
compiled_y = torch.compile(forward, mode='max-autotune',fullgraph=True)(x)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't need max-autotune. Also, it should go in test_torchinductor.py

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate the quick review! I put it under CommonTemplate but I'm having trouble running it locally.

I'm on a mac and it's looking like this line
if HAS_CPU and not torch.backends.mps.is_available():
is short circuiting the CommonTemplate tests from running.

If I comment out the and not torch.backends.mps.is_available() part
python test/inductor/test_torch inductor.py -k test_unfold_zero_dimension_tensor
passes!

torch/_inductor/lowering.py Outdated Show resolved Hide resolved
lamroger and others added 2 commits November 8, 2023 14:04
Co-authored-by: peterbell10 <peterbell10@live.co.uk>
x = torch.rand([1, 0], dtype=torch.float32)

y = forward(x)
compiled_y = torch.compile(forward, fullgraph=True)(x)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lamroger Just out of curiosity, I executed this line only inside python interpreter. It still throws out ZeroDivisionError. Do you have any clue why? but pytest passes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm maybe you can reload the python REPL or look at the stack trace to see which files are being executed and make sure the right ones are patched locally.

That worked for me when I was fiddling around

Copy link

@akdong91 akdong91 Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly. it is working now without an exception. Probably, the outdated module was still used.

@ezyang ezyang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Nov 9, 2023
@peterbell10
Copy link
Collaborator

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 9, 2023
@pytorchmergebot
Copy link
Collaborator

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

Skylion007 pushed a commit to Skylion007/pytorch that referenced this pull request Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: inductor open source release notes: inductor triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ZeroDivisionError in torch.compile with torch.unfold_copy and Zero-Dimension Tensor
6 participants