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

Non-uniform FFT layer #27193

Closed
zaccharieramzi opened this issue Mar 27, 2019 · 6 comments
Closed

Non-uniform FFT layer #27193

zaccharieramzi opened this issue Mar 27, 2019 · 6 comments
Assignees
Labels
comp:ops OPs related issues comp:signal tf.signal related issues stat:awaiting response Status - Awaiting response from author type:feature Feature requests

Comments

@zaccharieramzi
Copy link
Contributor

zaccharieramzi commented Mar 27, 2019

System information

  • TensorFlow version (you are using): 1.13
  • Are you willing to contribute it (Yes/No): Yes (but I only know how to code in Python)

Describe the feature and the current behavior/state.
Have a layer similar to the fft layer that will implement the non-uniform FFT.

Will this change the current api? How? This will allow to have one more option when using the signal module, nfft.

Who will benefit with this feature? People working with neural networks involving Non-uniform FFT. The main application I know of is MRI, but according to the Wikipedia page there may be more applications. In particular, I think computed tomography can make use of it to accelerate acquisition times.

Any Other info. Since the NuFFT is not based on the FFT it would require a custom kernel and is therefore not trivial to implement. I haven't looked much into it but it could potentially make use of the Flatiron Institute library.

@muddham muddham added comp:apis Highlevel API related issues stat:awaiting tensorflower Status - Awaiting response from tensorflower type:feature Feature requests labels Mar 28, 2019
@muddham muddham self-assigned this Mar 28, 2019
@muddham
Copy link

muddham commented Mar 28, 2019

@zaccharieramzi Thanks for your contribution. we will keep this noted.

@muddham muddham removed the stat:awaiting tensorflower Status - Awaiting response from tensorflower label Apr 1, 2019
@muddham muddham assigned jvishnuvardhan and unassigned muddham Apr 3, 2019
@jvishnuvardhan jvishnuvardhan added comp:ops OPs related issues and removed comp:apis Highlevel API related issues labels Apr 4, 2019
@jvishnuvardhan jvishnuvardhan added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label Apr 4, 2019
@aselle
Copy link
Contributor

aselle commented Jun 7, 2019

@zaccharieramzi. While this is an interesting possible feature, we have no plans to work on it currently. Let us know if you plan to start this work. It could easily go external to to TF in a custom op library to start and prove out the usability of the feature. Thanks!

@zaccharieramzi
Copy link
Contributor Author

Sure. So for the record I have been working on it in this PR: odlgroup/odl#1488.

It still needs some polishing to fit the requirements of odl but I am using it as is currently in a project and it seems to work as expected.

@tensorflowbutler tensorflowbutler removed the stat:awaiting tensorflower Status - Awaiting response from tensorflower label Jun 8, 2019
@rryan rryan added the comp:signal tf.signal related issues label Sep 19, 2019
@zaccharieramzi
Copy link
Contributor Author

zaccharieramzi commented Mar 2, 2020

So I understood better how non-uniform fft works (which is definitely based on the fft, apologies), and I translated this code from pytorch to tf.
The result is here.

EDIT

You can now install tfkbnufft from pip: pip install tfkbnufft.

@rmothukuru
Copy link
Contributor

@zaccharieramzi,
Can you please let us know if we can close this issue as it has been implemented? Thanks!

@rmothukuru rmothukuru self-assigned this Apr 23, 2021
@rmothukuru rmothukuru added the stat:awaiting response Status - Awaiting response from author label Apr 23, 2021
@zaccharieramzi
Copy link
Contributor Author

Well it has been implemented but not in TensorFlow itself.
Closing anyway as I think it won't be the case in the near future, and the current solution is satisfying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:ops OPs related issues comp:signal tf.signal related issues stat:awaiting response Status - Awaiting response from author type:feature Feature requests
Projects
None yet
Development

No branches or pull requests

7 participants