mmuckley / torchkbnufft

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

Issue with batched toeplitz kernels #46

Closed andrewwmao closed 2 years ago

andrewwmao commented 2 years ago

Hey Matt, I seem to be consistently getting an error when attempting to call "calc_toeplitz_kernel" with 'omega' with dimension (N, 3, klength) and 'weights' with dimension (N, 1, klength). This seems to occur even when N=1. The error goes away if I simply don't supply the density compensation weights. Any suggestions for how to fix?

error_msg

mmuckley commented 2 years ago

I will probably need to look into this a bit, but it seems like a batching issue.

andrewwmao commented 2 years ago

I should also mention---the calculation works if N=1, I squeeze the batch dimension from omega but not from weights. Not sure if this is expected behavior.

mmuckley commented 2 years ago

More evidence for a batching issue!

mmuckley commented 2 years ago

Hello @andrewwmao, I created #53 and didn't find an issue with current package operation. Could you have a look at the test and see if there are any changes I need to have it match your use case?

andrewwmao commented 2 years ago

Hey Matt, it looks like the basic tests you've written are returning the same error in the calc_toeplitz_kernel call for me, which suggests it's probably a problem on my end.

mmuckley commented 2 years ago

What version of PyTorch are you using? It seems I'm currently testing with 1.10. https://github.com/mmuckley/torchkbnufft/blob/main/requirements.txt

andrewwmao commented 2 years ago

I'm also using version 1.10. Right now I am just calling the function in a loop for better memory efficiency with large N, so I'm not too invested in tracking down the discrepancy at the moment. Thanks for your help!

mmuckley commented 2 years ago

Okay I am going to merge #53 for now and close this issue. If you encounter it in the future feel free to open up a new issue with further details.