Tangential position range consistency

[SomayehSaghamanesh]

Changes in this pull request

Changed min/max_tang_pos_num definitions in two classes to be consistent with ProjDataInfo.cxx.

Testing performed

It is built successfully but fails in 8 tests (in make test). Probably needs more modification?
I could handle projection files with tang180 (with lm_to_projdata and convert_projdata_types) without error. Before implementing above changes, I got below error:
"ERROR: The tangential_pos range (-90 to 89) for this projection data is too large. Maximum supported range is from -89 to 90"

Related issues

#1507

Checklist before requesting a review

• I have performed a self-review of my code
• [] I have added docstrings/doxygen in line with the guidance in the developer guide
• [] I have implemented unit tests that cover any new or modified functionality (if applicable)
• The code builds and runs on my machine
• [] documentation/release_XXX.md has been updated with any functionality change (if applicable)

[KrisThielemans]

I'm not sure about this. We have

STIR/src/buildblock/ProjDataInfoCylindricalNoArcCorr.cxx

Lines 206 to 208 in 0e667f7

	uncompressed_view_tangpos_to_det1det2[v_num][tp_num].det1_num = (v_num + (tp_num >> 1) + num_detectors) % num_detectors;
	uncompressed_view_tangpos_to_det1det2[v_num][tp_num].det2_num
	= (v_num - ((tp_num + 1) >> 1) + num_detectors / 2) % num_detectors;

Subtracting

diff =  ((tp_num >> 1) + ((tp_num + 1) >> 1) + num_detectors/2 ) % num_detectors

Checking some examples (and relying on

STIR/src/buildblock/ProjDataInfoCylindricalNoArcCorr.cxx

Lines 168 to 169 in 0e667f7

	BOOST_STATIC_ASSERT(-1 >> 1 == -1);
	BOOST_STATIC_ASSERT(-2 >> 1 == -1);

, it transpires that this is equal to

diff =  (tp_num + num_detectors/2 ) % num_detectors

Therefore, setting tp_num = -(num_detectors/2) gives diff==0. On the other hand, using the current max_tang_pos_num also gives us diff==0. Both are "self-coincidences", which don't make a lot of sense.

It is probably ok to have a too large look-up table (although see this comment, but I think that if you're having this problem, your num_tangential_poss is too large. The max is num_detectors -1 really (which arguably we should check).

[KrisThielemans]

I'm reluctant to merge this. If anything, we should keep the current max. However, this goes rather deep, and as you say 8 tests failed, I cannot see how we can merge as-is to cope with your case where num_tangential_poss=num_detectors_per_ring (which should mean that the first bin is always zero)

[SomayehSaghamanesh]

That's quite a lot of work to resolve this inconsistency. When I trim projdata (which is feasible with trim_projdata), then I can safely work with the current version of max, so I am OK if you skip these changes (merge) and as you said, it fails many tests. So it would be great to only keep those two utilities in the #1563 and skip this merge.

…

On Wed, Nov 12, 2025 at 12:06 PM Kris Thielemans ***@***.***> wrote: ***@***.**** commented on this pull request. I'm reluctant to merge this. If anything, we should keep the current max. However, this goes rather deep, and as you say 8 tests failed, I cannot see how we can merge as-is to cope with your case where num_tangential_poss=num_detectors_per_ring (which should mean that the first bin is always zero) — Reply to this email directly, view it on GitHub <#1508 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXA62ELPYDHNLG3LOJ35ZST34MIETAVCNFSM6AAAAACL32VWKCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTINJSHA4TGNZSGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[KrisThielemans]

ok, I'm going to close this. Further comments on the trimming in #1563