MagneticResonanceImaging / MRIReco.jl

Julia Package for MRI Reconstruction
https://magneticresonanceimaging.github.io/MRIReco.jl/latest/
Other
85 stars 22 forks source link

reshape hack so that ESPIRIT is compatible with the MKL backend of FFTW #118

Closed JakobAsslaender closed 1 year ago

JakobAsslaender commented 1 year ago

Hi, it seems that FFTW with the MKL backend does not support calls like fft!(x, 3:ndims(x)): https://github.com/JuliaMath/FFTW.jl/issues/252

I implemented a hack to circumvent this problem by doing a reshape and then calling fft!(x, 2:ndims(x)-1). If found virtually no speed regression when using the FFTW backend and, well, no error when using the MKL backend ;). (I tested it speed wise with the small example in the test and replace @time with @btime for reliable benchmarking.

codecov-commenter commented 1 year ago

Codecov Report

Base: 66.68% // Head: 66.68% // No change to project coverage :thumbsup:

Coverage data is based on head (e1c28e7) compared to base (8d19c14). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #118 +/- ## ======================================= Coverage 66.68% 66.68% ======================================= Files 37 37 Lines 1696 1696 ======================================= Hits 1131 1131 Misses 565 565 ``` | [Impacted Files](https://codecov.io/gh/MagneticResonanceImaging/MRIReco.jl/pull/118?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MagneticResonanceImaging) | Coverage Δ | | |---|---|---| | [src/Tools/CoilSensitivity.jl](https://codecov.io/gh/MagneticResonanceImaging/MRIReco.jl/pull/118/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MagneticResonanceImaging#diff-c3JjL1Rvb2xzL0NvaWxTZW5zaXRpdml0eS5qbA==) | `73.41% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MagneticResonanceImaging). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MagneticResonanceImaging)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

tknopp commented 1 year ago

Oh this is ugly but I am pragmatic here. The code still makes sense and it basically says: Ignore all dimensions before the spatial ones. So fine for me.