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

Floating-point formatting #915

Merged
merged 10 commits into from
Jan 21, 2018
Merged

Floating-point formatting #915

merged 10 commits into from
Jan 21, 2018

Conversation

abolz
Copy link
Contributor

@abolz abolz commented Jan 15, 2018

Try to fix #360. printf doesn't have a "to shortest" mode and std::to_chars is not available yet. Use the Grisu2 algorithm for floating-point formatting which is a nice compromise between using printf with a precision of digits10 and max_digits10, i.e. the resulting decimal representation is guaranteed to round-trip and is almost always the shortest such representation.

…loating-point conversion

This is an attempt to fix #360. The algorithm produces
decimal representations which are guaranteed to roundtrip
and in ~99.8% actually produces the shortest possible
representation. So this is a nice compromise between using
a precision of digits10 and max_digits10.

Note 1:

The implementation only works for IEEE single/double precision
numbers. So the old implementation is kept for compatibility
with non-IEEE implementations and 'long double'.

Note 2:

If number_float_t is 'float', not all serialized numbers can
be recovered using strtod (strtof works, though). (There is
exactly one such number and the result is off by 1 ulp.)
This can be avoided by changing the implementation (the fix
is trivial), but then the resulting decimal numbers are not
exactly short.
@abolz abolz changed the title Dtoa Floating-point formatting Jan 15, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 9b9919d on abolz:dtoa into d9446b0 on nlohmann:develop.

@nlohmann nlohmann self-assigned this Jan 21, 2018
@nlohmann nlohmann added this to the Release 3.1.0 milestone Jan 21, 2018
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

@abolz I have some questions regarding the PR.

o->write_characters(begin, static_cast<size_t>(end - begin));
}

void dump_float(number_float_t x, std::false_type /*is_ieee_single_or_double*/)
Copy link
Owner

Choose a reason for hiding this comment

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

Just out of curiosity: why do we still need the snprintf approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the implementation only works for IEEE single and double precision.
It should work in case long double is an alias for double, though.

@@ -7079,14 +8108,36 @@ class serializer
void dump_float(number_float_t x)
{
// NaN / inf
if (not std::isfinite(x) or std::isnan(x))
Copy link
Owner

Choose a reason for hiding this comment

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

Why can NaN be ignored here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its actually not ignored: the isnan test is just redundant. isfinite return false for NaN

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@nlohmann nlohmann merged commit 010e596 into nlohmann:develop Jan 21, 2018
@nlohmann
Copy link
Owner

Thanks a lot!

nlohmann added a commit that referenced this pull request Jan 21, 2018
@abolz abolz deleted the dtoa branch January 22, 2018 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loss of precision when serializing <double>
3 participants