Corrections to CovarianceSampling class and corresponding test

[SergioRAgostinho]

The purpose of this class is to provide a sampling method which

Point Cloud sampling based on the 6D covariances. It selects the points such that the resulting cloud is as stable as possible for being registered (against a copy of itself) with ICP. The algorithm adds points to the resulting cloud incrementally, while trying to keep all the 6 eigenvalues of the covariance matrix as close to each other as possible.
This class also comes with the \a computeConditionNumber method that returns a number which shows how stable a point cloud will be when used as input for ICP (the closer the value it is to 1.0, the better).

Based on the following publication:
"Geometrically Stable Sampling for the ICP Algorithm" - N. Gelfand, L. Ikemoto, S. Rusinkiewicz, M. Levoy

This 6D covariance matrix which takes some operations over the points and the normals into account. From the class description the objective seems to be to ensure that the new sub sampled point cloud should have a(n augmented) covariance matrix with eigen values very close to the original point cloud. That is not what is being tested in the unit tests.

The condition number just provides us with partial information from the maximum and minimum eigen values and we can already see from those that something is indeed changing.

I erased the filtered index checks because they provide no meaningful information on whether things are running well or breaking. They just warn us that something changed.

The class needs to have its unit tests rewritten from scratch but that is for someone who's willing to go through the paper and understand it thoroughly. For this release I prefer to simply relax the existing requirements.

[taketwo]

Nice simplifications!

One thing I don't entirely understand is the epsilons in the condition number tests. How do you derive them? Comments say that they should have the same order of magnitude, but to me this means they should have the same number of digits?

[SergioRAgostinho]

One thing I don't entirely understand is the epsilons in the condition number tests. How do you derive them? Comments say that they should have the same order of magnitude, but to me this means they should have the same number of digits?

I started with a similar order of magnitude requirement. In my opinion the "same digits" argument doesn't really work. A good example is when your most significant digit is 1. Having 0.999 would break the requirement. So I originally went to enforcing the following.

x_ref = 200 // an expected reference
x_new // the new value

0.1 <= x_new / x_original <= 10

In this example you're allowing it to go to

x_new_min = 20
x_new_min = 2000

But this feels too loose and you would never say that 20 is in the same magnitude order of 200.

In the end I just defaulted to "Ok it needs to be roughly there. Let's set it at 10%". The comment then became misleading.

[taketwo]

OK makes sense. Could you update the comment to match the assertions and fix the conflict in .appveyor.yml?

[SergioRAgostinho]

..and also this guy. Although it still needs a final review.