mckib2 / pygrappa

Python implementations of GRAPPA-like algorithms.
https://pygrappa.readthedocs.io/en/latest/
MIT License
62 stars 13 forks source link

grappa,py or cgrappa.py not working on anisotropic matrices #41

Closed kanghyunryu closed 4 years ago

kanghyunryu commented 4 years ago

First, I am very impressed with your implementations. However, when applying the grappa for my data which has image size of 300x192 And I found out that neither grappa.py or cgrappa.py (except for lustig_grappa) works on these images. Thank you.

mckib2 commented 4 years ago

Hi @kanghyunryu , I was able to replicate the issue with cgrappa, but my test example works with grappa (see tests/test_nonsquare_matrix_size.py). I believe the issue is in src/get_sampling_patterns.cpp. I'll look into what's going on here.

Is there a way you can send me the data you're using which gives you an error?

mckib2 commented 4 years ago

A quick fix for this issue was released with pygrappa-0.14.1. The test_nonsquare_matrix_size.py script demonstrates the solution. Let me know if this fixes the issue for you.

The plan going forward right now is to maybe use a different, more flexible method for finding unique sampling patterns based on a k-d tree, so I probably won't spend very much time looking into what's going wrong in get_sampling_patters.cpp.

kanghyunryu commented 4 years ago

Thank you for the quick response. I apologize for the mistake, as you mentioned cgrappa is what is causing the error not grappa.py Thanks I thinks everythings are fine and I will be thrilled for your plans on more flexible methods which may accelerate the code. Also, I am wondering about vcgrappa implementation. Your code seems to be working when sampling patterns of regular coil and virtual coils are equivalent (for instance when sampling patterns are 0, R, 2R, ....) However, when regular coil and virtual coil sampling patterns is not equivalent, it does not work. I think it's because your code seems to match the virtual coil sampling patterns ( zero them out when sampling pattern does not match). I am also wondering about the nlgrappa.py, you seems to be augmenting more coils by squaring them. This might not be beneficial where ACS lines are small. Is there any chance of implementing "KerNL: Kernel-Based Nonlinear Approach to Parallel MRI Reconstruction. IEEE Trans Med Imaging. 2019 Jan;38(1):312-321" which seems to be working fine also for small ACS lines. Thanks for your wonderful sharing of codes!

A quick fix for this issue was released with pygrappa-0.14.1. The test_nonsquare_matrix_size.py script demonstrates the solution. Let me know if this fixes the issue for you.

The plan going forward right now is to maybe use a different, more flexible method for finding unique sampling patterns based on a k-d tree, so I probably won't spend very much time looking into what's going wrong in get_sampling_patters.cpp.

mckib2 commented 4 years ago

Hi @kanghyunryu , the vc-grappa implementation simply conjugates the existing coil channels and appends them to the existing coil channels. The sampling pattern between real and virtual coils should be the same, so I'm not sure what you mean by having different patterns. Have I misunderstood?

If you have a dataset that does not share a single sampling pattern between all coil channels, then there does not exist an implementation in pygrappa that can handle it. I can see an argument for this feature if you were doing some kind of joint grappa reconstruction using differently undersampled acquisitions. Please open a new issue if you have this use case, could be interesting to look into, I've not heard of this and unsure how one might go about adapting grappa for it.

nl-grappa currently only has a polynomial kernel data augmentation implemented, and part of that includes channels for so-called "squared coils." This comes straight from the original nl-grappa paper. My implementation does not work as well as I'd like and I find the original implementation provided by the original author in MATLAB difficult to parse, so I'm unsure if there's a better way to implement the paper. I'm happy to look at other grappa-like algorithms, if you have any code for KerNL please submit a pull request!

mckib2 commented 4 years ago

As the original issue has been addressed, I'll go ahead and close this issue. Please feel free open new issues in response to the other items discussed. Thanks!