Fix incorrect tests in test_usm_ndarray_dlpack#1982
Merged
ndgrigorian merged 1 commit intomasterfrom Feb 1, 2025
Merged
Conversation
These tests would fail on machines with more than 2 devices for a given platform due to an incorrect asusmption that the DLPack device ID would match that of the cached root devices, of which only 2 are kept per platform
|
Deleted rendered PR docs from intelpython.github.com/dpctl, latest should be updated shortly. 🤞 |
|
Array API standard conformance tests for dpctl=0.19.0dev0=py310h93fe807_503 ran successfully. |
Collaborator
oleksandr-pavlyk
approved these changes
Feb 1, 2025
Contributor
oleksandr-pavlyk
left a comment
There was a problem hiding this comment.
Yes, you're right. Expect this change to slow down run-time of this test file on the machine with many cards, but correctness first.
Was it failing in Tiber?
Collaborator
Author
It was during a test run on a machine with two 1550 Max cards. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The test
test_dlpack_deviceattempted to query a device using DLPack device ID from the list of root devices created byall_root_devicesfixture intest_usm_ndarray_dlpack.This logic is incorrect, as
all_root_devicescaches only the first two root devices per platform. As the DLPack device ID is based on the ordinal ID of the device as it is canonically presented inget_devices, this could fail if a given platform had more than 2 associated devices (in this case, for a machine with 2 PVC. Each has 2 stacks, and therefore, 4level_zeroandopencl:gpudevices).