rock-core / base-types

C/C++ and Ruby definition of the base types in Rock
6 stars 39 forks source link

Add base::allClose() #95

Closed arneboe closed 5 years ago

arneboe commented 8 years ago

Similar to numpy.allclose(). Fuzzy comparison operator for Eigen::DenseBase.

doudou commented 8 years ago

What do you use it for ? How different would it be to use Eigen's isApprox() in your use case ?

arneboe commented 8 years ago

What do you use it for ?

Planning to use it for performance improvements in envire mars. I need to check whether a transformation has actually changed before updating the whole transformation tree.

How different would it be to use Eigen's isApprox() in your use case ?

Eigen's isApprox() internally uses the Frobenius norm and fails when one of the matrices is very close to zero.

jakobs commented 8 years ago

From the Eigen Doc:

Because of the multiplicativeness of this comparison, one can't use this function to check whether this is approximately equal to the zero matrix or vector. Indeed, isApprox(zero) returns false unless this itself is exactly the zero matrix or vector. If you want to test whether *this is zero, use internal::isMuchSmallerThan(const RealScalar&, RealScalar) instead.

sounds like a thing for @chhtz.

I personally don't like the allClose() name.

chhtz commented 8 years ago

Some thoughts on the implementation:

I don't have any strong opinion on the naming, or the necessity of this function. I can confirm that Eigen's isApprox will not work close to zero (and I don't know an Eigen function which does while still working for arbitrarily large values). I'm not sure if making this a coefficient-wise condition is a good idea, though. E.g., consider the vectors [1e9,1.0, 1e9] [1e9, 1.1, 1e9]; I'd consider them close, given their absolute magnitude, yet the second element has a 10% relative distance (which itself is not close).

chhtz commented 8 years ago

I need to check whether a transformation has actually changed before updating the whole transformation tree.

I'm not sure, if this is a good way to check. Either check if they are exactly equal (which should be save with now-standard SSE math), or add a flag somewhere. Otherwise, even minor changes in one transformation can add up to big differences in a chain of transformations.

doudou commented 6 years ago

Ping

doudou commented 5 years ago

No replies from anyone and no consensus ...