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

Comparison of double and floating point in collision grid meta data #243

Closed
jolting opened this issue May 2, 2016 · 3 comments
Closed

Comments

@jolting
Copy link
Member

jolting commented May 2, 2016

Is it safe to add some epsilon in here, so that collision grids don't regenerate if they are close enough?

https://github.com/MRPT/mrpt/blob/master/libs/nav/src/tpspace/CParameterizedTrajectoryGenerator.cpp#L625

@jolting
Copy link
Member Author

jolting commented May 3, 2016

@jlblancoc Actually this bug is more severe than I originally thought.
Collision grids will always regenerate.

This commit broke it:
e5db1da

This writes doubles to the file.

        *f << m_x_min << m_x_max << m_y_min << m_y_max;
        *f << m_resolution;

And this reads floats.

        *f >> ff; if(ff!=m_x_min) return false;
        *f >> ff; if(ff!=m_x_max) return false;
        *f >> ff; if(ff!=m_y_min) return false;
        *f >> ff; if(ff!=m_y_max) return false;
        *f >> ff; if(ff!=m_resolution) return false;

It doesn't appear that MRPT does any type checking when deserializing.

@jlblancoc
Copy link
Member

jlblancoc commented May 3, 2016

Thanks for the discovery @jolting !!

I'll verify it and correct it ASAP.
Type-checking if only done at the object (CSerializable and derived classes) level. POD types are not checked to avoid the huge overload that it would represent...

@jlblancoc
Copy link
Member

jlblancoc commented May 3, 2016

@jolting please, get the changes from that commit above.

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

2 participants