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

Fix for bug in ray tracing projection which produced axial artifacts due to flipped theta angle #1231

Conversation

markus-jehl
Copy link
Contributor

@markus-jehl markus-jehl commented Aug 7, 2023

Fixes #1223

… is being flipped when obtaining phi and s.
Copy link
Collaborator

@KrisThielemans KrisThielemans left a comment

Choose a reason for hiding this comment

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

Do we now test with segment !=0 ? You'd have hoped that this would have shown the original mistake.

src/include/stir/ProjDataInfoGeneric.inl Outdated Show resolved Hide resolved
src/include/stir/LORCoordinates.inl Outdated Show resolved Hide resolved
@markus-jehl
Copy link
Contributor Author

Do we now test with segment !=0 ? You'd have hoped that this would have shown the original mistake.

Ah, I forgot to mention this! I tried to modify @danieldeidda 's test to include non-zero segments and look at non-trivial entries in the proj data. Unfortunately, even though the proj data look very similar, the bin values still vary in too many places to compare the proj data bin by bin (which might be why the test currently only looks at segment 0, axial_pos 0, tangential_pos 0 and then loops through the views). So I'm still unsure how to best test this without simplifying too much.

@KrisThielemans
Copy link
Collaborator

I tried to modify @danieldeidda 's test to include non-zero segments and look at non-trivial entries in the proj data. Unfortunately, even though the proj data look very similar, the bin values still vary in too many places to compare the proj data bin by bin (which might be why the test currently only looks at segment 0, axial_pos 0, tangential_pos 0 and then loops through the views).

sorry, could you elaborate a little bit? Which test was this?

Otherwise, this is good to go, as soon as you add something to the release notes of course!

@markus-jehl
Copy link
Contributor Author

I tried to modify @danieldeidda 's test to include non-zero segments and look at non-trivial entries in the proj data. Unfortunately, even though the proj data look very similar, the bin values still vary in too many places to compare the proj data bin by bin (which might be why the test currently only looks at segment 0, axial_pos 0, tangential_pos 0 and then loops through the views).

sorry, could you elaborate a little bit? Which test was this?

Otherwise, this is good to go, as soon as you add something to the release notes of course!

I mean the run_map_orientation_test in test_blocks_on_cylindrical_projectors.cxx. I have locally modified it to call set_blocks_projdata_info instead of set_direct_projdata_info, and then write out the projdata1 and projdata2 to compare them visually. The obvious flipped vertical lines are gone, but if I extend the bin comparison to use non-trivial positions there are some bins with large differences.

@KrisThielemans
Copy link
Collaborator

if I extend the bin comparison to use non-trivial positions there are some bins with large differences

hmmm. that isn't reassuring!

Anyway, let's merge this then and create an issue for that. squash-merge ok?

@KrisThielemans KrisThielemans merged commit 6f11f06 into UCL:master Aug 17, 2023
5 checks passed
@markus-jehl
Copy link
Contributor Author

if I extend the bin comparison to use non-trivial positions there are some bins with large differences

hmmm. that isn't reassuring!

Anyway, let's merge this then and create an issue for that. squash-merge ok?

Sure! I'll create the issue right away

@markus-jehl markus-jehl deleted the issue/1223-ray-tracing-projector-introduces-axial-artifacts-for-blocks-on-cylindrical-scanners branch January 29, 2024 13:44
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

Successfully merging this pull request may close these issues.

Ray tracing projector introduces axial artifacts for BlocksOnCylindrical scanner geometry
2 participants