Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eigen Dot product of normalized Vector3fs returns numbers outside -1 .. 1. Using std::acos on this will generate NaN values. #1033

Closed
mschoeler opened this issue Dec 5, 2014 · 7 comments

Comments

@mschoeler
Copy link
Contributor

From pcl/segmentation/include/pcl/segmentation/impl/lccp_segmentation.hpp Lines 434++

const Eigen::Vector3f source_normal = sv_source->normal_.getNormalVector3fMap (). normalized ();
const Eigen::Vector3f target_normal = sv_target->normal_.getNormalVector3fMap (). normalized ();
float dot_normal = source_normal.dot (target_normal);
float normal_angle = std::acos (dot_normal) * 180. / M_PI;

This leads in some cases to dot_normal > 1 or dot_normal < 1 and to normal_angle being NaN.
This again causes LCCP to treat some connections on perfectly flat surfaces to be concave.
I implemented a quick fix:

if (dot_normal > 1)
{
  dot_normal = 1;
}
if (dot_normal < -1)
{
  dot_normal = -1;
}

but this is not nice, obviously. Does anybody know the cause why Eigen's dot product returns numbers not correctly normalized and how to prevent it?

@VictorLamoine
Copy link
Contributor

Did you check the content of sv_source->normal_ and sv_target->normal_ ? Maybe it holds NaN values.

@mschoeler
Copy link
Contributor Author

no it does not. Here is an example output I just generated.

source_normal = -0.3420201, 0.9396927, 0.0000000
target_normal = -0.3420201, 0.9396927, 0.0000000
dot_normal = 1.0000001
normal_angle = NaN

@VictorLamoine
Copy link
Contributor

source_normal and target_normal are normalized but are stored in floats.
Your computations are correct but the rounding errors makes dot_normal sometimes a little bit bigger than 1 (or smaller than -1).

Switching to doubles should help but I don't think the problem will disappear.
Why not round the number ?! (let's say to 4 numbers after the comma for example)

@taketwo
Copy link
Member

taketwo commented Dec 5, 2014

I think that the reason Victor gave is correct. And I think there is no better way than to do:

dot_normal = dot_normal > 1.0 ? 1.0 : (dot_normal < -1.0 ? -1.0 : dot_normal);

(Or the same way as you did with ifs, though note that you can insert else in the middle and save yourself a comparison from time to time.)

@mschoeler
Copy link
Contributor Author

I also think it is an rounding error. Still I find it strange, that Eigen has no build in protection against this (I also could not find an issue regarding this). Using the acos after a dot product of normalized vectors is a very common operation and yielding 0 or NaN is a very big difference. Maybe it makes sense to include a function which calculates the angle betwee two vectors in PCL? Until we find a better solution I will prepare this fix.
Thanks again!

@taketwo
Copy link
Member

taketwo commented Dec 5, 2014

Maybe it makes sense to include a function which calculates the angle between two vectors in PCL?

It would totally make sense. (But maybe there exists one somewhere and we are just not aware of it :))

Still I find it strange, that Eigen has no build in protection against this

I see no way how such a protection could be implemented. Dot product routine does not know whether it multiplies normalized vectors or not. Normalization routine has no idea whether its result will be used further for dot product.

@mschoeler
Copy link
Contributor Author

You could for example round the dot product to one digit less than the precision of float. That way you probably could adapt Victor's idea and get exactly 1 instead of 1+. It would not hurt the angle calculation too much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants