SyneRBI
SIRF data algebra optimised
Changes in this pull request
Merges array-ptr (optimising mostly image data) and try-fix-algebra (optimising mostly acquisition data) branches.
Testing performed
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.
@evgueni-ovtchinnikov I'm a bit confused. Do we need this PR? Better to get smaller ones merged first. Maybe it's just a test to see if they work together?
@KrisThielemans I cannot push to Casper's array-ptr branch, so I had to work on my cloned copy of it. As Casper kept adding new stuff to array-ptr, I had to clone more copies, so now I have
array-ptr-my
array-ptr-qfix
contiguity-check
contiguity-check-my
...
This caused conflicts resulting in some code I moved from one script to another being moved back on the next iteration (e.g. TODO bit in acquisition_data.py that I moved to a new script asarray.py).
This is of course mostly due to my lack of understanding GitHub machinery.
Recently I remembered about my branch try-fix-algebra of Dec 2024, on which I achieved the reduction of timings for PET acquisition and image algebra by factors of 2 and 15 respectively, so I renamed it data-algebra-opt, merged array-ptr on it and cleaned it up. Trying to merge master on it today gave me Already up to date message, which I understand indicates that the two branches do not have conflicts, and data-algebra-opt can be safely merged on master (GitHub shows a lot of differences between the two, but most of them are either clean additions of new stuff or whitespace issues).
Latest performance data:
sirfuser@vagrant:~/devel/buildVM/sources/SIRF/tests$ python3 pet_algebra_timings.py ~/Laptop/PET/kappa.hv 5
test sirf as_array + numpy + fill = total with asarray
x * 2 8.38e-02 4.99e-01 + 5.21e-03 + 4.84e-01 = 9.88e-01 6.58e-03
x + 2 7.79e-02 5.12e-01 + 5.64e-03 + 4.86e-01 = 1.00e+00 6.07e-03
x + y 8.80e-02 9.94e-01 + 7.04e-03 + 4.84e-01 = 1.49e+00 7.53e-03
x * y 1.11e-01 9.95e-01 + 8.46e-03 + 4.87e-01 = 1.49e+00 8.56e-03
x / y 1.12e-01 9.92e-01 + 6.75e-03 + 4.82e-01 = 1.48e+00 8.59e-03
To remind you, PETRIC participants complained about SIRF data algebra performing worse than using as_array + numpy + fill, which means we made SIRF data algebra more than 200 times faster than it was before PETRIC (look at x + y)!
