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

json: insufficient precision for float64 serialization #15397

Closed
slonik-az opened this issue Sep 23, 2020 · 5 comments
Closed

json: insufficient precision for float64 serialization #15397

slonik-az opened this issue Sep 23, 2020 · 5 comments

Comments

@slonik-az
Copy link

Nim's standard json module serializes float numbers with 16 significant decimal digits

var n: int = c_sprintf(addr buf, "%.16g", value)

which is insufficient to represent all of 64 bit IEEE-754 floats. One needs at least 17 significant decimal digits, see https://en.wikipedia.org/wiki/Double-precision_floating-point_format for details.

The following MWE shows the problem and a possible solution that changes precision from 16 to 17 digits.

MWE

import json
import fenv
import strformat

let
    x1eps   = 1.0+epsilon(float64) # smallest float64 larger than 1
    jsnStr1 = $(%*{"fnum": x1eps }) # json uses 16 significant digits, not enough!
    jsnStr2 = fmt("""{{ "fnum": {x1eps:.17g} }}""") # need at least 17 significant digits
    jsnNd1  = parseJson(jsnStr1)
    jsnNd2  = parseJson(jsnStr2)
    x1      = jsnNd1{"fnum"}.getFloat(NaN)
    x2      = jsnNd2{"fnum"}.getFloat(NaN)

echo "jsbStr1 = ", jsnStr1
echo "jsbStr2 = ", jsnStr2
echo "jsnNd1  = ", jsnNd1
echo "jsnNd2  = ", jsnNd2
echo "x1      = ", x1
echo "x2      = ", x2

# Results:
doAssert (x1 != x1eps)
doAssert (x1 == 1.0)
doAssert (x2 == x1eps)

Additional Information

$ nim -v
Nim Compiler Version 1.2.6 [MacOSX: amd64]
Compiled at 2020-09-02
Copyright (c) 2006-2020 by Andreas Rumpf
git hash: 211868b85cf8b11baabf06d75aaf7e2015d82e2b
active boot switches: -d:release
@timotheecour
Copy link
Member

timotheecour commented Sep 23, 2020

See related discussion discussing riu ryu as a better approach

@bluenote10
Copy link
Contributor

see e.g. #13276 #13196 #13363

@slonik-az
Copy link
Author

See related discussion discussing riu as a better approach

@timotheecour : By 'riu' did you mean 'ryu' as in https://github.com/ulfjack/ryu ?

@timotheecour
Copy link
Member

yes, ryu is tracked here: #13365

can we close this issue as duplicate of #13196 ?

@slonik-az
Copy link
Author

Closing as duplicate of #13196 with solution tracked in #13365

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 a pull request may close this issue.

3 participants