-
Notifications
You must be signed in to change notification settings - Fork 5k
Double.Parse gets the wrong answer for inputs with as few as 1 significant digits. #4406
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
Comments
Here are some other input strings from the test program in the comments of dotnet/roslyn#4221 that
Note that we get the wrong answer for some cases of numbers with just three digits. |
On 32-bit, Bit patterns: This behaves correctly on 64-bit though. |
I have written some tests (https://gist.github.com/leppie/4239b6fda4c999c4cd77) for Here are the failures I get (running on 32-bit):
Note: IronScheme does not use I have cherry picked some by hand to check on http://www.exploringbinary.com/floating-point-converter/ and they all match. All of the results only differ by one bit. There is no pattern that I can see though. ECMA-334 says round-to-nearest mode should be used for real literals, and I expect Update: Updated results as |
One thing which may be handy here is to use my DoubleConverter.cs which allows the exact value of a Use with: Console.WriteLine(DoubleConverter.ToExactString(foo)); For example, to show Neal's first example:
Output:
|
@jskeet: It was used in the Roslyn issue (dotnet/roslyn#4221) but it this is really a 3 pronged issue given:
Testing against broken expectations will not get us anywhere :( Also, |
Ha - I didn't know DoubleConverter was really used except by me :) It sounds like really there are only two root issues:
It sounds like my output in the previous comment is due to the |
And running on 64-bit I get these failures:
There seems to be quite a number of differences in the failures between 32-bit and 64-bit. Note: IronScheme does not use Update: Updated results as |
Interesting fact: All of these tests roundtrip correctly. At least it is consistent in one way 👍 |
If any of you would care to offer a correct C# implementation of floating-point scanning, we'd use it in Roslyn (C# and VB compilers)! |
Another problem number.
|
Appears that is only for printing and not parsing :( |
Wrong answer with as few as 1 input digitsThe strings /cc @MattGertz @jaredpar |
@terrajobst Would you folks consider a contribution to replace |
@gafter it is something we would consider. The biggest issue I see is the compat issue with existing code for the full framework, if we felt the risk was too high we could consider #ifdef'ing it for CoreCLR. |
@weshaggard Do you expect that we have customers depending on the fact that |
A suitable parser can be found in https://github.com/dotnet/roslyn/blob/614299ff83da9959fa07131c6d0ffbc58873b6ae/src/Compilers/Core/Portable/RealParser.cs |
There are a number of issues with
Double.Parse
. Ideally we would like it to follow the IEEE spec section 5.12.2 (for inputs of 20 or fewer digits, round to nearest representable double, but when two distinct representable numbers are precisely equidistant, round to even), give results that are the same when run on x86 vs x64 (we currently differ on the string"0.6822871999174"
, for example, only returning the correct result on x86), and be monotonic. It is currently none of those.Here is a short snippet of C# code that illustrates all three issues. When run on x64, it demonstrates that
Double.Parse
is not monotonic.The strings "1e126" and "5e-107"appear to parse into different numbers from double.Parse depending on whether you're running on a 32-bit or 64-bit runtime. The former gets the right result from double.Parse only on 64-bit systems; the latter gets the right result from double.Parse only on 32-bit systems.
See also dotnet/roslyn#4221, which demonstrates that this bug is inherited into the compile-time behavior of the C# compiler. Because the VS IDE runs in x86 and the batch compiler runs in x64, the design-time and run-time experiences are inconsistent.
The text was updated successfully, but these errors were encountered: