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

Create benchmark for espirit #6

Open
tknopp opened this issue Sep 15, 2022 · 13 comments
Open

Create benchmark for espirit #6

tknopp opened this issue Sep 15, 2022 · 13 comments

Comments

@tknopp
Copy link
Member

tknopp commented Sep 15, 2022

Espirit is right now part of benchmark1 which was made on purpose since we wanted to compare the entire chain when we wrote the initial MRIReco publication.

Now that we improved this benchmark suite it would be great to have a dedicated benchmark for the coil estimation and we could probably use the same sensitivity profiles in benchmark1.

@JakobAsslaender: This might be interesting for you (since you worked on that code):

  • have you made any time comparisons?
  • have you made any accuracy comparisons?
@aTrotier
Copy link
Contributor

Last time I tried it MRIReco espirit was far more slower than BART espirit. But I did not really investigates why (ComplexF32 etc, parameters etc)

I will create a benchmark for that on CI to check if it is still the case

@JakobAsslaender
Copy link
Member

Not sure when you last tried. I put a lot of work into making ESPIRIT faster with substantial success. The biggest difference to BART that I am aware of is this: MagneticResonanceImaging/MRIReco.jl#55 (comment) , which is on my TODO list, but I honestly won't get around to do this any time soon. So anyone feel free to give it a go.

To answer @tknopp question: I did not do any systematic comparison so far and it could well be that there are other bottlenecks that make the Julia code slower compared to BART, which is certainly well optimized.

When relocating the benchmark to a dedicated benchmark, we might use this as an opportunity to move the actual code out of the MRIReco package into a dedicated MRICoilSensitivity.jl package or similar? It would make things slimmer and more modular to use. Thoughts, @tknopp ?

@aTrotier
Copy link
Contributor

@JakobAsslaender just tried it on one of my project and espirit is actually "fast" enough to be used in it.

@aTrotier
Copy link
Contributor

aTrotier commented Sep 29, 2022

@JakobAsslaender bart = 42sec and julia = 157 sec
Matrix 128x128x128, channel = 8

https://atrotier.github.io/MRIRecoVsBART_Benchmark/benchmark_espirit.html

@JakobAsslaender
Copy link
Member

Great, thanks for setting it up! This is a great benchmark for further developments! I will use it when I get around to implementing above mentioned Uecker-magic.

@tknopp
Copy link
Member Author

tknopp commented Oct 3, 2022

@aTrotier: This looks good and I try to integrate it into our infrastructure. One question though? How can I get a ground truth sensitivities in order to compare accuracy?

@aTrotier
Copy link
Contributor

aTrotier commented Oct 3, 2022

if you use the option -S rather than -s, bart returns the sensitivity maps :

sens_ground_truth = bart(1,"phantom -3 -x$sx -S$nch");

@tknopp
Copy link
Member Author

tknopp commented Oct 3, 2022

I added becnchmark3 with the espirit comparison. The sensitivities look like this:

Bildschirmfoto 2022-10-03 um 20 33 50

More interesting are the timings:
Bildschirmfoto 2022-10-03 um 20 36 25

Here also with a logarithmic y-axis:
Bildschirmfoto 2022-10-03 um 20 34 29

I had no look at any performance optimizations so far.

@tknopp
Copy link
Member Author

tknopp commented Oct 3, 2022

Some first progress by MagneticResonanceImaging/MRIReco.jl@acff670:

Bildschirmfoto 2022-10-04 um 00 50 32

@tknopp
Copy link
Member Author

tknopp commented Oct 4, 2022

Teamwork over at MagneticResonanceImaging/MRIReco.jl#111

Bildschirmfoto 2022-10-04 um 22 31 51

And just for verification. Here are the maps:

Bildschirmfoto 2022-10-04 um 22 31 04

@aTrotier
Copy link
Contributor

aTrotier commented Oct 4, 2022

Great job !

@JakobAsslaender
Copy link
Member

Nice! I am very happy to loose to the pros over at BART by this margin ;)

@JakobAsslaender
Copy link
Member

Oh, realizing only now that our code is actually faster. 🚀

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

3 participants