search

SyneRBI / SIRF

Main repository for the CCP SynerBI software
http://www.ccpsynerbi.ac.uk
Other
58 stars 29 forks source link

error in `CoilSensitivityData.calculate` in SIRF-Exercises #949

Closed KrisThielemans closed 3 years ago

KrisThielemans commented 3 years ago

When my SIRF is compiled with ISMRMR 1.4.1, running SIRF-Exercises/Introductory/acquisition_model_mr_pet_ct.ipynb gave

mr_acq = mr.AcquisitionData(os.path.join(examples_data_path('MR'),'grappa2_1rep.h5'))
preprocessed_data = mr.preprocess_acquisition_data(mr_acq)

csm = mr.CoilSensitivityData()
csm.smoothness = 50
csm.calculate(preprocessed_data)

This gives me an error

error: ??? "'binary algebraic operations cannot be applied to unsorted data'

@ckolbPTB do you see this as well?

ckolbPTB commented 3 years ago

preprocessed_data.sort()

needs to be added after preprocessed_data = mr.preprocess_acquisition_data(mr_acq)

johannesmayer commented 3 years ago

Thanks, that looks like the reason, @ckolbPTB . I will modify the C++ layer s.t. when it reads in the data it automatically sorts them by time, s.t. actually we never need to call the sort() method from Python again. It does not do anything anymore anyway.

KrisThielemans commented 3 years ago

@evgueni-ovtchinnikov any reason why not to call sort automatically? I'm assuming that the overhead is low?

johannesmayer commented 3 years ago

Ah I just saw, that this is not the problem, (as @ckolbPTB actually said), but the return value of the preprocess_acquisition_data() method is not sorted.

KrisThielemans commented 3 years ago

ok. The question is what is the best: let it sort it on time automatically, or let the user do it, or let the CSM stuff call sort. Up to the MR experts...

johannesmayer commented 3 years ago

Ok, so I proposed a solution in PR #950 (there are now more PR's than issues :dancing_men: ). Feel free to tell me to modify it there in the PR 👍