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

Boost.JSON test has been added and the old test has been renamed. #277

Merged
merged 1 commit into from
Oct 25, 2020

Conversation

nuald
Copy link
Collaborator

@nuald nuald commented Oct 25, 2020

No description provided.

@nuald nuald merged commit 282b27c into kostya:master Oct 25, 2020
@nuald nuald deleted the boost branch October 25, 2020 04:13
using namespace std;

struct coordinate_t {
float x;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't these floats be doubles? I think Boost.JSON will store them as double internally anyways.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but looks like it's some precision problem with Boost.JSON - when I used double, it's read 1.1 as 1.1000...00001, so auto-comparison 1.1 == 1.1 failed (I know that it's not quite proper way to compare rational numbers, but I didn't have that problem before). It could be some intermittent problem as I've used HEAD instead of stable release, and using floats fixed that, so I let it slip.

Copy link
Contributor

@beached beached Oct 25, 2020

Choose a reason for hiding this comment

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

both those values are correct, I think. I wonder if a test like

bool are_close( double lhs, double rhs ) {
  std::uint64_t l;
  memcpy( &l, &lhs, sizeof( std::uint64_t ) );
  std::uint64_t r;
   memcpy( &r, &rhs, sizeof( std::uint64_t ) );
  return std::abs( l - r ) < 3;
}

Or something like that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hesitate to change the testing code, because literally all other code including C++-based (thus using the same doubles internally) don't have this problem. There could be some actual issue in Boost.JSON, but as it's not a stable release, I don't want to do make any conclusions yet. I think I'll test it again when they have the stable release, and if the issue still occurs, I'll dig the actual reason and contact them.

Copy link
Contributor

@beached beached Oct 25, 2020

Choose a reason for hiding this comment

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

I just tested and it's encoding 3.3 as 3.2999999999999998224, others are as 3.3000000000000002665. Both are correct. The test should probably be fixed as 3.3 cannot be encoded as exactly IEEE floating point and they are both on either side of it. The two encodings are 1 ulp apart. std::nextafter might help in the check or if it's just to see if the values are parsing in the correct order, use integers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately, I can't use almostEqual-like comparison utilizing ulps/nextafter as it's not quite portable to other languages (not all of them have rich math libraries). However, I've verified numbers with base 2 (2.0, 0.5, 0.25), and it works fine in tests, I'll update the Boost.JSON code to use doubles, and update other tests to use those numbers too.

Copy link
Contributor

Choose a reason for hiding this comment

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

That seems better. Simple and, assuming they are IEEE754, it should always be exact.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in #279, verified on all tests, all work correctly, and changed Boost.JSON test to use doubles.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that I just tested and it's encoding 3.3 as 3.2999999999999998224, others are as 3.3000000000000002665.

I would expect parsers to always return 3.2999999999999998224 since it is the nearest floating point value. You can verify that it at 1.77 x 10^-16 of 3.3. Meanwhile, the other value you propose is at a distance 2.66 x 10^-16. I would certainly expect your compiler to treat 3.3 as 3.2999999999999998224.

nuald added a commit to nuald/benchmarks that referenced this pull request Oct 31, 2020
nuald added a commit that referenced this pull request Dec 3, 2020
* Re-run bf mandel tests (addressing #258).

* Maintenance update (#261)

* Re-run bf mandel tests (addressing #258).
* Preparation for Perl 7.
* Lua module conflicts have been resolved.

* Update README.md

Fixed UPDATE date.

* Maintenance update (#265)

* Added workaround for Intel JCC bug (#263).

* Maintenance update.

* Added JCC bugfix flag to clang. (#266)

* Vala numbers have been updated. (#268)

* Update test_dawjsonlink.cpp (#271)

Updated code to reflect API changes in v2 of DAW JSON Link

* Maintenance update with the renamed tests (#269). (#270)

* Switch the simdjson code to On Demand (#272)

* Updating simdjson to 0.6 API (on demand).

* Making things safer by specifying the commit + removing unnecessary std:: qualifiers.

* Minor changes following review

* Maintenance update and simdjson test changes (#273)

* Maintenance update

* Maintenance update and simdjson test changes.

* The JSON key ordering tests have been added. (#275)

* Boost.JSON test has been added and the old test has been renamed. (#277)

* Updated to use better numbers for floating point ops, changed Boost.JSON test to use doubles. (#279)

* Improved precisions of the tests (#281)

* Median cals have been added.

* Try another formatting.

* Changed memory calculations.

* Printing has been moved out from JSON benchmarks.

* Up-to-dated results.

* TOC updated.

* Typo

* Added printer to Scheme langs.

* Racket code has been fixed to use idiomatic struct instead of macros.

* C++ and Kotlin tests have been updated.

* D tests have been fixed to use proper initializations.

* Go and Elixir tests have improved.

* Nim microoptimization have been removed.

* Julia test has been updated.

* All haskell tests have been updated.

* Lua tests have been improved.

* OCaml tests have been improved.

* SML tests have been improved.

* Fsharp test has been improved.

* Tcl tests have been improved.

* Perl tests have been updated to use v5.32.

* Other tests have been fixed.

* Haskell networking has been fixed.

* Fixed JS code.

Co-authored-by: Darrell Wright <beached@users.noreply.github.com>
Co-authored-by: Daniel Lemire <lemire@gmail.com>
nuald added a commit to nuald/benchmarks that referenced this pull request Dec 4, 2020
* Re-run bf mandel tests (addressing kostya#258).

* Maintenance update (kostya#261)

* Re-run bf mandel tests (addressing kostya#258).
* Preparation for Perl 7.
* Lua module conflicts have been resolved.

* Update README.md

Fixed UPDATE date.

* Maintenance update (kostya#265)

* Added workaround for Intel JCC bug (kostya#263).

* Maintenance update.

* Added JCC bugfix flag to clang. (kostya#266)

* Vala numbers have been updated. (kostya#268)

* Update test_dawjsonlink.cpp (kostya#271)

Updated code to reflect API changes in v2 of DAW JSON Link

* Maintenance update with the renamed tests (kostya#269). (kostya#270)

* Switch the simdjson code to On Demand (kostya#272)

* Updating simdjson to 0.6 API (on demand).

* Making things safer by specifying the commit + removing unnecessary std:: qualifiers.

* Minor changes following review

* Maintenance update and simdjson test changes (kostya#273)

* Maintenance update

* Maintenance update and simdjson test changes.

* The JSON key ordering tests have been added. (kostya#275)

* Boost.JSON test has been added and the old test has been renamed. (kostya#277)

* Updated to use better numbers for floating point ops, changed Boost.JSON test to use doubles. (kostya#279)

* Improved precisions of the tests (kostya#281)

* Median cals have been added.

* Try another formatting.

* Changed memory calculations.

* Printing has been moved out from JSON benchmarks.

* Up-to-dated results.

* TOC updated.

* Typo

* Added printer to Scheme langs.

* Racket code has been fixed to use idiomatic struct instead of macros.

* C++ and Kotlin tests have been updated.

* D tests have been fixed to use proper initializations.

* Go and Elixir tests have improved.

* Nim microoptimization have been removed.

* Julia test has been updated.

* All haskell tests have been updated.

* Lua tests have been improved.

* OCaml tests have been improved.

* SML tests have been improved.

* Fsharp test has been improved.

* Tcl tests have been improved.

* Perl tests have been updated to use v5.32.

* Other tests have been fixed.

* Haskell networking has been fixed.

* Fixed JS code.

Co-authored-by: Darrell Wright <beached@users.noreply.github.com>
Co-authored-by: Daniel Lemire <lemire@gmail.com>
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