Tencent / rapidjson

A fast JSON parser/generator for C++ with both SAX/DOM style API
http://rapidjson.org/
Other
14.25k stars 3.53k forks source link

Precision of written doubles is slightly off #954

Open lasalvavida opened 7 years ago

lasalvavida commented 7 years ago

From: https://github.com/KhronosGroup/glTF-Sample-Models/issues/63#issuecomment-301497749

If you write the double -36.973846435546875 with rapidjson::Writer, the string that is written is -36.97384643554688.

I'm aware this is a very small error, but there seems to be some information lost when writing the double to a string.

For reference, using std::ostringstream with precision 17 produces -36.973846435546875 as expected.

miloyip commented 7 years ago

I can reproduce this issue. Need some time to fix it.

Llerd commented 7 years ago

Not sure if this is the identical issue, but here's a snippet of something I ran into:

    rapidjson::Document doc1;
    rapidjson::Value val(-0.24f);
    rapidjson::Value valCopy;
    valCopy.CopyFrom(val, doc1.GetAllocator());
    doc1.SetObject();
    doc1.AddMember("val1", valCopy, doc1.GetAllocator());

    // Verify data
    if (doc1["val1"] == val)
        nop(); // Ok

    // Save to string
    rapidjson::StringBuffer buffer;
    rapidjson::PrettyWriter<rapidjson::StringBuffer> writer(buffer);
    doc1.Accept(writer);
    std::string st1 = buffer.GetString();

    // Parse from string
    rapidjson::Document doc2;
    doc2.Parse(st1.c_str());

    // Verify data
    if (doc2["val1"] == val)
        nop(); // Does not trigger
ghost commented 6 years ago

I'm hoping that this issue gets some attention since this problem means that certain double values don't properly roundtrip (double -> string -> double; the final value is different from the first), which is particularly important for my use case.

I was able to partially solve the problem for now by replacing the Grisu2 implementation in this codebase with another (in my case, I used the one from https://github.com/night-shift/fpconv), which suggests a bug in the C++ Grisu2 implementation somewhere. I tested with a large number of randomly generated doubles, and found that many times the output from this Grisu2 implementation is off by 1 ULP.

EDIT: Removed an additional comment about the parsing of double string values, unrelated to this issue. That was my mistake: I thought I had set the kParseFullPrecisionFlag but hadn't.

akovski commented 5 years ago

hi, I was trying to reproduce it, but my rapidjson outputs -36.973846435546878 which is correct. is this bug still valid?

Llerd commented 5 years ago

I haven't tested the original issue, but the code I posted above still behaves incorrectly.

EDIT: never-mind. I added the kParseFullPrecisionFlag as mentioned above, and now it works. Sorry about that. At least my code-snippet works as it should.