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

Incorrect handling of precision in some cases #13

Open
solarissmoke opened this issue Nov 7, 2023 · 1 comment
Open

Incorrect handling of precision in some cases #13

solarissmoke opened this issue Nov 7, 2023 · 1 comment

Comments

@solarissmoke
Copy link

solarissmoke commented Nov 7, 2023

The way GeojsonEquality check handles precision can result in incorrect results. The following example illustrates the problem:

const point1 = {"type": "Point", "coordinates": [100.55719328, 23.0365925]}
const point2 = {"type": "Point", "coordinates": [100.55719327926636, 23.03659249995325]}

const eq1 = new GeojsonEquality({ precision: 7});
const eq2 = new GeojsonEquality({ precision: 6});
eq1.compare(point1, point2)   // returns true
eq2.compare(point1, point2)   // returns false !

This is incorrect. If two points are equal to a precision of 7, then by definition they must be equal to a precision of 6 as well. The problem here arises from the use of toFixed to round the coordinates. The problem is that this introduces rounding errors that then cause the equality check to fail.

I think a better way to check that values are within the required precision is something like:

Math.abs(a - b) < (10**-6) // returns true, as expected
Math.abs(a - b) < (10**-7) // returns true, as expected

Very happy to make a PR if you agree that this is a preferable approach.

@gagan-bansal
Copy link
Contributor

@solarissmoke my apologies for responding late. Thank you for pointing out this issue.
Your approach seems better in this case as well as in general. You can proceed with PR if still you have time or else fix this on coming weekend.

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

No branches or pull requests

2 participants