-
Notifications
You must be signed in to change notification settings - Fork 44
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
Remove subprocess from skiplists #2505
base: main
Are you sure you want to change the base?
Conversation
Closes intel#800 Signed-off-by: Kirill Suvorov <kirill.suvorov@intel.com>
# Wait until driver complete all the jobs for the device_print, especially test_subprocess | ||
# require this which captures stdout when child exits. | ||
getattr(torch, device).synchronize() |
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.
Is it necessary to move this?
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.
Yes, first of all the synchronize
should be called.
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.
this needs to be done in upstream as well
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.
Looks reasonable, I have created the upstream PR
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.
Is it necessary to move this?
even with the new driver?
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.
Is it necessary to move this?
even with the new driver?
yes
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.
Can you please check if these tests work with 2025.0.0 without moving these lines?
It was checked in Aug, and at that time, they work with 2025.0.0.
# https://github.com/intel/intel-xpu-backend-for-triton/issues/800 | ||
test/unit/language/test_subprocess.py::test_print[device_print-float16] | ||
test/unit/language/test_subprocess.py::test_print[device_print-float32] | ||
test/unit/language/test_subprocess.py::test_print[device_print-float64] | ||
test/unit/language/test_subprocess.py::test_print[device_print_scalar-float16] | ||
test/unit/language/test_subprocess.py::test_print[device_print_scalar-float64] | ||
test/unit/language/test_subprocess.py::test_print[device_print_scalar-float32] | ||
# https://github.com/intel/intel-xpu-backend-for-triton/issues/1704 |
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.
To make sure it works, you need to run corresponding workflow separately.
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.
The no-basekit
and the conda
workflows don't work now. I can leave these skiplists, but I believe it should work as it works for other cases.
Closes #800
A770 checking