search

MagneticResonanceImaging / MRIReco.jl

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

Fix typo in trajectory function and add 32-bit test for non-CS reconstructions #70

Closed alexjaffray closed 2 years ago

alexjaffray commented 2 years ago

I noticed when converting between RawAcquisitionData and AcquisitionData that some trajectory information wasn't getting carried over, resulting in multiple interleaves being appended upon conversion. This is due to a typo in the trajectory function, where there are underscores where they shouldn't be (i.e traj_ vs traj). I have removed them.

UPDATE

I have been poking around in the test files and found that although all tests pass with #67, the tests for reconstruction which are not CS recons use 64-bit data for the kspace data and sensitivity maps. The default for ISMRMRD is 32 bit, so there should be a test for 32 bit reconstructions using the standard SENSE reconstructions. I wrote one (included in this PR) and it is currently failing on the master. This reflects my experience with my reconstruction pipeline throwing errors for the same reason after pulling the most recent changes.

tknopp commented 2 years ago

Thanks for the catch with Float32. This is a larger issue. I have a look.

tknopp commented 2 years ago

Update: I just pushed a major update to master, which makes the Trajectory parametric. This was tedious work. The tests are passing again but the Float32 reconstruction test you initiated (see https://github.com/MagneticResonanceImaging/MRIReco.jl/blob/master/test/testReconstruction.jl#L429) is still failing.

tknopp commented 2 years ago

Update: I fixed the remaining issue and also included the original fix in RawAcqData. Therefore, closing this PR. Thanks!