search

mmuckley / torchkbnufft

A high-level, easy-to-deploy non-uniform Fast Fourier Transform in PyTorch.
https://torchkbnufft.readthedocs.io/
MIT License
201 stars 44 forks source link

ToepNufft's scale is inconsistent with KbNufft when norm = 'ortho' and grid size != 2*im_size #38

Closed guanhuaw closed 2 years ago

guanhuaw commented 2 years ago

Hi Matthew, Thanks for such an elegant implementation for NUFFT on Pytorch. I have on little question: when choosing 'ortho' norm and the grid_size is not exactly 2*im_size, the scale of ToepNUFFT is inconsistent with AdjKbNufft(KbNufft()). This also happens before 1.0.0. Thanks in advance.

mmuckley commented 2 years ago

Hello @guanhuaw, thanks for bringing this up. Are the values consistent without the use of norm="ortho"?

guanhuaw commented 2 years ago

Thanks! When the norm = None, the values are consistent under my test cases.

mmuckley commented 2 years ago

Working on this (slowly, after work on my own time). I created a test suite in this new branch and was able to reproduce the issue.

mmuckley commented 2 years ago

@guanhuaw PR #39 should fix this. Would you be willing to review it and try it on your use case?

mmuckley commented 2 years ago

I'm going to go ahead and merge. If you encounter issues with the new version please let me know.

guanhuaw commented 2 years ago

Hi @mmuckley thanks for the update! When testing with the latest commit, it seems that norm = None or norm = 'ortho' will produce the same results for the ToepNufft module, deviant from v1.2. My guess is that the updated calc_toeplitz_kernel function causes this problem.

mmuckley commented 2 years ago

Hello @guanhuaw could you clarify the problem? I don't understand your message.

guanhuaw commented 2 years ago

Thanks! The results are exactly the same for norm = None or norm = 'ortho' scenarios, and there is no scaling factor between the two.