MagneticResonanceImaging / MRIReco.jl

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

Move DiagOp to LinearOperatorCollection.jl #168

Open JakobAsslaender opened 7 months ago

JakobAsslaender commented 7 months ago

Quick question: wouldn't it make more sense for DiagOp to be part of LinearOperatorCollection.jl?

This change would actually not be breaking as MRIOperators.jl reexports LinearOperatorCollection.jl.

https://github.com/MagneticResonanceImaging/MRIReco.jl/blob/77dac7813a3a02c6f0c756b982dd015a61e9354b/MRIOperators/src/MRIOperators.jl#L82

tknopp commented 7 months ago

yes that makes sense. We also want to move the CompositionOp to LinearOperatorCollection.jl (and rename it to ProdOp).

nHackel commented 2 months ago

I've moved the operator to the collection package in the GPU PR. One "issue" is/was that the DiagOp uses @floop, which is not a dependency of the collection (yet).

One could do a weak dependency in LinearOperatorCollection. For now I've opted to do a little bit of type piracy :pirate_flag: and have added a dispatch on dense vectors in MRIOperators.jl which uses @floop