Open SomayehSaghamanesh opened 2 weeks ago
robbietuk
commented
2 weeks ago The class inheritance is as follows
I suggest using this https://github.com/UCL/STIR/blob/e168b074b3f7d21b4ebf8872680b4bb115022719/src/buildblock/ProjDataInfo.cxx#L123-L129 in the child classes for consistency?
Maybe worth noting, the definition of the max/min tang poss in ProjDataInfoCylindricalNoArcCorr is very old (circa 2003). Somethings may break because they were built upon this methodology.
SomayehSaghamanesh
commented
2 weeks ago Yes, I have already followed it in PR #1508, changed two classes to be consistent with ProjDataInfo.cxx.
robbietuk
commented
2 weeks ago I am sorry, I reread the full function. These values are used for slightly odd checks in initialise_uncompressed_view_tangpos_to_det1det2. I'm not sure what these https://github.com/UCL/STIR/blob/e168b074b3f7d21b4ebf8872680b4bb115022719/src/buildblock/ProjDataInfoGenericNoArcCorr.cxx#L152-L163 checks are, but since both classes' methods are a copy and paste of one another. Your fix in #1508 seems correct to me.
SomayehSaghamanesh
commented
1 week ago Just not sure why 8 tests (including FBP, OSMAPOSL, etc) are failed after these changes. Haven't gone through them yet. Any comment is appreciated.
There are two different definitions (as below), which result in inconsistency in tangential position range. This is problematic at least in cases handling full tangential positions. In ProjDataInfo.cxx: min_tangential_pos_num = -(num_tang_poss / 2); max_tangential_pos_num = min_tangential_pos_num + num_tang_poss - 1;
In ProjDataInfoCylindricalNoArcCorr.cxx and ProjDataInfoGenericNoArcCorr.cxx : const int min_tang_pos_num = -(num_detectors / 2) + 1; const int max_tang_pos_num = -(num_detectors / 2) + num_detectors;