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

Add docs on autodiff and difference with original reference, use of isign etc. #223

Open
mmlyj opened this issue Dec 6, 2024 · 2 comments
Assignees

Comments

@mmlyj
Copy link

mmlyj commented Dec 6, 2024

Hi,dear authors.elegant implementation of your codes, I'm trying to replace my ugly code with yours.But when I'm reading the code, I notice that there is a change of isign in finufft and cufinutt for autodiff in adjnufft? Why?I didn't see similar thing as in this link(https://github.com/guanhuaw/Bjork). It seems that I miss some basic knowledge. you are trying to use same code to calculate the gradient of traj for all backend? But I also notice that your other backend didn't have funtions like toggle_grad_traj( if I didn't understand it rightly). It would be grad that you give me some tips about it.
Thanks in advances

@chaithyagr
Copy link
Member

Hello @mmlyj , thank you for your interest in our work. Indeed, you are right, we use a different way to compute our gradients by using the isign in finufft, cufinufft and gpuNUFFT, as compared to what was done by original authors. Sorry that I didnt update this in docs. I think thats the best way to explain rather than in issue here, so I shall use this issue to update the docs accordingly.

But I also notice that your other backend didn't have funtions like toggle_grad_traj( if I didn't understand it rightly).

Can you point to the codes? If I remember correctly we currently only support finufft for CPU and cufinufft and gpuNUFFT for GPU for autodiff capabilities. TorchkbNUFFT and tensorflow-nufft support it internally and as they are usually slower, we didnt add support for it

@chaithyagr chaithyagr changed the title Adds support to isign in finufft, cufinufft,gpunufft? Add docs on autodiff and difference with original reference, use of isign etc. Dec 6, 2024
@chaithyagr chaithyagr self-assigned this Dec 6, 2024
@mmlyj
Copy link
Author

mmlyj commented Dec 6, 2024

It's so nice of your quick replys.It makes sense for me now that "the current vsersion only support finufft for CPU and cufinufft gpuNUFFT for GPU for autodiff capabilities". As other backend don't support autodiff, there is no need to inplement the toggle_grad_traj in other backend(/operators/autodiff.py line 81,101,).Glad to see the doc about the isign.Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants