-
Notifications
You must be signed in to change notification settings - Fork 2
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
accuracy / quantization of (cuda) relative difference prior #100
Comments
After a 2nd thought, I guess part of the reason could be that the kappa image has a much bigger max than the input images, Maybe normalizing the kappa image by a global factor, that could be compensated for in the penalty weight, could improve numerical accuracy. |
This is due to SyneRBI/SIRF#1290. Easily fixable I guess, @evgueni-ovtchinnikov. However, it does mean new docker images and all that. Is this necessary to fix now? |
I think changing the docker image 27 days before the deadline might cause a lot of confusion. I guess the only concern would be that methods using some kind of line search could be affected (but my feeling is that the impact is probably minor). To be 100% transparent, adding a short notice at the very end of the wiki might be a good idea. |
Gradient calculations are not affected. Only line searches would be. Agreed that bringing it to the attention via the wiki is a good idea. I'll put it in the "updates" page. We could make a special docker if anyone needs it. thanks for spotting this! |
@casperdcl could you confirm that we're running SyneRBI/SIRF-SuperBuild@5f1a7f4 now with updated Docker image? If so, close this issue. |
yes; closing |
(1) When I evaluate
data.prior(x)
usingsirf.STIR.CudaRelativeDifferencePrior
for x in {OSEM, reference solution} for all 4 available data sets I get the results shown below. It seems that all results are multiples of1/64
. When I try to calculate the RDP myself using the numpy arrays, I do not see this effect.I think the issue is minor, but I stumbled upon it when trying to numerically verify the gradient of the RDP using finite differences.
(2) The same observation also holds for
sirf.STIR.RelativeDifferencePrior
(non CUDA version), but this time multiples of1/128
and for the last 3 data sets there is a1/128
difference between the CUDA and non-CUDA versionThe text was updated successfully, but these errors were encountered: