[SyneRBI/SIRF] Hackathon project8: Expose CUDA-managed STIR image queries in sirf.STIR (PR #1380)
Dimitra-Kyriakopoulou
This PR is PART of Hackathon Project 8: update STIR data to CUDA managed pointers, then adapt sirf.STIR data-containers (images first) and expose the underlying CUDA managed pointer to Python. This PR is the SIRF part of that work. The companion STIR changes are available in the fork at Dimitra-Kyriakopoulou/STIR, branch Hackathon_Project08, and the runnable benchmark is in the repository Dimitra-Kyriakopoulou/ hackathon_project_08_stir_cuda_managed_pointers.
Changes in this pull request
- Added image-level CUDA-managed-memory queries in cSTIR.
- Added Python exposure in sirf.STIR for the managed-image path.
- The new Python-facing image queries expose whether the image is CUDA-managed and the underlying address, and provide a CUDA-array-style view path for Python consumers.
- Scope is deliberately images first, matching the Project 8 brief.
NOTE: Project 8 requires more than a STIR internal fix: sirf.STIR also has to recognise the managed-memory-backed image and expose it at the Python layer.
Testing performed
- Validated the C++ SIRF image probe end to end (ok sirf image elements=24).
- Validated Python import of sirf.STIR with the managed-image factory.
- Checked the new Python image properties on a real managed image:
- cuda_array_interface
- Ran the phantom-clone benchmark through the managed-image path; the managed image matched the source phantom exactly.
NOTE: The runnable proof-of-use benchmark is kept separately in hackathon_project_08_stir_cuda_managed_pointers.
Related issues
Checklist before requesting a review
- I have performed a self-review of my code
- I have added docstrings/doxygen in line with the guidance in the developer guide
- I have implemented unit tests that cover any new or modified functionality
- [ x] The code builds and runs on my machine
- CHANGES.md has been updated with any functionality change
Contribution Notes
Please read and adhere to the contribution guidelines.
Please tick the following:
- [ x] The content of this Pull Request (the Contribution) is intentionally submitted for inclusion in SIRF (the Work) under the terms and conditions of the Apache-2.0 License.
You can view, comment on, or merge this pull request online at:
https://github.com/SyneRBI/SIRF/pull/1380
Commit Summary
- d305813 Expose CUDA-managed STIR image queries
- 4ede79f Fix typo and enhance README clarity
- ed3aa08 Update CMakeLists.txt
- 91ab0f8 Temporarily enable CI for Hackathon_Project8
- 2dfc6ad Update build-test.yml
File Changes
(7 files)
- M .github/workflows/build-test.yml (3)
- M examples/C++/CMakeLists.txt (2)
- M examples/README.md (2)
- M src/xSTIR/cSTIR/cstir_p.cpp (4)
- M src/xSTIR/cSTIR/include/sirf/STIR/stir_data_containers.h (2)
- M src/xSTIR/cSTIR/stir_data_containers.cpp (44)
- M src/xSTIR/pSTIR/STIR.py (22)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Dimitra-Kyriakopoulou
@Dimitra-Kyriakopoulou pushed 1 commit.
- f01041d Guard CUDA runtime include for non-CUDA analysis
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
@KrisThielemans commented on this pull request.
Initial impressions of this are good. More thorough review will follow later.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Dimitra-Kyriakopoulou
@Dimitra-Kyriakopoulou pushed 1 commit.
- 823343d Limit Hackathon_Project8 CI to STIR-related SIRF components
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Dimitra-Kyriakopoulou
Dimitra-Kyriakopoulou left a comment (SyneRBI/SIRF#1380)Dear Professor @KrisThielemans,
THANK YOU WHOLEHEARTEDLY!!!
- The failing online test seems to be in xGadgetron/MR, while this branch only changes xSTIR. Locally I validated the STIR-side path, so I did not see that Gadgetron failure. I therefore now pushed a temporary CI adjustment to focus this branch on the STIR-related components it changes.
- After pushing that change, however, I lost the previous full-suite comparison and I think I may also have seen another failure... If so, unfortunately you will need to rerun the tests twice -which would likely have been unnecessary if I had documented the earlier failures more carefully before changing the workflow: I AM REALLY SORRY ABOUT THAT.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
KrisThielemans left a comment (SyneRBI/SIRF#1380)I think this should be closed, as all functionality is in #1381. If so, please close it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Dimitra-Kyriakopoulou
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.