MagneticResonanceImaging / MRIReco.jl

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

Complex Float32 data are not well handle #103

Closed aTrotier closed 1 year ago

aTrotier commented 1 year ago

While working on a benchmark against bart (which is using Float32). I wanted to create a data acq with that possibility. One of the issue was in CartesianTrajectory3D

I had to change the type of the keyword parameters to Float32

T=Float32
 tr = MRIBase.CartesianTrajectory3D(Float32,128,128,numSlices=128,TE=T(0),AQ=T(0))

Should we change that ?

It is also the same issue for the reconstruction regularization parameters

tknopp commented 1 year ago

yes these need to be fixed but in a clever way. The point is that the user will often specify TE, AQ, lambda ... using Float64 even if the data should be reconstructed in Float32. Thus, these parameters should be promoted to the eltype of the array instead of enforcing them to be of the same type as the array (or in this case the first argument of the function).

aTrotier commented 1 year ago

I'll put that on my to do. Especially for all the high level functions.

Le ven. 9 sept. 2022 à 21:27, Tobias Knopp @.***> a écrit :

yes these need to be fixed but in a clever way. The point is that the user will often specify TE, AQ, lambda ... using Float64 even if the data should be reconstructed in Float32. Thus, these parameters should be promoted to the eltype of the array instead of enforcing them to be of the same type as the array (or in this case the first argument of the function).

— Reply to this email directly, view it on GitHub https://github.com/MagneticResonanceImaging/MRIReco.jl/issues/103#issuecomment-1242379008, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC5P7O7MRXG644EGJFIFNXDV5OFRBANCNFSM6AAAAAAQI2YOJM . You are receiving this because you authored the thread.Message ID: @.***>

migrosser commented 1 year ago

Commit d1efe5d should fix the issue with the type of TE and AQ in the trajectories. Furthermore, I changed setupIterativeReco so that the type of lambda is adapted to the underlying datatype in AcquisitionData in commit b84a9ee

aTrotier commented 1 year ago

Thanks !

Le ven. 30 sept. 2022 à 19:29, migrosser @.***> a écrit :

Commit d1efe5d https://github.com/MagneticResonanceImaging/MRIReco.jl/commit/d1efe5d2c3334e1055612c7dc16e459f13676168 should fix the issue with the type of TE and AQ in the trajectories. Furthermore, I changed setupIterativeReco so that the type of lambda is adapted to the underlying datatype in AcquisitionData in commit b84a9ee https://github.com/MagneticResonanceImaging/MRIReco.jl/commit/b84a9eeabf4fccbadd76b5630e9f2250c46f552a

— Reply to this email directly, view it on GitHub https://github.com/MagneticResonanceImaging/MRIReco.jl/issues/103#issuecomment-1263834009, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC5P7OYAAA4OFPTBUM36MPLWA4PQPANCNFSM6AAAAAAQI2YOJM . You are receiving this because you authored the thread.Message ID: @.***>