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

Created on 8 Aug 2019  路  7Comments  路  Source: dotnet/runtime

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__
```C#
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:__
```C#
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?

area-System.Runtime bug

Most helpful comment

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.

All 7 comments

cc: @Anipik, @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.

@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.

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

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

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?

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

Was this page helpful?
0 / 5 - 0 ratings

Related issues

btecu picture btecu  路  3Comments

noahfalk picture noahfalk  路  3Comments

yahorsi picture yahorsi  路  3Comments

GitAntoinee picture GitAntoinee  路  3Comments

sahithreddyk picture sahithreddyk  路  3Comments