Skip to content

Conversation

@rparolin
Copy link
Collaborator

@rparolin rparolin commented Oct 29, 2025

Implement __hash__ and __eq__ for cuda.core classes

This PR implements __hash__() and __eq__() methods for cuda.core classes (Stream, Event, Context, Device), enabling them to be used as dictionary keys and in sets.

Fixes #664

…lasses, enabling their use as dictionary keys and in sets.
@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented Oct 29, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@rparolin rparolin requested a review from leofang October 29, 2025 18:32
@rparolin
Copy link
Collaborator Author

/ok to test eeef948

@rparolin rparolin marked this pull request as ready for review October 29, 2025 19:07
@rparolin rparolin added the cuda.core Everything related to the cuda.core module label Oct 29, 2025
@rparolin rparolin added this to the cuda.core beta 9 milestone Oct 29, 2025
@rparolin rparolin requested a review from mdboom October 29, 2025 19:11
@Andy-Jost
Copy link
Contributor

+1 for this change. Longer term, it would be great to achieve this sort of thing with a mix-in class. I think what gets in the way is some internal inconsistency, e.g., using _id versus _handle for handle-like members. Perhaps something to consider if we refactor or reimplement anything.

@github-actions

This comment has been minimized.

@rparolin
Copy link
Collaborator Author

+1 for this change. Longer term, it would be great to achieve this sort of thing with a mix-in class. I think what gets in the way is some internal inconsistency, e.g., using _id versus _handle for handle-like members. Perhaps something to consider if we refactor or reimplement anything.

What would be the benefit of using a mixin? I feel like it would require a bunch of machinary of parameterize it to the point it solves the problems you pointed out (ie. being able to handle different handle member names)

@rparolin
Copy link
Collaborator Author

/ok to test 714091c

@rparolin
Copy link
Collaborator Author

/ok to test fff600c

@rparolin
Copy link
Collaborator Author

/ok to test 8227c21

@rparolin rparolin added the P1 Medium priority - Should do label Oct 29, 2025
@rparolin rparolin self-assigned this Oct 29, 2025
@leofang leofang added the enhancement Any code-related improvements label Oct 29, 2025
@rparolin
Copy link
Collaborator Author

/ok to test b8cc00d

@rparolin
Copy link
Collaborator Author

rparolin commented Nov 4, 2025

/ok to test af5c52e

@rparolin
Copy link
Collaborator Author

rparolin commented Nov 4, 2025

/ok to test 8c9900d

@rparolin
Copy link
Collaborator Author

rparolin commented Nov 4, 2025

/ok to test 95047e4

@rparolin rparolin requested review from Andy-Jost and leofang November 4, 2025 23:33
Comment on lines +14 to +16
# ============================================================================
# Context Equality Tests
# ============================================================================
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would probably be difficult, but is there any way we can add a test for when we have different contexts and/or an invalid context in some way?

Copy link
Contributor

@mdboom mdboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comments aren't requirements -- I mostly defer to previous reviewers here.

@rparolin
Copy link
Collaborator Author

rparolin commented Nov 6, 2025

/ok to test ae74733

@rparolin
Copy link
Collaborator Author

rparolin commented Nov 6, 2025

/ok to test 98e8fe0

@rparolin rparolin merged commit 7ca634b into NVIDIA:main Nov 10, 2025
106 of 111 checks passed
@github-actions
Copy link

Doc Preview CI
Preview removed because the pull request was closed or merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.core Everything related to the cuda.core module enhancement Any code-related improvements P1 Medium priority - Should do

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEA]: cuda.core constructs which share underlying C structure should hash to same value

8 participants