Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Add binding for spread and interpolate, and also maybe support density estimator #306

Closed
chaithyagr opened this issue Jul 4, 2023 · 5 comments

Comments

@chaithyagr
Copy link
Contributor

The spread and interpolate are equally important kernels and it will be nice to have them exposed also.

Additionally, it would help to have density compensation estimators in place.

Most are implemented in tensorflow-nufft and tensorflow-mri already (although these features are not limited to MRI)

@ahbarnett
Copy link
Collaborator

@wendazhou is writing a faster spreader as a separate repo, which we hope to then use in finufft.

But for now if you work in C++ you can already call anything in libfinufft.so, looking in the finufft::spreadinterp namespace. You could easily write a Py wrapper to them.

But writing wrappers exposing them cleanly from all 5 languages is not worth it for us yet.

@chaithyagr
Copy link
Contributor Author

If I do happen to write the interface for python, can I make a PR?

@wendazhou
Copy link
Contributor

Hi @chaithyagr ,

As Alex mentioned, I am in the process of finishing up a standalone spreading package which should be significantly more performant (making use of explicit vectorization) and flexible (e.g. user configurable polynomial weights, spreading to both real-valued and complex-valued targets etc.) and an initial version should be available next week. I will probably have some rudimentary python bindings by then.

Did you have a specific application / problem in mind? I can try to check if your usage would fit within the API of that package, and modify the API if necessary.

@chaithyagr
Copy link
Contributor Author

Hi @wendazhou , I have made my PR, the uses of it is in mri-nufft, which is applied specifically for MRI, and to estimate the Density compensators.

@ahbarnett
Copy link
Collaborator

Updates:

  1. Wenda has moved to OpenAI a few months ago, and I doubt will have time to write this package, but Robert Blackwell is starting to benchmark his SIMD-optimized speedups.
  2. This is a feature request that is relatively complicated to implement, and breaks the mathematical framework of what a NUFFT is. Therefore I'm moving this to an ongoing Discussion.
    I look forward to understanding why precisely things beyond a NUFFT are needed as part of this library. Best, Alex

@flatironinstitute flatironinstitute locked and limited conversation to collaborators Nov 29, 2023
@ahbarnett ahbarnett converted this issue into discussion #378 Nov 29, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants