Skip to content

Conversation

jagerman
Copy link
Contributor

@jagerman jagerman commented Apr 24, 2016

boost::compute::detail::make_literal is losing some precision when making literal floating point values because it uses std::numeric_limits::digits10, but that only gives us the number of decimal digits that will survive a decimal->native>decimal conversion; what we are doing is a native->decimal->native conversion, which can require up to ::max_digits10 (which is larger than ::digits10 by 2 (double) or 3 (float)).

However, max_digits10 is a c++11 feature, so this commit uses digits10 + 3 when the c++11 numeric_limits isn't available. That should be enough for an IEEE-style float, though is slightly more than necessary for some (e.g. double).

I also added a test for this (which fails with the current conversion for both float and double, and passes with the proposed commits).

make_literal is losing some precision when making literal floating
point values because it uses digits10, but that only gives us the
number of decimal digits that will survive a decimal->native>decimal
conversion; what we are doing is a native->decimal->native conversion,
which requires the user of ::max_digits10 (which is 2 (double) or 3
(float) larger than ::digits10).

max_digits10 is a c++11 feature, however, so this commit uses
digits10 + 3 when the c++11 numeric_limits isn't available.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 88.82% when pulling 7f18293 on jagerman:increase-literal-precision into 48217d2 on boostorg:develop.

@kylelutz kylelutz merged commit 2b346db into boostorg:develop Apr 25, 2016
@kylelutz
Copy link
Collaborator

Thanks for the fix!

@jszuppe
Copy link
Contributor

jszuppe commented Apr 30, 2016

I've just noticed that on OSX it gives errors (link):

/Users/travis/build/boostorg/compute/test/test_literal_conversion.cpp:29: error: in "literal_conversion_float": check iss >> x has failed
/Users/travis/build/boostorg/compute/test/test_literal_conversion.cpp:30: error: in "literal_conversion_float": check char(iss.get()) == 'f' has failed [0xffffffff != 'f']
/Users/travis/build/boostorg/compute/test/test_literal_conversion.cpp:29: error: in "literal_conversion_float": check iss >> x has failed
/Users/travis/build/boostorg/compute/test/test_literal_conversion.cpp:30: error: in "literal_conversion_float": check char(iss.get()) == 'f' has failed [0xffffffff != 'f']
/Users/travis/build/boostorg/compute/test/test_literal_conversion.cpp:29: error: in "literal_conversion_float": check iss >> x has failed
/Users/travis/build/boostorg/compute/test/test_literal_conversion.cpp:30: error: in "literal_conversion_float": check char(iss.get()) == 'f' has failed [0xffffffff != 'f']
/Users/travis/build/boostorg/compute/test/test_literal_conversion.cpp:37: error: in "literal_conversion_float": check values[0] == roundtrip[0] has failed [1.23456788 != 0]
/Users/travis/build/boostorg/compute/test/test_literal_conversion.cpp:38: error: in "literal_conversion_float": check values[1] == roundtrip[1] has failed [1.234568 != 0]
/Users/travis/build/boostorg/compute/test/test_literal_conversion.cpp:39: error: in "literal_conversion_float": check values[2] == roundtrip[2] has failed [1.23456812 != 0]

@jagerman
Copy link
Contributor Author

jagerman commented May 2, 2016

I'm at a loss as why that is happening; my best guess is some OSX/clang++ bug w.r.t. std::istringstream operators and the stream status (I found at least one reference suggesting that, for some versions, peek() doesn't set eof as expected).

I've rewritten the test code in PR #607 -- I don't know whether that will fix it (I don't have an easily accessible OS X machine), but if it fails, it should at least help figure out which part of the conversion is failing.

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

Successfully merging this pull request may close these issues.

4 participants