TimeSpan.FromSeconds does not return expected value after recent change to precision

[KrzysFR]

Following dotnet/coreclr#24279 that improves precision of TimeSpan.FromMilliseconds, I'm having issues with compatibility with .NET Framework, and more importantly weird inconsistencies in test code:

.NET Framework 4.7.2

Console.WriteLine(TimeSpan.FromSeconds(78043.43));
Console.WriteLine(TimeSpan.FromMilliseconds(78043430));
Console.WriteLine(TimeSpan.FromSeconds(78043.43) == TimeSpan.FromMilliseconds(78043430));

outputs:

21:40:43.4300000
21:40:43.4300000
True

.NET Core 3.0 preview 7:

Console.WriteLine(TimeSpan.FromSeconds(78043.43));
Console.WriteLine(TimeSpan.FromMilliseconds(78043430));
Console.WriteLine(TimeSpan.FromSeconds(78043.43) == TimeSpan.FromMilliseconds(78043430));

outputs:

21:40:43.4299999
21:40:43.4300000
False

I don't really mind the fact that there will be a difference in behavior between netfx and .NET Core (loss in precision vs not), but the main issue for me is that calling TimeSpan.FromSeconds(CONSTANT) and TimeSpan.FromMilliseconds(CONSTANT * 1000) on 3.0-preview7 does not return the same value (different Ticks).

This caused a lot of failing unit tests similar to Assert.That(...., Is.EqualTo(TimeSpan.FromSeconds(123.45))) that pass on netfx but not in .NET Core due to the weird behavior.

Looking at the implementation:

• TimeSpan.FromSeconds(78043.43) calls TimeSpan.Interval(78043.43, 10000000) which computes double millis = 78043.43 * 10000000 which gives 780434299999.99988, but then is truncated to a long, producing 780434299999 ticks, which is off by one.
• TimeSpan.FromMilliseconds(78043430) calls TimeSpan.Interval(78043430, 10000) so millis is now 780434300000 which does not change when truncated to a long.

remark: maybe the variable should now be renamed into 'ticks' instead of 'millis' here?

Maybe the +0.5/-0.5 rounding should still be applied, but on the ticks before truncating?

[stephentoub]

cc: @Anipik, @tannergooding

[tannergooding]

This has to do with double being the closest representable value and the gap between values increasing as the magnitude increases.

There are some tricks that could be done to keep it accurate without incurring the rounding loss/etc, but they may come at a perf cost.

[Const-me]

@tannergooding I wonder why don't you want to round? I think rounding is the best thing to do here, I always use round in my numeric code when I'm casting from float to int, it's 50% more precise in average.

The performance cost is barely measurable. If you have SSE4.1 it's a single instruction, roundsd. If you don't have SSE4.1 slightly more, but also cheap.

[DanielBryars]

Yes this change in behaviour confused me no end. The documentation is now wrong (because it is documented to round to 1ms) - I've described this in a stackoverflow question

[joperezr]

Keeping this in 5.0 to at least document the new/changed behavior.

[ericstj]

There are some tricks that could be done to keep it accurate without incurring the rounding loss/etc, but they may come at a perf cost.

@tannergooding were you planning to do something here?

[tannergooding]

I don't think there is anything reasonable that could be done for .NET 5 aside from documenting the change. Anything further would require quite a bit of investigation and would likely hit some barriers simply due to the precision loss introduced by floating-point values

[tvone-timmoore]

We have come across this bug in .NET 5, after migrating from .NET 4.6 to .NET 5/6.

As you can see from below that the .NET 5 version has less precision than .NET 4.6 so returns different values.

The .NET 5 TimeSpan.FromMilliseconds() method has chopped some of the value off.

We found this because it caused our app to crash when trying to parse what was previously 800 to an int.

double duration = 799.99999999999989;

.NET 4.6 (good)

TimeSpan.FromMilliseconds(duration)
{00:00:00.8000000}
    Days: 0
    Hours: 0
    Milliseconds: 800
    Minutes: 0
    Seconds: 0
    Ticks: 8000000
    TotalDays: 9.2592592592592591E-06
    TotalHours: 0.00022222222222222221
    TotalMilliseconds: 800
    TotalMinutes: 0.013333333333333334
    TotalSeconds: **0.79999999999999993**

.NET 5 or 6 (bad)

TimeSpan.FromMilliseconds(duration)
{00:00:00.7999999}
    Days: 0
    Hours: 0
    Milliseconds: 799
    Minutes: 0
    Seconds: 0
    Ticks: 7999999
    TotalDays: 9.2592581018518525E-06
    TotalHours: 0.00022222219444444443
    TotalMilliseconds: 799.9999
    TotalMinutes: 0.013333331666666667
    TotalSeconds: **0.7999999**
    _ticks: 7999999

[tannergooding]

As you can see from below that the .NET 5 version has less precision than .NET 4.6 so returns different values.

This is a case of more precision, not less. TimeSpan more accurately represents the value you input rather than rounding it and silently dropping specified precision.

[tvone-timmoore]

At first it looks like more but it is less, so .NET 5 round down when it should be rounding up.

.NET 4.6 = 0.79999999999999993
.NET 5.0 = 0.7999999

[tvone-timmoore]

I've done some investigation and found the reason for the bug in .NET 5/6/7:

All versions use a long to store the ticks, so they have the same internal storage.
Use of * and / results in slightly different values, but this is probably not really a problem.

• .NET 4.7 : TotalMilliseconds uses (double)_ticks * MillisecondsPerTick
• .NET 5.0 : TotalMilliseconds uses (double)_ticks / TicksPerMillisecond

BUT the main different is that .NET 4.7 has an extra line to handle double rounding, whereas the other .NET versions do not, which causes the bug by truncating the double value without adjusting for double rounding before type casting to a long.

[Edit] I've corrected my above statement about the bug, but I don't see any strike-thru option.

.NET 4.7

       private static TimeSpan Interval(double value, int scale) 
       {
            double tmp = value * scale;     // For FromMilliseconds() scale is 1

            // >>>>> The next line is missing from .NET 5.0
            double millis = tmp + (value >= 0? 0.5: -0.5);

            return new TimeSpan((long)millis * TicksPerMillisecond);   // TicksPerMillisecond = 10000
        }

.NET 5

        private static TimeSpan Interval(double value, double scale)
        {
            double ticks = value * scale;      // For FromMilliseconds() scale is 10000

            // >>>>> Missing adjustment for double rounding

            return IntervalFromDoubleTicks(ticks);
        }

        private static TimeSpan IntervalFromDoubleTicks(double ticks)
        {
            return new TimeSpan((long)ticks);
        }

[tvone-timmoore]

I have hacked a copy of the .NET 5.0 TimeSpan code to test this for FromMilliseconds(). (yes I know I can folk the code :))
As you can see my "fixed version" TimeSpan5_0Fixed returns the correct values now. It may not give correct values for the other From methods as it's just a hack.

    private static TimeSpan5_0Fixed Interval(double value, double scale)
    {
        double ticks = value * scale;

        double millis = ticks + (value >= 0 ? 0.5 : -0.5);

        return IntervalFromDoubleTicks(millis);
    }

[tvone-timmoore]

The TimeSpanTests is missing the edge cases for doubles and only has safe values, I believe it should also include numbers like 0.99999999999999993.

    public static void ConvertToTimeSpanPrecisionTest()
    {
        Assert.Equal(12345, TimeSpan.FromMilliseconds(1.23456).Ticks);
        Assert.Equal(12345, TimeSpan.FromMilliseconds(1.234567).Ticks);

        Assert.Equal(12345600, TimeSpan.FromSeconds(1.23456).Ticks);

        Assert.Equal(1.23456 * 60 * 10_000_000, TimeSpan.FromMinutes(1.23456).Ticks);
    }

[jkotas]

       // >>>>> The next line is missing from .NET 5.0
       double millis = tmp + (value >= 0? 0.5: -0.5);

This was deleted in dotnet/coreclr#24279 for some reason.

[tannergooding]

The logic isn't really correct and isn't correctly accounting for rounding or floating-point inaccuracies for large values.

[tvone-timmoore]

The logic isn't really correct and isn't correctly accounting for rounding or floating-point inaccuracies for large values.

Maybe as stated it's just a "hack" (from .NET 4.7 code) to demonstrate there is a bug in .NET 5/6/7, as the From methods use doubles the code does not correctly handle doubles. :)

[tannergooding]

The algorithm, realistically, should be breaking the double into its integral and fractional parts (using the internal Math.ModF).

The integral portion, provided its in range of ulong.MaxValue should be converted to a ulong (and otherwise fail). This will be an exact conversion, noting this is exact from the represented double and not necessarily exact from whatever the user "typed" as some literal.

The fractional portion then needs to be carefully handled since scaling it will result in precision loss. For FromSeconds and below, we need to scale by up to 10 million (ticks per second). This should be small enough that any rounding/precision loss is minimal and not visible within the range of a "tick".

FromDays (864 billion ticks) and FromHours (`36 billion ticks) likely need some consideration. The rounding loss should be minimal and potentially still within range of a "tick", but I've not checked the math to validate this.

[Const-me]

@tannergooding The old way was strictly better.

The rounding used in old .NET runtime resulted in 0.25 ticks average rounding error when converting from double to ticks, and no systematic bias.
The truncation used in the current version results in 0.5 ticks average rounding error which is twice as high, and it also introduced a systematic bias towards zero.

Here’s a demo of that systematic bias caused by truncation.

using System;

static class Program
{
	static void Main( string[] args )
	{
		TimeSpan initial = TimeSpan.FromMinutes( 1.23456 );
		TimeSpan ts = initial;
		for( int i = 0; i < 1000000; i++ )
		{
			ts = TimeSpan.FromSeconds( ts.TotalSeconds * Math.PI );
			ts = TimeSpan.FromSeconds( ts.TotalSeconds / Math.PI );
		}

		TimeSpan diff = initial - ts;
		Console.WriteLine( "Difference: {0} ticks, {1} ms", diff.Ticks, diff.TotalMilliseconds );
	}
}

The old runtime prints 0 ticks, 0 ms, the .NET 6 runtime prints 564209 ticks, 56.4209 ms.

The correct answer is zero: the body of the loop first multiplies the timespan by a constant, then divides by the same constant.

[tannergooding]

It was strictly better for some inputs and worse for others. We should not "fix" this by putting back in still broken behavior.

We should fix this properly and ensure that it works for both large and small values, which I gave a description of how to do in my last comment.

[tannergooding]

The correct answer is zero: the body of the loop first multiplies the timespan by a constant, then divides by the same constant.

This is notably not correct. (x * y) / y is not guaranteed to produce x because floating-point has finite precision and therefore introduces rounding error.

A simplistic example is (Math.PI * double.Epsilon) / double.Epsilon which produces 3, not Math.PI. This also reproduces with many other inputs/examples.

[Const-me]

and worse for others

Do you have a test case when it’s worse?

This is notably not correct. (x * y) / y is not guaranteed to produce x because floating-point has finite precision and therefore introduces rounding error.

The formula is more complex than just FP64 math because TimeSpan is keeping integer ticks @ 10MHz, and TimeSpan.FromSeconds rounds to integer. As you see from my demo, rounding to nearest integer actually stabilizes the math, while truncation introduces a systematic bias which causes numerical errors to accumulate with more operations on the TimeSpan.

A simplistic example is (Math.PI * double.Epsilon) / double.Epsilon which produces 3

I don’t believe the example is relevant because Math.PI * double.Epsilon is about 1.48E-323. If the value is seconds, not only it’s well below 1 TimeSpan tick, it’s even well below Planck time i.e. doesn’t make any sense.

[tannergooding]

Do you have a test case when it’s worse?

We have had and have fixed multiple precision bugs in the last few releases, especially where inputs were being unnecessarily truncated and the full tick precision wasn't being respected.

I don’t believe the example is relevant because Math.PI * double.Epsilon is about 1.48E-323. If the value is seconds, not only it’s well below 1 TimeSpan tick, it’s even well below Planck time i.e. doesn’t make any sense.

Again, this was a simplistic example showing that you cannot and should not rely on (x * y) / y returning x. There are many scenarios across the entire 64-bit tick range and entire range of inputs for Days, Hours, etc when represented as a double that imprecision will be introduced.

The correct fix here is to update the algorithm to correctly break apart the double into an exact integer part and inexact fractional part. The inexact fractional part is then the only place where rounding considerations exist. For the Add*(double value) functions that are present, this rounding consideration should all be easily mitigated within the confines of the tick range.

[tannergooding]

There is some decently complex math that goes into this, but the premise is that the integer part of any double up to 2^53 is exactly representable.

Likewise, the primitive operations (including +, -, *, /, etc) take the input "as given", compute the result "as if to infinite precision and unbounded range", and then round the result "to the nearest representable value".

The integer and fractional portions can always be split into their own "exact" values. This leaves a fractional portion which, provided the scale amount is less than floor(log10(2^53)) (15) digits will result in another "exact" integer representation of the fractional portion and which can be used to adjust the tick value further. There will be up to 4.265.. more digits represented which can then have rounding error present and this is small enough that it shouldn't matter. -- Even in the case of Days where 1.0 represents 864 billion ticks, we need at most 1.1574 * 10^-12 to represent a single "tick" and so fractional portion will scale correctly into the integer realm.

[Const-me]

We have had and have fixed multiple precision bugs in the last few releases, especially where inputs were being unnecessarily truncated and the full tick precision wasn't being respected.

Can you recall or find any of these? Without knowing what specifically was fixed, it’s not really possible to come up with a good solution.

But from general perspective, a simple rounding should be enough. FP64 precision is enough to represent exact ticks up to 28.5 years or something. And these Add* functions should convert the double argument into ticks (using banker’s rounding which is precisely what these CPU rounding instructions are doing), then do int64 addition on the value.

The correct fix here is to update the algorithm to correctly break apart the double into an exact integer part and inexact fractional part. The inexact fractional part is then the only place where rounding considerations exist.

I’m not sure that’s actually correct. Ideally, look for a well-tested off-the-shelf library which implements either FP128 (like boost) or arbitrary-precision (like GMP but beware of the license)

However, I don’t think it’s worth the complexity for the time spans. At least not until I saw a realistic use case which demonstrates the errors of the simple FP64 + rounding approach. These libraries are very complicated and not particularly fast, I only using them when I absolutely need to.

[tannergooding]

These libraries are very complicated and not particularly fast, I only using them when I absolutely need to.

The perf here won't be hurt and isn't going to involve anything complex. In fact, several parts will likely become faster or mitigated by other handling we already have/need.

ModF (the API that splits into integer and fractional parts) already has a fast implementation that defers to the CRT. But even without that, the implementation is very trivial.

ModF requires getting the actual bits (a 1-cycle single instruction), a comparison + branch to handle NaN/Infinity. And otherwise trivial arithmetic (integer shift + subtract to get the unbiased exponent; and then two shift + masks to get the relevant integer and fractional parts).

You're looking at about 15 instructions (in the worst case) with no more than 2 (generally predicted) branches to get the relevant parts and ensure this is a correctly computed result and without introducing rounding error. -- On modern hardware, you can actually extract these parts in 2 instructions since there are dedicated hardware support for this.

Can you recall or find any of these? Without knowing what specifically was fixed, it’s not really possible to come up with a good solution.

There are a number of bugs related to TimeSpan: https://github.com/dotnet/runtime/issues?q=is%3Aissue+TimeSpan+is%3Aopen+label%3Abug

Many of these are related to anything more precise than 1ms not working as expected. New APIs covering microseconds and nanoseconds will likewise be impacted (#23799)

The most correct thing, and which will not cause any kind of significant slowdown but which will ensure correct computation is to handle this in a way that minimizes rounding error and best preserves the input values given by the user. This would involve using ModF as I've described.

[tvone-timmoore]

The more I use .NET 5 the more it looks like there is a problem with the System.Double type.

.NET 5

		float tf = 1000000.0f * 1.005f; // 1005000
		double  td = 1000000.0d * 1.005d; // **1004999.9999999999**
		decimal tm = 1000000.0m * 1.005m; // 1005000.0000

.NET FW 4.7.2

		float tf = 1000000.0f * 1.005f;  // 1005000
		double  td = 1000000.0d * 1.005d;  // **1005000**
		decimal tm = 1000000.0m * 1.005m;  // 1005000.0000

[tannergooding]

double, float, and Half are binary IEEE 754 types. Due to being binary (base-2) rather than decimal (base-10) any represented value is built up by various "powers of 2".

That is, 1.005 as a decimal-based value is (1 * 10^0) + (0 * 10^-1) + (0 * 10^-2) + (5 * 10^-3). Any decimal value can be constructed by taking the individual digits (which can be 0 through 9) and multiplying that by a power of 10 representing that digits "place".
Since double is a binary-based value, any representable number is constructing by taking the individual binary digits (which can be 0 or 1) and multiplying that by a power of 2 representing that digits place.

Since you cannot construct 1.005 using such powers of 2, it cannot be exactly represented and instead gets rounded to the nearest representable value. This exact value is 1.004999999999999893418589635984972119331359863281251. The next largest representable is 1.0050000000000001154631945610162802040576934814453125. The difference between the first value and exactly 1.005 is 1.06581410364015027880668640136718749 × 10^-16. The difference between the second value and exactly 1.005 is 1.154631945610162802040576934814453125 × 10^-16. Therefore the first value is closer and the "nearest representable value".

Using double inherently adds noise and error to the algorithm and that becomes visible across many kinds of operations.

[tannergooding]

.NET Framework has "many" bugs around floating-point parsing and formatting that resulted in precision being lost and incorrect rounding. These are bugs that cannot be fixed due to the very high compat bar that .NET Framework has.

.NET Core, since 3.0, has fixed these bugs and documented the bug fix (breaking change). There was a blog post that went out at the time describing the change, the why, and some of the problematic scenarios that could be hit: https://devblogs.microsoft.com/dotnet/floating-point-parsing-and-formatting-improvements-in-net-core-3-0/

The simplest example is that .NET Framework will not correctly handle double.Parse(Math.PI.ToString()). The returned value did not equate to Math.PI and resulted in "silent" truncation and loss of precision, which was not IEEE 754 standards compliant.

.NET Core 3.0 and later ensures that the value roundtrips by default and so there is no loss in precision in the displayed/formatted string. The actual computation for 1000000.0d * 1.005d hasn't changed, only what value gets displayed when you convert it to a string.

[tvone-timmoore]

@tannergooding Thanks for the detailed reply, it is appreciated, and the link.

Using {0:R} I can see the value of the double in .NET FW 4.7 is indeed 1004999.9999999999.

We are evaluating the impact on our software given the link you have provided.