New remote branch pushed: bug-15037-PlaneGeometry

I figured out that the general computation is "correct" but not accurate:

Testing mapping Map(pt3d_mm, pt2d_mm) and compare with expected:
Expected pt2d_mm [43.478260040283, 80.000000000000]
Result testpt2d_mm [43.478260040283, 79.999946594238][FAILED]

For comparing these two points we use 2*mitk::eps (0.000023841858). Unfortunately, the test fails just on the continuous windows dartclient. To account for the inaccurate computation I will temporarily adapt the eps value for only this testcase to 10*mitk::eps, in order to make the dashboard green.

Todo:
-Figure out why our PlaneGeometry has such a high inaccuracy in some cases. Someone has to check if we do unnecessary casts from double to float or float to double. We should be able to lower this eps threshold.

@Basti: Could you, as a geometry expert, please have a look at this or pass the bug to someone else who can? We really need to figure out if this problem is only caused by ITK or if it is us. Feel free to ask Stefan or me for details.

Summary:

In T9987 Peter discovered, that we have high inaccuracy in our PlaneGeometry. He fixed this by increasing the threshold (mitk::eps) for each respective test (e.g. 3*mitk::eps).

Since the change to ITK 4, this effect reoccurred in the PlaneGeometryTest for unknown reasons.

I started looking for the bug and rewriting the confusing test. I think the test is correct, but it is too huge and confusing. It should be split to multiple tests. I pushed my first changes to: personal/kilgus/PlaneGeoDebugging. This branch contains a little more organized test including output of the wrong values with high precision.

I figured out that the member m_ScaleFactorMMPerUnitY is already a bit inaccurate. m_ScaleFactorMMPerUnit is initialized via:

mitk::Geometry2D::SetIndexToWorldTransform(

mitk::AffineTransform3D* transform)

{
[...]

m_ScaleFactorMMPerUnitY=GetExtentInMM(1)/GetExtent(1);

[...]

GetExtentInMM(1) returns also an inaccurate value. Inside we use a vnl float method:

m_IndexToWorldTransform->GetMatrix().GetVnlMatrix().get_column(direction).magnitude()

and multiply that value with a double method:
GetExtent(direction);

I assume a cast happens here which results in high inaccuracy (4-5th decimal place) but further testing is necessary.

The main problem is that the default floating data type is float instead double.
The precision of a float can be taken as 6-7 digits. If a point has a high value (like for example 100) the accuracy which is to be expected will be around 0.0001. This is clearly over mitk::eps which is only useful for values up to 10.

This is one reason why the test fails.

A second reason is the summing of the rounding errors.
For example: At a position of the test, the value of m_ScaleFactorMMPerUnitY should be set to 1/3. The value that is stored inside is
0.333333373070
Representing 1/3 as float will give us:
0.333333343267
So the error made at this point is relative small and is expected due to the multiple operations that have to be carried out to calculate m_ScaleFactorMMPerUnitY. But in a second step m_ScaleFactorMMPerUnitY is used to calculate the transformation of a Index point to a world point. Basically this is a small operation of
x_index = x_world / m_ScaleFactorMMPerUnitX
But the error propagation for division is especially bad. Converting the index 100 in world will give us
299,999999910
299,999964237
(Expecting 300) depending if the value given by mitk or the float representation of 1/3 is used.

What is the status of this bug? Please change the target milestone to "AfterNextRelease" if this bug is not relevant or cannot be fixed in time for the 2013-06 release.

This bug has a high severity and was not fixed within the 2013-06 release. Setting target milestone to next release.

Current release is finished. Reseting target milestone...

It seems that this bug is fixed due to several changes in the last time. Although the eps is still times 10, it is around 10^-13, so pretty small.

It might even be possible to reduce the used eps but since it's already small i leave it as it is.