[SyneRBI/SIRF] Integer valued sirf.ObjectiveFunction.value (Issue #1170)
Imraj Singh
I am reconstructing with SIRF and have only encountered integer values for sirf.ObjectiveFunction.value.
Is this expected behaviour? I could be doing something wrong, but I am not sure I am... I had a look at how it is exposed in SIRF and it seems that a float type is used, but all my values are in the format "1234567.0" with the values after the decimal non-existent...
Help would be appreciated!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
this is obviously a bit strange, but floats do have limited precision, and there could be some printing stuff going on.
Note that STIR uses doubles for the objective function, so if SIRF uses floats, we'd loose some precision there.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Imraj Singh
I have managed to reproduce this problem in the ML_reconstruction.ipynb notebook.
For the acquired_data in the notebook I multiply by an arbitrary value i.e. "100000"
In the reconstruction I print the objective function value:
As you can see there is a problem with precision it seems. I am a bit worried if the could affect gradients etc?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
As you can see there is a problem with precision it seems.
Do I?
Yes, the increment is small. there's a large (and in a sense arbitrary) offset to the KL. @robbietuk created an issue UCL/STIR#697 on the offset and had a PR but ran out of steam UCL/STIR#760.
I am a bit worried if the could affect gradients etc?
the computation of the gradient is independent of the computation of the value. The latter is much more sensitive to numerical problems. (Adding lots of numbers with limited precision is tricky).
Do check if SIRF uses doubles.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
David Atkinson
If you multiply by 1000 do you get the same effect of everything after the decimal point being zero? In that case we know it is wrong because we have seen there are more significant figures.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
what's the status here please?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Kris Thielemans
@Imraj-Singh this one is a bit urgent. Can you clarify if we need to fix anything?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Imraj Singh
@KrisThielemans I am not sure if the precision loss due to the KL offset will have meaningful ramifications for assessing convergence etc? I did quickly look at the source and it seems that the STIR value is case to float in SIRF: https://github.com/SyneRBI/SIRF/blob/6c7259d1e37f6112610619883c385325b2998c63/src/xSTIR/cSTIR/cstir.cpp#L1152
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Evgueni Ovtchinnikov
Fixed now - see #1264. @Imraj-Singh Please try again.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.