-
Notifications
You must be signed in to change notification settings - Fork 13
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
Change density to density_compensators #114
Change density to density_compensators #114
Conversation
Don't worry too much about the density tests, density compensations methods will probably need a new tests setup in the future. |
This can be merged, @paquiteau ? |
Maybe we should add the same normalization as you did for gpuNUFFT ? |
I was just going through the codes, we seem to have redundant similar pipe code at: mri-nufft/src/mrinufft/operators/interfaces/tfnufft.py Lines 139 to 157 in f49c8c0
@Lenoush maybe can you do the same update there also and then make the call to the Also can you please also implement the normalization like it is done at : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there ! This keeps getting better.
One final thing left I guess: Change test_gpunufft.py to test_density,py and add a test to test the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, We can merge this right @paquiteau ?
change : tfmri.estimate_density( samples, shape, method="pipe", max_iter=15)
to : tf.math.reciprocal_no_nan(tfmri.estimate_density( samples, shape, method="pipe", max_iter=15 ))