ros / roscpp_core

ros distribution sandbox
88 stars 116 forks source link

ros/time.h fromSec implementation suffers from floating point inaccuracy, and stores invalid data #48

Closed televator closed 8 years ago

televator commented 8 years ago

In some corner cases, floating point inaccuracy can lead to invalid data where nsec is calculated to be outside [0;1e9-1]. To illustrate the problem think of the following

#include <cmath>
#include <boost/math/special_functions/round.hpp>
int main()
{
        double someint = 1031.0; // some integer
        double t = std::nextafter(someint, 0); // someint - epsilon
        std::cout << next << std::endl;    // prints 1031.000000
        // ros implementation of fromSec
        uint32_t sec = (uint32_t)floor(t);
        uint32_t nsec = (uint32_t)boost::math::round((t-sec) * 1e9);
        std::cout << "sec"  << sec  << std::endl;   // prints 1030 (!)
        std::cout << "nsec"<< nsec << std::endl;  // prints 1000000000 (!)
}

(compile with c++11) Correct behavior would be sec: 1031 nsec: 0, of course.

I guess the rounding behavior is desired in general, so a remaining fix would be to catch this specific case. Alternatively, the order could be changed to multiply with 1e9 -> round -> split into secs/nsec. The latter solution requires an appropriately long integer data type for the temporary.

dirk-thomas commented 8 years ago

Thank you for reporting this problem. Can you please contribute a pull request with your proposed fix checking if nsec is 1000000000 and then normalizes the time again (sec++, nsec=0). Thanks.