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

Comparison of objects containing floats #639

Closed
amrcode opened this issue Jun 26, 2017 · 9 comments
Closed

Comparison of objects containing floats #639

amrcode opened this issue Jun 26, 2017 · 9 comments
Labels
documentation kind: question state: please discuss please discuss the issue or vote for your favorite option

Comments

@amrcode
Copy link

amrcode commented Jun 26, 2017

The operator==() method for json objects uses '==' on the underlying number_float_t type. This may result in false negatives and is contrary to the function's documentation which states "Floating-point numbers are compared indirectly: two floating-point numbers f1 and f2 are considered equal if neither f1 > f2 nor f2 > f1 holds". Could this behavior be changed to abs(f1 - f2) < std::numeric_limits<number_float_t>::epsilon() ?

@nlohmann
Copy link
Owner

I think this is a documentation issue. We changed the comparison to use ==, but forgot to adjust the documentation. The rationale of using == is not to introduce behavior that is different from double's behavior.

@amrcode
Copy link
Author

amrcode commented Jun 26, 2017

That makes sense. Is it recommended to avoid adding floats to json objects then? Using a string or other representation seems much safer, though slower.

@nlohmann
Copy link
Owner

What do you mean with "safer"? The JSON specification itself mentions using C doubles to represent floating point numbers.

@amrcode
Copy link
Author

amrcode commented Jun 26, 2017

I mean that using them in a json object potentially breaks the ability to compare json objects. I came across the documentation discrepancy when I was creating unit tests for my code and had literal doubles in my "expected" object and when I compared it with an input object operator== said the objects were not equal despite dump() returning identical strings. At first there were a few bits different in the hex of the doubles but they did not affect significant-enough digits to show up when printed. Later it seemed that even doubles with identical hex representations were sometimes not equal, I suspect due to conversions going into and out of the json object.

@nlohmann nlohmann added the state: please discuss please discuss the issue or vote for your favorite option label Jun 26, 2017
@amrcode
Copy link
Author

amrcode commented Jun 27, 2017

This: https://randomascii.wordpress.com/2012/03/21/intermediate-floating-point-precision/ is a good writeup of various issues with working with floating point values. Using the built-in == for comparison is, I think, the correct way to go. Being a general-purpose library the burden of dealing with special cases should be on the user because not everyone is going to want to do this comparison the same way. It would be nice to have some control over the comparison operations but I don't see a clear, easy way to achieve that.

@nlohmann
Copy link
Owner

In the end, I think the best would be for a user with special comparison needs to provide her own comparison function and cope with the types as necessary. I think all internally stored values can be queried using the get() functions, so basically this function could be realized with a switch over the type().

@nlohmann
Copy link
Owner

nlohmann commented Jul 7, 2017

I think there is nothing to do from the library side, right?

@amrcode
Copy link
Author

amrcode commented Jul 7, 2017

I don't think so, other than the documentation update. Thanks!

nlohmann added a commit that referenced this issue Jul 7, 2017
Removed deprecated documentation of the comparison operator.
@nlohmann
Copy link
Owner

nlohmann commented Jul 7, 2017

Right. Thanks for reporting this!

@nlohmann nlohmann closed this as completed Jul 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation kind: question state: please discuss please discuss the issue or vote for your favorite option
Projects
None yet
Development

No branches or pull requests

2 participants