Add __eq__ and __hash__ to Buffer, LaunchConfig, Kernel, ObjectCode #1534
Conversation
Make these classes hashable and comparable: - Buffer: identity based on (type, ptr, size) - LaunchConfig: uses _LAUNCH_CONFIG_ATTRS tuple for forward-compatible identity; also updates __repr__ to use the same attribute list - Kernel: identity based on (type, handle) - ObjectCode: identity based on (type, handle), triggers lazy load Stream, Event, Context, Device already had __eq__/__hash__.
Make these classes hashable and comparable: - Buffer: identity based on (ptr, size) - LaunchConfig: uses _LAUNCH_CONFIG_ATTRS tuple for forward-compatible identity; also updates __repr__ to use the same attribute list - Kernel: identity based on handle - ObjectCode: identity based on handle, triggers lazy load Remove type(self) from __hash__ in Event, Context, Buffer, LaunchConfig, Kernel, ObjectCode - CUDA handles are already unique, and removing type(self) maintains the hash/equality contract for subclasses. Add parameterized hash tests for all 8 core API classes. Stream, Device already had correct __eq__/__hash__ without type(self).
Andy-Jost
added
enhancement
|
/ok to test e182f4c |
| def sample_device(init_cuda): | ||
| return Device() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_stream(sample_device): | ||
| return sample_device.create_stream() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_event(sample_device): | ||
| return sample_device.create_event() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_context(sample_device): | ||
| return sample_device.context |
There was a problem hiding this comment.
Once this change and the change adding __weakref__ members are both merged, I plan to revisit these fixtures (which overlap with test_weakref.py). There is an opportunity to rewrite and simplify test_hashable.py, test_comparable.py and test_weakref.py, possibly combining them.
This comment has been minimized.
This comment has been minimized.
|
/ok to test a208225 |
|
/ok to test e819a5a |
I think the reason to include |
| def _identity(self): | ||
| return tuple(getattr(self, attr) for attr in _LAUNCH_CONFIG_ATTRS) |
There was a problem hiding this comment.
It may be worth caching this
There was a problem hiding this comment.
Is LaunchConfig intended to be immutable? Currently it is not:
>>> config = LaunchConfig(grid=(1,2), block=1)
>>> config
LaunchConfig(grid=(1, 2, 1), cluster=None, block=(1, 1, 1), shmem_size=0, cooperative_launch=False)
>>> config.grid = (2,2,2)
>>> config
LaunchConfig(grid=(2, 2, 2), cluster=None, block=(1, 1, 1), shmem_size=0, cooperative_launch=False)
There was a problem hiding this comment.
In theory we could invalidate the cache on setter functions, but we can punt this to if it ever becomes a bottleneck.
Change object_code fixture to compile to cubin instead of ptx to avoid CUDA_ERROR_UNSUPPORTED_PTX_VERSION when the toolkit version is newer than the driver version on test machines. Also remove outdated "type salt" reference from assertion message.
|
/ok to test 09b11e6 |
I thought so as well, but then I found tests explicitly checking that subclasses compare equal to non-subclasses managing the same driver resource. This also came up in a recent issue where it was decided for streams to use |
|
2 participants
Summary
Complete hashability support for core API classes by adding
__eq__and__hash__to Buffer, LaunchConfig, Kernel, and ObjectCode.Changes
__eq__and__hash__to Buffer, LaunchConfig, Kernel, ObjectCodetype(self)from__hash__in Event, Context (and the new classes) - CUDA handles are already unique, and this maintains the hash/equality contract for subclasses_LAUNCH_CONFIG_ATTRStuple for forward-compatible identity in LaunchConfig__repr__in LaunchConfig to use the same attribute listIdentity basis:
(ptr, size)(grid, cluster, block, shmem_size, cooperative_launch)handlehandle(triggers lazy load)Stream, Event, Context, Device already had
__eq__/__hash__.Test Plan
test_hashable.pycover all 8 core classes