MagneticResonanceImaging / MRIReco.jl

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

Optimization of reconstruction_direct for cartesian cases #93

Closed aTrotier closed 1 year ago

aTrotier commented 2 years ago

I am playing a little bit with large cartesian multi-echo datasets. It seems that the bottleneck is the samplingDensity function because it uses the NFFT function to compute it.

When I remove the setupDirectReco steps and the weighting parts of the reconstruction_direct. I get thus benchmarks (for a reduced datasets). with sdc : 11s without : 0.5s

Do we really need that step for the cartesian case ?

tknopp commented 2 years ago

No of course not! I think there is some "isCartesian" property within the trajectory, which can be used to omit this.

aTrotier commented 2 years ago

https://github.com/MagneticResonanceImaging/MRIReco.jl/blob/eb566097ef13f8926e9586dda579a645eee40de2/MRIBase/src/Datatypes/AcqData.jl#L216

Yes but it does not bypass the sdc function. It is the case in setupIterativeReco with :densityWeighting = false :

  # density weights
  densityWeighting = get(recoParams,:densityWeighting,true)
  if densityWeighting
    weights = samplingDensity(acqData,reconSize)
  else
    numContr = numContrasts(acqData)
    weights = Array{Vector{Complex{T}}}(undef,numContr)
    for contr=1:numContr
      numNodes = size(acqData.kdata[contr],1)
      weights[contr] = [1.0/sqrt(prod(reconSize)) for node=1:numNodes]
    end
  end

Maybe I can refactor that and put else part into the samplingDensity which will be called for the cartesian case and only if :densityWeighting = false for non-cartesian ?

Seems good to you ?

Edit : By the way, setupIterativeReco is called even for direct reconstruction

migrosser commented 2 years ago

This is a good point. I just modified the samplingDensity-method, such that it checks whether a trajectory is Cartesian and uses the analytic weights if applicable. Hopefully, this solves the problem.

Thanks for pointing out that setupIterativeReco is called even for direct reconstruction. This is also something which we should address. Should be a simple fix though.

tknopp commented 1 year ago

I think this was fixed in https://github.com/MagneticResonanceImaging/MRIReco.jl/commit/effdcac6bf46addd49f9bef7670b8a0eaa3874f0 If not please re-open.