Page MenuHomePhabricator

Equal-method for vectors of class mitkVector.h is returning wrong results
Closed, ResolvedPublic

Description

This bug caused T9727. The problem is the equal method of mitkVector.h. There the condition diff.squared_magnitude() < mitk::eps is checked, even for quaternions (or other vectors). This means that if for example the difference of all components of a quaternion is 0.001 the methods returns true because 0.001^2+0.001^2+0.001^2+0.001^2=0.000004 < mitk::eps.

This means the vectors [0.001,0.001,0.001,0.001] and [0,0,0,0] are deemed to be equal at a moment.

Perhaps it's a solution to use sqrt(diff.squared_magnitude()) < mitk::eps. But further discussion is required how to handle equality of vectors.

Event Timeline

This bug caused a discussion about the use of the constant mitk::eps. The following ideas came up during the discussion but there was no final decision how to handle mitk::eps in future.

Ideas:

  • mitk::eps should only handle problems with the machine accuracy (not other problems like algorithm convergence, etc.)
  • there may be a need of different mitk::eps for double and float
  • one can use the epsilon constant offered by vnl and remove mitk::eps
  • the equal methods should be renamed if it uses mitk::eps instead of real equality (to avoid wrong uses by programmers)
  • there should be a special equals method for quaternions instead of using the normal method of vectors (which makes no sense like described above)

Due to its severity, this bug is considered relevant for the upcoming 2012.09 release.

Please check the status and consider fixing this bug for 2012.09.

Solution proposal:

  • component wise comparison (absolute difference)
  • user defined epsilon as method parameter (default 0?)

New remote branch pushed: bug-9987-VectorEqual

(In reply to comment #3)

Solution proposal:

  • component wise comparison (absolute difference)
  • user defined epsilon as method parameter (default 0?)

Your fix is simple and sounds sensible, but please set the default to mitk::eps. Comparing floats with == almost always is not what you want. And IF you want it, you could just write == instead of Equals().

already using mitk::eps as default. i don't think it good, but at least it is the same as before.

Granting release fix since master is already locked due to upcoming release. Peter could you please merge asap.

[47a799]: Merge branch 'bug-9987-VectorEqual'

Merged commits:

2012-11-21 16:25:22 Peter Neher [7701db]
using user defined epsilon and component wise comparison. default is still mitk::eps

New remote branch pushed: bug-9987-VectorEqualComp

[bab36d]: Merge branch 'bug-9987-VectorEqualComp'

Merged commits:

2012-12-05 16:58:28 Peter Neher [428c72]
COMP: adjusted epsilon in tests

[df8dc2]: Merge branch 'bug-9987-VectorEqualComp'

Merged commits:

2012-12-05 17:33:20 Peter Neher [35fd64]
COMP: plane geometry test

[1923d4]: Merge branch 'bug-9987-VectorEqualComp'

Merged commits:

2012-12-05 18:14:00 Peter Neher [5830b5]
COMP: slice navi controller

[ed4b59]: Merge branch 'bug-9987-VectorEqualComp'

Merged commits:

2012-12-05 18:24:10 Peter Neher [cb84ab]
COMP: slice navi controller