Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STIR's NiftyPET /NIPET support cannot be upgraded to NiftyPET 2 #52

Open
KrisThielemans opened this issue Nov 29, 2023 · 3 comments
Open

Comments

@KrisThielemans
Copy link

@rijobro wrapped NiftyPET into STIR. He had to hard-wire some of the python NIPET initialisations into the STIR C++, but it wasn't too bad.

I've now looked at updating to NIPET2. It seems that more and more initialisation is now only done in Python. For instance, we used get_txlut() to initialise txLUT structure. This is now done in transaxial_lut in mmraux.py.

It makes no sense to "copy" all that Python code into STIR's C++ wrapper. It would mean that we have even more hard-wiring, and presumably will have broken code with the next NIPET update. But I'm assuming you don't want to put all that Python code back into C++...

It therefore looks like we will have to remove NiftyPET's wrapper from STIR sadly, @pjmark @casperdcl any ideas?

@casperdcl
Copy link
Member

Indeed... the ethos is to use Python unless for performance reasons C++/CUDA is needed.

I suppose STIR wouldn't be able to use NiftyPET, but SIRF would?

@KrisThielemans
Copy link
Author

KrisThielemans commented Dec 1, 2023

This is a longer discussion, how to integrate Python-only PET packages into SIRF, and this is not the best place for it.

Briefly, my guess is this would be a fair amount of work due to all the "corrections" (i.e. normalisation etc). I have no idea how that works in NIPET and therefore how much work it would be. It might be easier to just wrap in CIL, but as you know, CIL at the moment needs numpy arrays and therefore CPU-GPU copies (but that might be how NIPET-Python is set-up anyway). It's probably not very high on anyone's priority list though.

@casperdcl
Copy link
Member

casperdcl commented Dec 1, 2023

NiftyPET (NIPET and NIMPA) use CuVec arrays, which are simultaneously both numpy arrays and also CUDA arrays :)

I suppose CIL might indeed be an appropriate place for integrating NiftyPET.

/CC @paskino

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants