Conformance report for reference

[miloyip]

https://github.com/miloyip/nativejson-benchmark/blob/master/sample/conformance_Nlohmann%20(C%2B%2B11).md

[nlohmann]

Thanks a lot! I'll have a look!

[miloyip]

Actually, this library did very well.
Roundtrip test for real numbers is not a must.

[nlohmann]

I know. We spent some time in the past to improve the results - but in the end this meant storing whether or not a + was given in the exponent... Thanks for your continuing efforts on the benchmark!

[gregmarr]

@miloyip This benchmark states that it is testing conformance to the JSON standard, and then tests things that are not part of the standard. It should be made clear that the roundtrip test is not in fact testing conformance to the standard, but contains arbitrary tests where "pass" vs "fail" has no relation to standard conformance, so as not to mislead users. Probably the easiest way would be to move the roundtrip tests out of the conformance section to a separate section.

[nlohmann]

As summary: The failing tests are roundtrip24-roundtrip27. These tests have been discussed in miloyip/nativejson-benchmark#33 and #187. @twelsby wrote a great summary on this topic.

I shall add the conformance report to the test reports for reference.

I am not sure how to cope with this topic in the future. #302 shows there is still work to do on the parser side. However, achieving 100% "conformance" on this arbitrary benchmark set should not be the main goal for the library.

What do you think?

[gregmarr]

@miloyip pointed out in # 33 that using 17 digits of output rather than 15 would fix at least a couple of the issues. I didn't see that message before.

[nlohmann]

We currently use std::numeric_limits<long double>::digits10 as precision. When set to 17, a lot of test cases fail:

-------------------------------------------------------------------------------
object inspection
  serialization
  dump and floating-point numbers
-------------------------------------------------------------------------------
src/unit-inspection.cpp:34
...............................................................................

src/unit-inspection.cpp:219: FAILED:
  CHECK( s.find("42.23") != std::string::npos )
with expansion:
  18446744073709551615 (0xffffffffffffffff)
  !=
  18446744073709551615 (0xffffffffffffffff)

-------------------------------------------------------------------------------
object inspection
  serialization
  dump and small floating-point numbers
-------------------------------------------------------------------------------
src/unit-inspection.cpp:34
...............................................................................

src/unit-inspection.cpp:225: FAILED:
  CHECK( s.find("1.23456e-78") != std::string::npos )
with expansion:
  18446744073709551615 (0xffffffffffffffff)
  !=
  18446744073709551615 (0xffffffffffffffff)

-------------------------------------------------------------------------------
regression tests
  issue #228 - double values are serialized with commas as decimal points
-------------------------------------------------------------------------------
src/unit-regression.cpp:36
...............................................................................

src/unit-regression.cpp:380: FAILED:
  CHECK( j1a.dump() == "23.42" )
with expansion:
  "23.420000000000002" == "23.42"

src/unit-regression.cpp:381: FAILED:
  CHECK( j1b.dump() == "23.42" )
with expansion:
  "23.420000000000002" == "23.42"

src/unit-regression.cpp:393: FAILED:
  CHECK( j2a.dump() == "23.42" )
with expansion:
  "23.420000000000002" == "23.42"

-------------------------------------------------------------------------------
compliance tests from nativejson-benchmark
  roundtrip
-------------------------------------------------------------------------------
src/unit-testsuites.cpp:103
...............................................................................

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[1.2344999999999999]" == "[1.2345]"
with message:
  filename := test/data/json_roundtrip/roundtrip22.json

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[-1.2344999999999999]" == "[-1.2345]"
with message:
  filename := test/data/json_roundtrip/roundtrip23.json

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[4.9406564584124654e-324]" == "[5e-324]"
with message:
  filename := test/data/json_roundtrip/roundtrip24.json

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[2.2250738585072009e-308]"
  ==
  "[2.225073858507201e-308]"
with message:
  filename := test/data/json_roundtrip/roundtrip25.json

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[1.7976931348623157e+308]"
  ==
  "[1.7976931348623157e308]"
with message:
  filename := test/data/json_roundtrip/roundtrip27.json

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[4.9406564584124654e-324]"
  ==
  "[4.940656458412e-324]"
with message:
  filename := test/data/json_roundtrip/roundtrip28.json

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[2.2250738585071999e-308]"
  ==
  "[2.2250738585072e-308]"
with message:
  filename := test/data/json_roundtrip/roundtrip29.json

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[1.2345e-30]" == "[1.2345E-30]"
with message:
  filename := test/data/json_roundtrip/roundtrip30.json

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[1.2345e+30]" == "[1.2345E+30]"
with message:
  filename := test/data/json_roundtrip/roundtrip31.json

===============================================================================
test cases:   33 |   30 passed |  3 failed
assertions: 8648 | 8634 passed | 14 failed

While we can ignore roundtrip30 and roundtrip31 (I just added them to have the code run on all existing roundtrip files), the other errors

[gregmarr]

Looks like we should be using std::numeric_limits<long double>::max_digits10 instead.

The value of std::numeric_limits::max_digits10 is the number of base-10 digits that are necessary to uniquely represent all distinct values of the type T, such as necessary for serialization/deserialization to text. This constant is meaningful for all floating-point types.

http://en.cppreference.com/w/cpp/types/numeric_limits/max_digits10

As this is 17 for double, we'd then get and need to look at the rest of those failures above.
I'll bet most of them are were written to expect the slightly truncated result, and just need to be expanded.

[nlohmann]

I think it's not so easy, because now the following (previously succeeding) roundtrip tests are failing:

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[1.2344999999999999]" == "[1.2345]"
with message:
  filename := test/data/json_roundtrip/roundtrip22.json

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[-1.2344999999999999]" == "[-1.2345]"
with message:
  filename := test/data/json_roundtrip/roundtrip23.json

src/unit-testsuites.cpp:309: FAILED:
  CHECK( j.dump() == json_string )
with expansion:
  "[2.2250738585071999e-308]"
  ==
  "[2.2250738585072e-308]"
with message:
  filename := test/data/json_roundtrip/roundtrip29.json

[gregmarr]

Yeah, I didn't say it would be easy. :)
I think the issue is that those files are using numbers which can't be represented exactly in double, and so they're subject to the precision with which we're exporting.

[gregmarr]

1.2345 is between 1.23449999999999993072208326339 and 1.23450000000000015276668818842, and is closer to the former, so that's the more accurate representation. It rounds down because the 3 after the nines is the dropped character.
http://www.binaryconvert.com/result_double.html?hexadecimal=3FF3C083126E978D
http://www.binaryconvert.com/result_double.html?hexadecimal=3FF3C083126E978E

For the other, it's between 2.2250738585072000752518306399 and 2.22507385850719963116262078984.

http://www.binaryconvert.com/result_double.html?hexadecimal=4001CCF385EBC89C
http://www.binaryconvert.com/result_double.html?hexadecimal=4001CCF385EBC89D

[miloyip]

@gregmarr Actually Roundtrip tests are needed to test the correctness of stringify function. The other tests can only test the parsing function. The problem is that, there are no single "correct" way of stringifying some values, especially the numerical format. I will try to move some more controversial ones as additional/optional checks, and not to be counted in the overall score.

[gregmarr]

@miloyip Yes, some form of stringify testing is needed. As long as you can exclude the optional parts of it, and make the test so that it only tests the parts that are absolutely necessary, then that's true. The e vs E and presence or absence of + are the most notable parts. I previously thought that some of the tests were being too precise, but thanks to your explanation that I missed before, I see that there were some tests that were in fact valid failures.