Skip to content

Tangential position range consistency #1508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SomayehSaghamanesh wants to merge 2 commits into from
Closed

Tangential position range consistency #1508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6ad92a
Initial commit
SomayehSaghamanesh Sep 5, 2024
716f95d
Modified min/max_tang_pos_num to be consistent with ProjDataInfo.cxx
SomayehSaghamanesh Sep 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ install_manifest.txt
build/
install/
.vscode/
build_safir_updates/

# MATLAB

Expand Down
4 changes: 2 additions & 2 deletions src/buildblock/ProjDataInfoCylindricalNoArcCorr.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ ProjDataInfoCylindricalNoArcCorr::initialise_uncompressed_view_tangpos_to_det1de
assert(fabs(get_phi(Bin(0, 0, 0, 0)) - v_offset) < 1.E-4);
assert(fabs(get_phi(Bin(0, get_num_views(), 0, 0)) - v_offset - _PI) < 1.E-4);
#endif
const int min_tang_pos_num = -(num_detectors / 2) + 1;
const int max_tang_pos_num = -(num_detectors / 2) + num_detectors;
const int min_tang_pos_num = -(num_detectors / 2);
const int max_tang_pos_num = -(num_detectors / 2) + num_detectors - 1;
Comment on lines +181 to +182

@KrisThielemans KrisThielemans Nov 12, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).


if (this->get_min_tangential_pos_num() < min_tang_pos_num || this->get_max_tangential_pos_num() > max_tang_pos_num)
{
Expand Down
4 changes: 2 additions & 2 deletions src/buildblock/ProjDataInfoGenericNoArcCorr.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ ProjDataInfoGenericNoArcCorr::initialise_uncompressed_view_tangpos_to_det1det2()
const int num_detectors = get_scanner_ptr()->get_num_detectors_per_ring();
assert(num_detectors % 2 == 0);

const int min_tang_pos_num = -(num_detectors / 2) + 1;
const int max_tang_pos_num = -(num_detectors / 2) + num_detectors;
const int min_tang_pos_num = -(num_detectors / 2);
const int max_tang_pos_num = -(num_detectors / 2) + num_detectors - 1;

if (this->get_min_tangential_pos_num() < min_tang_pos_num || this->get_max_tangential_pos_num() > max_tang_pos_num)
{
Expand Down
4 changes: 3 additions & 1 deletion src/recon_buildblock/ProjMatrixByBinUsingRayTracing.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,11 @@ ProjMatrixByBinUsingRayTracing::set_up(

voxel_size = image_info_ptr->get_voxel_size();
origin = image_info_ptr->get_origin();
if (std::abs(origin.x()) > .05F || std::abs(origin.y()) > .05F)
if (std::abs(origin.x()) > .05F || std::abs(origin.y()) > .05F){
error("ProjMatrixByBinUsingRayTracing sadly doesn't support shifted x/y origin yet");
image_info_sptr->get_regular_range(min_index, max_index);
std::cout << "OriginX : " << origin.x() << " ; OriginY : " << origin.y() << "\n";
}

symmetries_sptr.reset(new DataSymmetriesForBins_PET_CartesianGrid(proj_data_info_sptr,
density_info_sptr_v,
Expand Down