-
Notifications
You must be signed in to change notification settings - Fork 9
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
ConvertStringToDouble slightly wrong #2
Comments
After removing lines where the reference number and your number have the same absolute difference to the input number, three bad cases remain:
And a few overflows cases:
|
Please recheck with the lastest version, where I've added Ryu-style algorithm variants. |
Now it is better, but still three clearly wrong cases:
And more including equidistance ones:
|
That was for calling ConvertStringToDouble. Calling RyuStringToDouble directly is worse:
|
Hm ok, then I will port the offical Ryu test cases to Pascal (from https://github.com/ulfjack/ryu/blob/master/ryu/tests/ ). for to see further. |
So, I've ported the relevant parts of the offical Ryu test cases to Pascal, which passes also as whole without one error. So what is left are probably then Ryu-Algorihm concept conditional differences to ensure e.g. roundtrips and other special characteristics. And since the Ryu paper ( https://dl.acm.org/citation.cfm?doid=3296979.3192369 ) has been peer reviewed several times, and since the one paper has also complete correctness proof of the algorithm, I would classify the Ryu algo as correct here. Even my reimplementation so far, since it passes also all offical Ryu testcase tests. The only difference to the original C implementation of the Ryu algorithm so far is that my reimplementation has no limit to a maximum count of to parsing 17 digits by dynamically truncating and roundig all further digits when needed. But please recheck it again anyway :-) |
See discussion at |
The only difference to the original C implementation of the Ryu
algorithm so far is that my reimplementation has no limit to a maximum
count of to parsing 17 digits by dynamically truncating and roundig
all further digits when needed.
Then it is probably rounding wrong
All the failing cases have at least 20 digits
|
So, after reading https://www.exploringbinary.com/decimal-to-floating-point-needs-arbitrary-precision/ i've reworked the slow path double parsing by parsing to a temporary 512-bit floaing point value, which breaking-tie-even-rounded down to a 64-bit floating point afterwards. But this new slow path is damn slow. So i'll implement more fine-tuned algo variants in between in the next days, from very fast (Eisel-Lemire), then fast (ryu) and then to damn slow (float512). Anyway, please recheck it. :-) |
That would be perfect Perhaps this is faster than damn slow: https://nigeltao.github.io/blog/2020/parse-number-f64-simple.html ? |
I have now implemented an additional a bit faster but still slow big-integer-based "Algorithm M" fallback path, but it currently only works for base 10. For the other bases (2 binary, 4, 8 octal, 16 hex) I will extend the algorithm in the next days, and when I am done with it, I will remove the old float512 fallback path. |
The 64-bit version passes all these tests But with the 32-bit version there seems to be a problem in ComputeFloat64.
|
Ok it seems, I've found out the issue. It is, or better, it was the FPU-using fast path of ComputeFloat64. Maybe I should IFdef'ing it out as whole, except for x86-64 and AArch64. |
So 32-bit used FPU and 64-bit used SSE. Then the 32-bit fast path works after |
The big-integer-based "Algorithm M" fallback path can also process base 2, base 4, base 8 and base 16 inputs. |
I have run it with the test cases of https://github.com/nigeltao/parse-number-fxx-test-data and it finds many slightly broken numbers
For example:
... parsingfail.txt
The text was updated successfully, but these errors were encountered: