Bug 15763 – std.math.approxEqual is not symmetric

Status
RESOLVED
Resolution
FIXED
Severity
minor
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
All
OS
All
Creation time
2016-03-05T01:05:34Z
Last change time
2019-09-09T19:47:56Z
Keywords
pull, trivial
Assigned to
Steven Schveighoffer
Creator
Ryuichi OHORI

Comments

Comment #0 by r.97all — 2016-03-05T01:05:34Z
Current implementation of std.math.approxEqual is not symmetric. The code below gives an example. import std.math : eq = approxEqual; immutable real a = (2e-3)-(1e-5), b = (2e-3)+(1e-5); assert ( a.eq(b)); // (error relative to b) = 1 / (100.5) assert (!b.eq(a)); // (error relative to a) = 1 / ( 99.5) IMO at least we need to document this. It may be enough since the case this asymmetricity do something harm is rare. If we "fix" it, we must consider the backward compatibility, and/or performance. I would be pleased with an alternative for approxEqual like this: return ((a - b).abs < abserr) || ((a - b).abs / (a.abs + b.abs) < relerr / 2); Any opinion?
Comment #1 by schveiguy — 2017-03-31T23:50:20Z
I think that you need to pick one of the two parameters to determine the relative error. One could use a some test to determine the "best" value, but "best" is subjective. Armed with the knowledge that the second parameter is the one that is the basis for the percentage difference, you can call it in the correct order. Working on Doc PR.
Comment #2 by schveiguy — 2017-04-01T00:23:04Z
Hm... also found an issue, because if rhs is a range, but lhs is a value, it swaps the arguments and calls approxEqual(rhs, lhs). Clearly, if rhs is the relative difference determination, you don't want to do this (as the comparison isn't symmetrical). PR: https://github.com/dlang/phobos/pull/5316
Comment #3 by github-bugzilla — 2017-04-10T14:36:56Z
Commits pushed to master at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/9b4ae1e779a0d086b5e07c76df99f3c097884f5f fix issue 15763 - Document behavior of relative difference. Also fix issue where symmetry was assumed when lhs was a value and rhs was not. https://github.com/dlang/phobos/commit/74a339ea51fc8d568152be7bde1eb46d2e6df4d3 Merge pull request #5316 from schveiguy/fix15763 fix issue 15763 - Document behavior of relative difference merged-on-behalf-of: Jack Stouffer <[email protected]>
Comment #4 by github-bugzilla — 2017-06-17T11:34:20Z
Commits pushed to stable at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/9b4ae1e779a0d086b5e07c76df99f3c097884f5f fix issue 15763 - Document behavior of relative difference. Also fix https://github.com/dlang/phobos/commit/74a339ea51fc8d568152be7bde1eb46d2e6df4d3 Merge pull request #5316 from schveiguy/fix15763
Comment #5 by github-bugzilla — 2017-08-07T12:26:41Z
Commits pushed to newCTFE at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/9b4ae1e779a0d086b5e07c76df99f3c097884f5f fix issue 15763 - Document behavior of relative difference. Also fix https://github.com/dlang/phobos/commit/74a339ea51fc8d568152be7bde1eb46d2e6df4d3 Merge pull request #5316 from schveiguy/fix15763
Comment #6 by github-bugzilla — 2018-01-05T13:28:08Z
Commits pushed to dmd-cxx at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/9b4ae1e779a0d086b5e07c76df99f3c097884f5f fix issue 15763 - Document behavior of relative difference. Also fix https://github.com/dlang/phobos/commit/74a339ea51fc8d568152be7bde1eb46d2e6df4d3 Merge pull request #5316 from schveiguy/fix15763
Comment #7 by dlang — 2019-09-09T19:47:56Z
PR https://github.com/dlang/phobos/pull/7180 makes clearer, that the function is not intended to be symmetric.