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

Format strings round incorrectly #1640

Closed
Suchiman opened this issue Jan 12, 2020 · 11 comments
Closed

Format strings round incorrectly #1640

Suchiman opened this issue Jan 12, 2020 · 11 comments
Labels
area-System.Numerics question Answer questions and provide assistance, not an issue with source code or documentation.
Milestone

Comments

@Suchiman
Copy link
Contributor

Suchiman commented Jan 12, 2020

According to Standard numeric format strings

On .NET Core 2.1 and later, the runtime selects the result with an even least significant digit (that is, using MidpointRounding.ToEven).

It only appears to do that for F0, using F1 for example, the results are incomprehensible to me sharplab

Console.WriteLine($"0.5: {0.5:F0}");
Console.WriteLine($"1.5: {1.5:F0}");
Console.WriteLine($"2.5: {2.5:F0}");
Console.WriteLine($"3.5: {3.5:F0}");
Console.WriteLine();
        
Console.WriteLine($"0.05: {0.05:F1}");
Console.WriteLine($"0.15: {0.15:F1}");
Console.WriteLine($"0.25: {0.25:F1}");
Console.WriteLine($"0.35: {0.35:F1}");
Console.WriteLine();
        
Console.WriteLine($"0.05: {Math.Round(0.05, 1):F1}");
Console.WriteLine($"0.15: {Math.Round(0.15, 1):F1}");
Console.WriteLine($"0.25: {Math.Round(0.25, 1):F1}");
Console.WriteLine($"0.35: {Math.Round(0.35, 1):F1}");

Results:

0.5: 0
1.5: 2
2.5: 2
3.5: 4

0.05: 0.1
0.15: 0.1
0.25: 0.2
0.35: 0.3

0.05: 0.0
0.15: 0.2
0.25: 0.2
0.35: 0.4

cc @tannergooding

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jan 12, 2020
@tannergooding
Copy link
Member

Math.Round, when specifying a digit count, has a bug and does not always return the correct result. This issue is tracked by https://github.com/dotnet/corefx/issues/36982.

For the other case, you must keep in mind that the literal given in C# source code (or parsed via Parse/TryParse) and the actual value represented may not be equivalent. Any string parsed to a double/float is parsed as if to "infinite-precision and unbounded-range" and then rounded to the nearest representable result.

Here are the values you gave and the exact representable value it is parsed to:

0.5     0.5
1.5     1.5
2.5     2.5
3.5     3.5
0.05    0.05000000000000000277555756156289135105907917022705078125
0.15    0.1499999999999999944488848768742172978818416595458984375
0.25    0.25
0.35    0.34999999999999997779553950749686919152736663818359375

As you can see:

  • 0.05 is actually just above the midpoint, so it rounds to 1, not 0
  • 1.5 is just below the midpoint, so it rounds to 1, not 2
  • 3.5 is just below the midpoint, so it rounds to 3, not 4
  • The other values are exactly representable and round as expected

@Suchiman
Copy link
Contributor Author

Thank you for the quick answer and explanation, trying to digest that since an hour... So my assumption that literal (not variables that already went through arithmetic) floating points are precise to the first few digits past decimal point is wrong, ToString() lies to you and basically MidpointRounding is unnecessary because it's extremely unlikely to ever hit the midpoint exactly (except on the first fractional digit it seems) and trying to produce equivalent output to JS and Haskell (from my sample size) or probably many other languages will be broken (even more) once Math.Round gets "fixed" to round based on otherwise invisible fractions. Also that means that rounding isn't even a real thing, it just scrubs those digits off until they're hidden behind the "representable" limit but while it looks like they are gone, they could come back at you if you were to round again!

Damn you IEEE754, once again ruining my day. I should just replace every occurrence of binary floating points by decimal and never ever look at them again, but then my results are probably more precise than what JS is outputting and i can't get comparative results, again, argh.

@Clockwork-Muse
Copy link
Contributor

... I mean, Math.Round is going to get you close to the desired values.

... and trying to produce equivalent output to JS and Haskell (from my sample size) or probably many other languages will be broken

... depending on how you're trying to generate values, it might not even be equivalent on the same C# process - adding some fraction, say. There's at least the ability to round-trip values if you go through string... so long as you use the appropriate format specifier. If you use something else, it's not likely to when dealing with real-world values.

This is before things like Java delegating to the hardware FPU and not enforcing strict 32/64-bit floating point (the hardware FPU may use 80 bits or more, which may subtly influence answers).

@tannergooding
Copy link
Member

floating points are precise to the first few digits past decimal point is wrong

Right, they can be incorrect with as few as one digit (e.g. 0.3 is actually 0.299999999999999988897769753748434595763683319091796875) or exact up to many more (the exact value with the most, for double, is 767 significant digits long: approx. 4.9406564584124654E-324).

trying to produce equivalent output to JS and Haskell

Unsure about Haskell, but Javascript numbers are explicitly spec'd to be the IEEE 754 double-precision floating-point type (i.e. System.Double in .NET).

Also that means that rounding isn't even a real thing

float and double are binary based numbers and so you can exactly represent powers of 2 (both positive and negative) within the overall range (MinValue to MaxValue). Due to the underlying representation, you can also represent whole numbers up to 2^24 (float) or 2^53 (double). Other numbers vary based on where it falls into the representation.

For example, you only have ~4 billion unique values for float (being 32-bits) and 2^64 values for double (being 64-bits), so a lot of values have to be rounded to the "nearest representable". The distance between values isn't constant across the entire range either, it actually changes every power of two. So, for example the distance between values from 0.5 to 1.0 (2^-1 and 2^0) is x (actually 1.1102230246251565404236316680908203125E-16) and the distance between values from 1.0 to 2.0 (2^0 and 2^1) is x * 2 (actually 2.220446049250313080847263336181640625E-16). This repeats all the way up to double.MaxValue and all the way down to 0 (with the same boundaries on the negative side). Once you hit 2^52 (for double) the gap between values is exactly 1, then 2 at 2^53, etc (so you can't even represent fractional portions at this point). It also means that approx. half of all representable values lie between -1.0 and +1.0.

You can actually find similar problems with any representation which attempts to represent a result which could have arbitrary precision in finite space (e.g. 2/3 can't be exactly represented in base 2 or 10 and ends up rounding the last digit and math which incurs large amounts of rounding error can be constructed for decimal as well). Its just that with decimal based floating-point numbers (like System.Decimal) it works more closely to how we expect "normal math" to work and you see less error (but it is also much slower since computers natively operate on a binary number system).

@tannergooding tannergooding added area-System.Numerics question Answer questions and provide assistance, not an issue with source code or documentation. and removed untriaged New issue has not been triaged by the area owner labels Jan 12, 2020
@tannergooding
Copy link
Member

(I've marked this as a question for now since the ToString behavior is "by-design" and the Math.Round issue is tracked by another bug, which I've transferred as #1643).

@Suchiman
Copy link
Contributor Author

Javascript numbers are explicitly spec'd to be the IEEE 754 double-precision floating-point type (i.e. System.Double in .NET).

Yeah but the rounding is different so the human output is different. Math.round uses Round half up which can be approximated in .NET using Math.Floor(value + 0.5) (it treats -0 as +0 but otherwise appears to work fine), then there's number.toFixed() which appears to do something like AwayFromZero but breaks for certain values, e.g.
0.015.toFixed(2) -> 0.01
Math.round(0.015 * 100) / 100 -> 0.02
Math.Round(0.015, 2, MidpointRounding.AwayFromZero) -> 0.02
but could be more float inprecision madness i guess.

Figuring that stuff out has been very frustrating 😞

@tannergooding
Copy link
Member

Math.round

This looks to be MidpointRounding.ToPositiveInfinity, which is also Math.Ceiling

number.toFixed()

Looks like the official spec allows between 0 and 100 digits, defaulting to 0 and calls number.prototype.ToString if the input >= 1e+21. At least from testing, it looks like toFixed also does to positive infinity. The spec is here:
image

We support all five IEEE 754 defined rounding modes, but Math.Round(value, digitCount) may not always return the correct result due to the bug I mentioned above, which may be leading to some of the issues you encountered above.

@Suchiman
Copy link
Contributor Author

Math.round

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/round#Description

If the fractional portion of the argument is greater than 0.5, the argument is rounded to the integer with the next higher absolute value. If it is less than 0.5, the argument is rounded to the integer with the lower absolute value. If the fractional portion is exactly 0.5, the argument is rounded to the next integer in the direction of +∞. Note that this differs from many languages' round() functions, which often round this case to the next integer away from zero, instead giving a different result in the case of negative numbers with a fractional part of exactly 0.5.

E.g.
while
Math.Round(-1.5, MidpointRounding.ToPositiveInfinity) -> -1
Math.round(-1.5) -> -1
(-1.5).toFixed() -> -2
but
Math.Round(1.4, MidpointRounding.ToPositiveInfinity) -> 2
Math.round(1.4) -> 1
(1.4).toFixed() -> 1
So the behavior on 1.4 exclues ToPositiveInfinity and the behavior on -1.5 excludes AwayFromZero, after consulting the summary chart on https://www.mathsisfun.com/numbers/rounding-methods.html i'm pretty sure it's Half round up

But i have to admit that i don't understand the spec for toFixed

@tannergooding
Copy link
Member

tannergooding commented Jan 12, 2020

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/round#Description

Right, the spec explicitly calls out:

If the fractional portion is exactly 0.5, the argument is rounded to the next integer in the direction of +∞

Which makes it ToPositiveInfinity See comment at the bottom as to the difference.

exclues ToPositiveInfinity

Yeah, I messed up. It looks to be AwayFromZero:

-3.5.toFixed() // -4
-2.5.toFixed() // -3
-1.5.toFixed() // -2
-0.5.toFixed() // -1
+0.5.toFixed() //  1
+1.5.toFixed() //  2
+2.5.toFixed() //  3
+3.5.toFixed() //  4

Math.Round(1.4, MidpointRounding.ToPositiveInfinity)

Ah, I see the issue. They treat Math.round as To nearest; ties to positive infinity; while we treat MidpointRounding.ToPositiveInfinity as the IEEE 754 double roundToIntegralTowardPositive(double) which is just to positive infinity (regardless of "nearest").

I think we had a discussion about this during API review, but I don't recall the details right now.

@Suchiman
Copy link
Contributor Author

@tannergooding thanks again for the answers. With some more tinkering, i was able to come up with the following helper class that manages to emulate for all cases that previously produced inconsistent output between C# and JS for me.

Interestingly enough, when you try to google for toFixed rounding, answers vary from "does not round" to "rounds but incorrectly" and "does round" and generally a recommendation to use Math.round if you want rounding, using Math.round(v * d) / d to round to specific amount of digits. Yet it seems that toFixed in JS rounds like the way Math.Round is intended to be fixed, not sure what that says about how people expect rounding to work and if that's a good idea to change in the first place.

Since the question is answered and there's probably nothing to be changed here, feel free to close.

static class JSMath
{
    public static double Round(double value)
    {
        return Math.Floor(value + 0.5);
    }

    public static decimal ToFixed(double value, int digits = 0)
    {
        if (digits > 20)
        {
            throw new ArgumentException("This implementation does not support more than 20 digits", nameof(digits));
        }

        Span<char> chars = stackalloc char[100];
        if (value.TryFormat(chars, out int written, "F21"))
        {
            chars = chars.Slice(0, written);
        }
        else
        {
            throw new NotImplementedException();
        }
        return Math.Round(Decimal.Parse(chars), digits, MidpointRounding.AwayFromZero);
    }
}

@tannergooding tannergooding added this to the Future milestone Jun 23, 2020
@tannergooding
Copy link
Member

Going to close this as I believe the original question was answered. Feel free to reopen if you feel it was not or if you feel there is still work needed here.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Numerics question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

No branches or pull requests

4 participants