Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

fixed nan interpolation issue#311

Closed
bluenote10 wants to merge 3 commits intoros:melodic-develfrom
bluenote10:fix_nan_interpolation
Closed

fixed nan interpolation issue#311
bluenote10 wants to merge 3 commits intoros:melodic-develfrom
bluenote10:fix_nan_interpolation

Conversation

@bluenote10
Copy link

fixes #310

Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working around the symptom by testing it in the same way. It would be much better to not do double comparisons of large values.

If you instead changed the ratio to diff the times into a Duration, that would all be integer math. And then just call Duration::toSec on the results the problem would be avoided completely.

And you also aren't doing floating point comparisions or extra conversions to floating point.

@bluenote10
Copy link
Author

How would you do that exactly? Duration does not have division and using Duration::toSec on the numerator and denominator separately probably doesn't fully solve the problem, because the denominator again can become zero. I could change the check to be (two.stamp_ - one.stamp_).toSec() == 0 accordingly?

@tfoote
Copy link
Member

tfoote commented Jul 12, 2018

A floating point representation won't have a problem representing a single nanosecond and will not "become" zero. It only looses precision with large values so if you do the differencing in fixed point math. It's safe to do the division using the floating point representation of the deltas (which are small).

https://bitbashing.io/comparing-floats.html

{
// Check for zero distance case
if( two.stamp_ == one.stamp_ )
if ((two.stamp_ - one.stamp_).toSec() == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not necessary and causes noteable extra computation.

@tfoote
Copy link
Member

tfoote commented Jul 13, 2018

Thanks for the fixup, with the revert there's now a whole bunch of unrelated whitespace changes. Generally they're good, but having the unrelated changes in the diffs will make tracking across branches much harder. Please keep the diff minimal to just the code changes.

@bluenote10
Copy link
Author

Closing in favor of #313

@bluenote10 bluenote10 closed this Jul 16, 2018
seanyen pushed a commit to ms-iot/geometry2 that referenced this pull request Mar 22, 2021
* Port eigen_kdl.h/cpp to ROS2

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Occasional NaN transforms

2 participants