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

multipleOf logic produces incorrect results #397

Open
DanielBauman88 opened this issue Nov 21, 2022 · 1 comment
Open

multipleOf logic produces incorrect results #397

DanielBauman88 opened this issue Nov 21, 2022 · 1 comment

Comments

@DanielBauman88
Copy link
Contributor

The logic for multipleOf compares the remainder of a division with the f64 EPSILON constant to decide if a number is a multiple or not. This approach doesn't make sense. EPSILON is simply the difference between 1.0 and the next larger representable number. With regards to the multipleOf calculation this is just an arbitrary constant and comparing remainders to it gives us no guarantees.

EG:

1e-15 is a multiple of 1e-16. But (1e-15 / 1e-16) % 1.0 = 1.7763568394002505e-15 which is greater than epsilon so it is incorrectly said to not be a multiple.

1.3e-16 is not a multiple of 1.3. But (1.3e-16 / 1.3) % 1.0 = 9.999999999999999e-17 which is less than epsilon so it is incorrectly said to be a multiple.

@DanielBauman88
Copy link
Contributor Author

DanielBauman88 commented Nov 21, 2022

The people over at Monaco have implemented a better way to implement this calculation without introducing incorrect results - https://github.com/microsoft/vscode-json-languageservice/blob/7a478cd29a387cf8813b0a32499e2347102d7a6c/src/parser/jsonParser.ts#L596

At a high level this solution works by multiplying both values by the smallest power of 10 that makes them both whole numbers and does the division on whole numbers to sidestep the issues with floating point math on fractions.

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

1 participant