MagneticResonanceImaging / MRIReco.jl

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

Extremely slow multiCoil reconstruction for 32 coils #108

Closed alexjaffray closed 1 year ago

alexjaffray commented 1 year ago

I've noticed that 32 channel reconstructions are extremely slow, even for simple Cartesian trajectories. This is new behaviour, as in release 0.5.0 I was able to perform the same reconstructions extremely quickly.

I have added a test in the testCartesianCG branch which replicates this problem.

alexjaffray commented 1 year ago

After some playing around with things, it seems that it is related to type inference, seems like the recent changes to MRIReco and RegularizedLeastSquares result in a lot of type inference overhead, which scales badly with increases in channel count.

For example, 4 coil reconstruction of fully sampled 64x64 phantom data results takes 22 seconds (99.89% of which is compile time) to reconstruct the first time, then 0.017 seconds on subsequent runs.

tknopp commented 1 year ago

Our first reconstructions are indeed quite slow. I am not sure if our recent changes have made things worse.

alexjaffray commented 1 year ago

@tknopp I understand the slowness of the first reconstruction as a general Julia characteristic, however 32 coil reconstruction of a single 64 x 64 slice using simulated data with a Cartesian trajectory should not take > 2 hours on CI... See the CI runs in the testCartesianCG branch for reference. This is a significant problem which should be looked into and was not picked up on the tests since the tests only include a maximum of 8 coils in the reconstruction tasks.

Prior to the changes (i.e in May of this year on v0.5.0) the first reconstruction time for 128 x 128 x 9 slices cartesian data with 32 coils was on the order of 30 seconds, and subsequent reconstructions took ~ 1 second. This exact same task now takes at least 2 hours, perhaps more.

tknopp commented 1 year ago

yes, we need to investigate that.

tknopp commented 1 year ago

This should be fixed by https://github.com/MagneticResonanceImaging/MRIReco.jl/commit/b6c4c8ccc3b7a874574bfb0eda47c60300e287f3

tknopp commented 1 year ago

And https://github.com/MagneticResonanceImaging/MRIReco.jl/commit/d9d22032e0260d175825c3fe2bccbf4fe7d8f015 brings the TTFIR (time to first image reconstruction) from 20 s to about 3.6 s. But watch out, the first using will be much longer, because it does image reconstruction during precompilation. Regular using goes from 15.5 s to 19 s here. This is because it needs to load more precompiled code. But still a clear net win.

alexjaffray commented 1 year ago

This is great! Thanks @tknopp for doing this so quickly!

Our 32 coil reconstructions work on master for spiral and cartesian trajectories so I'll close the issue.

tknopp commented 1 year ago

Perfect, and thanks for bringing this up Alex. Otherwise we would not have been able to fix it.