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 bug related to ComputeV for PICCS algorithm #619

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Pooryamn
Copy link
Contributor

Hi @AnderBiguri

I encountered an issue with the new version of the ComputeV function while working with TIGRE and running the PICCS algorithm. The new function works well for ASD_POCS but not for PICCS. To resolve this, I used the body of the old ComputeV function with the new function definition. This modification seems to work for all algorithms, including PICCS.

Could you please review the changes and verify if they are suitable for the repository?

Thank you!

@AnderBiguri
Copy link
Member

AnderBiguri commented Dec 10, 2024

Hi @Pooryamn Thanks for looking at this! indeed computeV requires some fixing.
while this may work numerically, I think its mathematically incorrect, unfortunately. This computes an impact weight of the backprojection (\sum A^T*I), meaning its very important that dVoxel does not change mathematically speaking. As this needs to be used with an image, nVoxel also can't change, ultimately making also sVoxel not allowed to be changed.

Theoretically this "expansion" of the projections is not needed, but I found out numerically that sometimes it causes edge cases in edge voxels ti have very small values, wich causes issues with numerical division, which is why I expand it. It still introduces many errors at certain geometries (but not others). I wonder if expanding it further may fix them.

But, in any case, we can't change the volume if we want to be mathematically accurate.

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.

2 participants