Open KrisThielemans opened 4 years ago
Correct me if I am wrong, but we can sort this out by reading the data in the correct order, right?
For example, the z
axis issue is simply making sure that when RDF9 sinograms are loaded, they are loaded with an axial flip, isn't it? I am not sure how all the geometric stuff is stored in STIR for all this data, so it may not work.
Now that I think about it perhaps some of our issues with axis flipping and non flipping (in some cases apparently) may come from using directly RDF data from using STIR sinograms (as I think we tested how similar they were in axially symmetric data). I need to trippe check all this again.
I agree that we can sort this out by reading the data in the correct order. However, this sits in various places at present. For instance https://github.com/UCL/STIR/blob/e92a728536df146a4f9e4a9d6b7e1f6332621c05/src/buildblock/ProjDataGEHDF5.cxx#L163-L169 and https://github.com/UCL/STIR/blob/e92a728536df146a4f9e4a9d6b7e1f6332621c05/src/include/stir/listmode/CListRecordGESigna.h#L76-L78 (Note that there's still a hardwired number there!) https://github.com/UCL/STIR/blob/e92a728536df146a4f9e4a9d6b7e1f6332621c05/src/IO/GEHDF5Wrapper.cxx#L825-L828
Possibly some other places as well.
@KrisThielemans I did try to put everything inside the Wrapper, but I seem to have missed CListRecordGESigna.cxx Completely!
I tried to list all the flips outside the wrapper here: https://github.com/UCL/STIR/issues/636. I have now added the couple I was missing.
@KrisThielemans hum it looks like that one + the one in line 168 cancel each other out?
could be, but another (better?) way to look at it is that both bits of code go from STIR-views to GE-views. initialise_proj_data
takes a GE-view, and is used to fill tof_data
, which is entirely indexed via GE-views (and GE axial-pos
which runs over both segments and STIR axial-positions; and GE tangential-pos
). So that makes sense (either we have tof_data
in GE coordinates, or in STIR coordinates, but definitely not in a mixture of the 2!)
According to current code, we have:
tangential_pos
flip, view
->num_views - 1 - view
, something for segments/axial positionsaxial_coord
, tangential_coord
-> num_tangential_coords() - 1 - tangential_coord
These are all interdependent of course.
I think the required change would be
tangential_pos
flip, no view
change, change the segments/axial positions relationaxial_coord
-> num_axial_coords 1 - axial_coord
, no change in tangential_coord
It looks like it would become simpler...
@KrisThielemans two questions:
invert_axis
, how does this work? Can you define a projector with inverted axis (and flipped ones)?the transformation you're describing in your last comment, is it GE -> STIR convention or the contrary?
I think I was talking about GE->STIR, but happily, the described conversions are symmetric as far as I can see!
you mention a workaround by using invert_axis, how does this work? Can you define a projector with inverted axis (and flipped ones)?
we currently cannot invert during projection. Might be possible to add (@danieldeidda added it in the SPECT projector), at the risk of slowing that down a bit more. At present, have to do it before or after. Example is here. Again those operations are symmetric.
the described conversions are symmetric as far as I can see!
Right!
According to current results with Signa data and Discovery MI (using #601), DICOM files from the scanner convert to STIR Interfile are flipped in "x" and "z" compared to STIR reconstructions in Interfile. This has drastic consequences for AC of course, or anything else.
Our current feeling is that this is because the GERDF9 code uses a different axis convention from the rest of STIR (in particular, the one used for the Siemens mMR).
Current work-around is to use
invert_axis
, but this will need work for data with offsets and of course makes it impossible to match up coordinates.