SyneRBI / PETRIC

PET Image Reconstruction Challenge 2024
https://www.ccpsynerbi.ac.uk/events/petric/
5 stars 3 forks source link

accuracy / quantization of (cuda) relative difference prior #100

Open gschramm opened 2 weeks ago

gschramm commented 2 weeks ago

(1) When I evaluate data.prior(x) using sirf.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 of 1/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 of 1/128 and for the last 3 data sets there is a 1/128 difference between the CUDA and non-CUDA version

/mnt/share/petric/Siemens_mMR_NEMA_IQ
data.prior(osem) = 1642433.25
/mnt/share/petric/NeuroLF_Hoffman_Dataset
data.prior(osem) = 179153.6875
/mnt/share/petric/Siemens_Vision600_thorax
data.prior(osem) = 190229.5
/mnt/share/petric/Siemens_mMR_ACR
data.prior(osem) = 169592.765625

/mnt/share/petric/Siemens_mMR_NEMA_IQ
data.prior(ref) = 375538.59375
/mnt/share/petric/NeuroLF_Hoffman_Dataset
data.prior(ref) = 273626.3125
/mnt/share/petric/Siemens_Vision600_thorax
data.prior(ref) = 251760.125
/mnt/share/petric/Siemens_mMR_ACR
data.prior(ref) = 78867.453125
gschramm commented 2 weeks ago

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, so the "kappa weights ~ kappa^2" should be huge compared to the pure "rdp differences" of neighboring pixels.

Maybe normalizing the kappa image by a global factor, that could be compensated for in the penalty weight, could improve numerical accuracy.

KrisThielemans commented 2 weeks ago

This is due to https://github.com/SyneRBI/SIRF/issues/1290. Easily fixable I guess, @evgueni-ovtchinnikov. However, it does mean new docker images and all that.

Is this necessary to fix now?

gschramm commented 1 week ago

I think changing the docker image 27 days before the deadline might cause a lot of confusion. I don't think that the implications are major, given that the evaluation is based on image-based metrics and given that the reference was calculated using a long BSREM (I guess the gradient calculation is not affected).

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.

KrisThielemans commented 1 week ago

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!