Skip to content
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.

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

tangential position range inconsistency #1507

Open
SomayehSaghamanesh opened this issue Sep 10, 2024 · 4 comments
Open

tangential position range inconsistency #1507

SomayehSaghamanesh opened this issue Sep 10, 2024 · 4 comments

Comments

@SomayehSaghamanesh
Copy link

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;

@robbietuk
Copy link
Collaborator

The class inheritance is as follows
image

I suggest using this

void
ProjDataInfo::set_num_tangential_poss(const int num_tang_poss)
{
min_tangential_pos_num = -(num_tang_poss / 2);
max_tangential_pos_num = min_tangential_pos_num + num_tang_poss - 1;
}

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
Copy link
Author

SomayehSaghamanesh commented Sep 11, 2024

Yes, I have already followed it in PR #1508, changed two classes to be consistent with ProjDataInfo.cxx.

@robbietuk
Copy link
Collaborator

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

const int min_tang_pos_num = -(num_detectors / 2) + 1;
const int max_tang_pos_num = -(num_detectors / 2) + num_detectors;
if (this->get_min_tangential_pos_num() < min_tang_pos_num || this->get_max_tangential_pos_num() > max_tang_pos_num)
{
error("The tangential_pos range (%d to %d) for this projection data is too large.\n"
"Maximum supported range is from %d to %d",
this->get_min_tangential_pos_num(),
this->get_max_tangential_pos_num(),
min_tang_pos_num,
max_tang_pos_num);
}
checks are, but since both classes' methods are a copy and paste of one another. Your fix in #1508 seems correct to me.

@SomayehSaghamanesh
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants