search

SyneRBI / SIRF

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

pSTIR.ListModeToSinograms.set_template from AcquisitionData #173

Closed KrisThielemans closed 3 years ago

KrisThielemans commented 6 years ago

currently we need a template file. It would be great if we can use an AcquisitionData object. One reason is that we can then easily create a template in Python as in

template_acq_data = AcquisitionData('Siemens_mMR',11,5,2)
lm2sino.set_template(template_acq_data)
evgueni-ovtchinnikov commented 6 years ago

this is STIR issue - LmToProjData.post_processing() fails if template_proj_data_name is empty

KrisThielemans commented 6 years ago

ok. Need to create a STIR issue then on how that needs to be reorganised

evgueni-ovtchinnikov commented 6 years ago

created STIR issue 234

KrisThielemans commented 3 years ago

SIRF tests currently fail on STIR test4, which unlists the data https://travis-ci.org/github/SyneRBI/SIRF-SuperBuild/jobs/759920159#L15169

This will be caused by https://github.com/UCL/STIR/pull/828 which I created to allow modifying set_template ...

It shouldn't go into an infinite loop though!

KrisThielemans commented 3 years ago

The loop is caused because stir::LmToProjData has now a virtual set_up(). Its post_processing() calls its set_up() for backwards compatibility. However, sirf:ListmodeToSinograms has "its own" set_up(), which calls post_processing(). This used to work, but as set_up() is virtual, it means that it will be calling itself...

I guess this is an example of the disadvantage of deriving from STIR classes. SIRF has no control over what they do. C++ rules mean that SIRF can be overriding STIR virtual functions with SIRF not realising...

I think the only thing to do is to call LmToProjData::set_up() as opposed to post_processing(). That is why we went down this track anyway. It will mean that you will have to update your STIR version (to either release_4 or master).

PS: One recommendation with virtual functions is to use the override attribute to let the compiler know that you're overriding. such that it can detect signature errors. But that wouldn't have helped in this case.

evgueni-ovtchinnikov commented 3 years ago

why not just rename set_up of sirf::ListmodeToSinograms? I just have tried it (simply renamed to set_up1) and it fixed the problem.

of course some nice alternative name is needed (preprocess? any other ideas?)

KrisThielemans commented 3 years ago

C++ side was handled in #876. Python and MATLAB side for set_template still to do.