CEA-COSMIC / pysap-mri

MRI external plugin for Python Sparse data Analysis Package
Other
43 stars 18 forks source link

Add Support for gpuNUFFT #87

Closed chaithyagr closed 4 years ago

chaithyagr commented 4 years ago

This PR adds support to gpuNUFFT.

Currrent, GPU implementation of NUFFT is pyNUFFT is comparitively slower and also not very memory efficient, and also faces some memory leaks.

The plan is to have this implementation right now and slowly move out from using pynufft, once this is stable.

@mathrip @Daval-G Heres how you can use this right away:

1) Install gpuNUFFT Library :

2) Use gpuNUFFT !

I have done adjoint operation tests on 2D and 3D and also multichannel, however it is local and I have added code updates in tests_local for this.

@zaccharieramzi I am adding you for review of codes and documentation: However, I am not adding gpuNUFFT installation in this PR, as the installation could be complicated based on presence of CUDA and MATLAB (my PR on gpuNUFFT builds both python bindings and MATLAB bindings, I still need to separate out the installations)

Although I cant add @Daval-G and @mathrip to review, being users, I hope you can give valuable feedback about stability and cases that I missed in code.

chaithyagr commented 4 years ago

Adding @Daval-G as official reviewer to this. @Daval-G please ensure the code is sane by running any examples. Feel free to suggest enhancements to both codes and also tests. We will rely on @zaccharieramzi for final review before merge.

chaithyagr commented 4 years ago

@zaccharieramzi I agree to that. In fact now, cuda as an earlier option does not make sense. This is because even gpuNUFFT is written in plain CUDA (while pynufft interfaces with help of reikna). In my opinion, we can call earlier methods as pynufft-cuda and pynufft-opencl and new method as gpuNUFFT

However, I didnt want to make these changes as it will affect some legacy codes and this move must be more gradual, and to be honest, I feel the best thing to do is remove pyNUFFT slowly as the only advantage it offers is openCL version of code.

chaithyagr commented 4 years ago

Added documentation and also updated API to new flow. I will wait for this to pass and merge this. In meantime @zaccharieramzi do let me know if there are any issues..