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.

Sign up for GitHub

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

Jump to bottom

Direct Torch and CUPY support for gpUNUFFT arrays #80

Merged
merged 15 commits into from
Feb 19, 2024
Merged

Direct Torch and CUPY support for gpUNUFFT arrays #80

merged 15 commits into from
Feb 19, 2024

Conversation

chaithyagr
Copy link
Member

This PR adds support for direct GPUNUFFT implementations. This prevents copies between CPU and GPU.
Also helps to better add support to torch tensors

@chaithyagr chaithyagr requested a review from paquiteau February 15, 2024 09:04
@paquiteau paquiteau changed the title Direct Torch and CIPY support for gpUNUFFT arrays Direct Torch and CUPY support for gpUNUFFT arrays Feb 15, 2024
@chaithyagr
Copy link
Member Author

Ready for review, all green soon!

@paquiteau
Copy link
Member

paquiteau commented Feb 15, 2024 •
edited by chaithyagr
Loading

Nicely done ! I think we can use this PR to modify a few things:

  • Use the @with_numpy_cupy decorator also for cufinufft
  • Expose the on-gpu pipe and data consistency method for gpuNUFFT
  • Add a stacked-gpunufft operator (very similar to stacked-cufinufft) for computing the 2.5D nufft (cufinufft is very fast in 2D, it would be nice to compare between the two)

@chaithyagr
Copy link
Member Author

chaithyagr commented Feb 15, 2024 •
edited
Loading

Nicely done ! I think we can use this PR to modify a few things:

  • Use the @with_numpy_cupy decorator also for cufinufft
  • Expose the on-gpu pipe and data consistency method for gpuNUFFT
  • Add a stacked-gpunufft operator (very similar to stacked-cufinufft) for computing the 2.5D nufft (cufinufft is very fast in 2D, it would be nice to compare between the two)

I can handle for 1 and 2 in this PR, 3) to me is a new PR.

@chaithyagr
Copy link
Member Author

DOne with both, ill let the tests run first. However, I would want to test both spectral_radius estimation and also pipe method running ion gpuNUFFT. @paquiteau any tests ? Ideas on where to write them / enable them?

@paquiteau
Copy link
Member

Spectral Radius (a.k.a Lipschitz Constant) is already tested in tests/test_interfaces
For Pipe density compensation , the easiest might be to compare to a known profile (e.g. 1/r ) for radial

@paquiteau paquiteau merged commit b5f5ef1 into mind-inria:master Feb 19, 2024
13 of 16 checks passed
chaithyagr added a commit to chaithyagr/mri-nufft that referenced this pull request Apr 11, 2024
@chaithyagr
Direct Torch and CUPY support for gpUNUFFT arrays (mind-inria#80)
6c17b8c 
* Written codes

* Have shapemismatch checks

* Added support

* Temporary

* add with numpy

* Fix pep508

* Linter stuff

* Added black

* Fixed ruff

* Point to release

* Move to gpuNUFFT

* Add with_numpy_cupy to cufinufft

* Added codes exposing spectral radius and pipe

* fix linting

* update test_ci

---------

Co-authored-by: GILIYAR RADHAKRISHNA Chaithya <cg260486@is247382.intra.cea.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paquiteau paquiteau Awaiting requested review from paquiteau

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chaithyagr @paquiteau