-
Notifications
You must be signed in to change notification settings - Fork 108
tests: Re-enable zero-size shapes in elementwise unary opinfo #2698
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
base: main
Are you sure you want to change the base?
Conversation
4453db6 to
f98bf42
Compare
IvanYashchuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great that tests are passing! Supporting 0-sized tensors is a niche but an important usability feature, better not to regress on this accidentally.
|
@KaelanDt, I don't have the Lightning account to view CI logs. Could you please review the changes and CI failures if they are related? |
riccardofelluga
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for enabling this again!
There seems to be some related failures in the logs:
thunder/tests/distributed/test_dtensor.py::DTensorTest::test_dtensor_opinfo_neg_nvfuser
thunder/tests/distributed/test_dtensor.py::DTensorTest::test_dtensor_opinfo_silu_nvfuser
thunder/tests/distributed/test_dtensor.py::DTensorTest::test_dtensor_opinfo_reciprocal_nvfuserWould you be able to investigate? If that is not possible, could you add a skip to those nvFuser tests and open a new issue?
@riccardofelluga Ok, that explains better what is going on, thanks for sharing! So it seems 0-shape tensors are not really working well on nvFuser backend, but as for other backends it is fine. Unfortunately, I'm not ready to fix nvFuser backend specific stuff (no HW available). I was actually observing that everything is good on CPU. Does it make sense for you to enable them globally, but skip again only for nvFuser? In this case, we will increase code coverage on some targets at least. |
Thanks for checking! Yess it does totally make sense to skip nvFuser ones but let's also open an issue for it so that we can track it in the future |
Enable back zero-size test cases that are actually passing