-
Notifications
You must be signed in to change notification settings - Fork 30
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
Start kernel_arg_type enums from 0 #1585
Conversation
Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞 |
Array API standard conformance tests for dpctl=0.17.0dev0=py310h15de555_69 ran successfully. |
The change itself seems fine, did you test whether numerical value of enum members in Python actually changed values because of this change? Should tests be added to verify numerical values of different enum members? |
I've tested in numba-dpex (that's actually how I've found this issue). Definitely we need to test this and other enums in following PRs. |
Array API standard conformance tests for dpctl=0.17.0dev0=py310h15de555_70 ran successfully. |
The new implementation uses values from C enum, and hence the consistency between values in Python and values in C are assured.
I have pushed a change to reimplement This change would drop coverage, and requires adding more tests, feel free to add those, otherwise I'd push changes to add more tests a bit later. |
Array API standard conformance tests for dpctl=0.17.0dev0=py310h15de555_71 ran successfully. |
Array API standard conformance tests for dpctl=0.17.0dev0=py310h15de555_72 ran successfully. |
LGTM! Tested localy for dpex - it works! |
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.
Nice improvements, four years in the waiting!
Fixes IntelPython/numba-dpex#1379 and #1581 . Other enums must be revisited.