MagneticResonanceImaging / MRIReco.jl

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

Add option for vectors and tuples #172

Closed JakobAsslaender closed 6 months ago

JakobAsslaender commented 6 months ago

Hi, this is a minor change adding the option to call DiagOp with a vector of operators or an NTuple of Operators. My goal is to prevent having to use the splat operator to improve speed.

The change should be non-breaking.

@tknopp: If approved, would you mind taking care of the release, as I still don't know how to release this multi-package system of MRIReco.jl

Thank you!!

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (77dac78) 86.68% compared to head (281cf80) 86.68%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #172 +/- ## ======================================= Coverage 86.68% 86.68% ======================================= Files 10 10 Lines 368 368 ======================================= Hits 319 319 Misses 49 49 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

JakobAsslaender commented 6 months ago

@tknopp, a friendly ping.

Can you approve, merge, and release?

tknopp commented 6 months ago

done, release it triggered!

JakobAsslaender commented 6 months ago

Thank you!

I just realized that LinearOperators.jl has similar functionality: https://github.com/JuliaSmoothOptimizers/LinearOperators.jl/blob/7a5456fb5d9c39c3ce29bc212e5f3c18943e6172/src/special-operators.jl#L241

Any reason to duplicate that work–or shall we decrepate DiagOp? I see that their mul! operation is not parallelized, but that could maybe be changed with a PR.