Add support for CUDA Array Interface for interoperability with other Python libraries (resolves #633)#675
Merged
neworderofjamie merged 11 commits intogenn-team:masterfrom May 5, 2025
Merged
Conversation
Contributor
JRV7903
commented
May 4, 2025
JRV7903
commented
- Added the cuda_array_interface property to the Array class to expose CUDA array data for interoperation.
- Implemented getDevicePointer() in the Array class to retrieve device pointers.
- Updated the cudaBackend.cc and backend.h files to handle the device pointers and support CUDA-specific array operations.
- Modified model_preprocessor.py to enable the inclusion of the cuda_array_interface property for array interoperability.
- Created a new test file test_cuda_interface.py for verifying the changes and ensuring the correct functionality of the CUDA array interface.
Making it pytest compatible because i somehow forgot to do it earlier
neworderofjamie
requested changes
May 4, 2025
Contributor
neworderofjamie
left a comment
neworderofjamie
left a comment
There was a problem hiding this comment.
This looks really great - nice work! Just a few minor suggestions
Co-authored-by: neworderofjamie <neworderofjamie@gmail.com>
Co-authored-by: neworderofjamie <neworderofjamie@gmail.com>
Co-authored-by: neworderofjamie <neworderofjamie@gmail.com>
- Removed unnecessary numpy dtype parameter in test_cuda_interface - Added backend_cuda fixture for selective test execution - Moved CUDA-specific doc comments from backend.cc to backend.h - Made getDevicePointer inline in the header - Fixed dtype resolution bug in model_preprocessor.py - Added CuPy to test dependencies and CI install list
Contributor
Author
|
Thank you for reviewing the PR. I’ve fixed the bug so that CuPy now correctly receives a NumPy datatype. I’ve also implemented the other requested changes. |
neworderofjamie
requested changes
May 5, 2025
Contributor
neworderofjamie
left a comment
neworderofjamie
left a comment
There was a problem hiding this comment.
Amazing! If you can accept my one suggestion, I will merge
Co-authored-by: neworderofjamie <neworderofjamie@gmail.com>
Contributor
Author
|
Thanks for the review! I’ve applied the dtype fix only to the CUDA array interface implementation, where it was necessary. I reverted the change in set_array, since NumPy’s view() works correctly with string-based dtype representations. Appreciate the feedback and the learning opportunity! |
neworderofjamie
approved these changes
May 5, 2025
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
2 participants
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.