Fix the DatetIme.Add methods docs to reflect the new behavior in .NET 7.0

[tarekgh]

In .NET 7.0 we have improved the calculation precision for most of Add methods in DateTime. This is done in the PR dotnet/runtime#73198. The docs of these APIs need to get updated to reflect the new behavior.
For example the doc https://docs.microsoft.com/en-us/dotnet/api/system.datetime.addseconds?view=net-6.0#remarks mention The value parameter is rounded to the nearest millisecond. which is not true any more.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[danmoseley]

What is the correct statement to make for .NET 7?

[tarekgh]

What is the correct statement to make for .NET 7?

The value parameter is rounded to the nearest tick.
We may need to explain a little what we mean here as the calculation is done as double operation and then cast to long which can lose some precision. @tannergooding has nice comment which explaining that.

[danmoseley]

Likely just enough to say the millisecond rounding exists for < .NET 7 and that's it.

[tarekgh]

But this unclear what is .NET 7.0 behavior is. no?

[danmoseley]

Sure, but maybe it's "good enough for almost everyone". Whatever works.

[tannergooding]

I think saying to at least the nearest millisecond is accurate. In some cases additional precision may be possible, but it is dependent on the exact inputs.

[tarekgh]

Note we need to have a comment for other Add APIs like AddMicrosoeconds. I don't think at least the nearest millisecond would be accurate to say that there.

[tannergooding]

Even AddMicroseconds is only "millisecond accurate" for certain inputs.

Consider that MaxMicroseconds > 2^53 and therefore if a user requested we add 9007199254740993 microseconds, we'd actually only add 9007199254740992.0 microseconds, being off by 1 microsecond (this is because 9007199254740993 is not exactly representable as a double). Given MaxMicroseconds we could be up to (I believe, did the math in my head) 128 off from the number a user typed in.

In practice it is microsecond accurate, and for any input up to 2^42 (again, math in head, might be slightly off) input it will even be nanosecond accurate. But it is variable.

[tarekgh]

Ok, what do you suggest? leave the docs as it is written The value parameter is rounded to the nearest millisecond. ? or want to add or change something?

[tannergooding]

Noting that there is no "fix" for such large inputs given an API that takes double. We'd have to expose an overload that takes long or ulong (which is technically a silent source breaking change, since AddMicroseconds(9007199254740993) now does something different).

The issue has to do with the finite space in which double can exactly represent integrals and many users not expecting that AddMicroseconds(9007199254740993) is actually AddMicroseconds((double)9007199254740993) which causes it to really be AddMicroseconds(9007199254740992.0) thus losing precision from what they typed out.

[tannergooding]

Ok, what do you suggest? leave the docs as it is written The value parameter is rounded to the nearest millisecond. ? or want to add or change something?

I think something like the following would cover the necessary information:

The value parameter is rounded to at least the nearest millisecond. More accuracy may be available for certain inputs provided they represent less than 2^53 ticks.

We could also keep it as is, where it is short/simple or have an expanded remarks section that goes into the problem space in detail.

[tarekgh]

Ok, let's keep it as it is for now and we can revisit if we get any complaints. I'll close this issue.

[bretcope]

I'm a puzzled by the decision to leave an incorrect statement in the documentation. Not only does it not round to the nearest millisecond, it doesn't really "round" at all by the typical definition of "round" - it truncates, and not by millisecond.

I don't think you need to describe the details of the new conversion. Developers generally understand there are limits to floating point precision. However, I'd love to see something like, "prior to .NET 7, value was rounded to the nearest millisecond."

I came across this because these changes broke some of my orbit propagation tests which are very sensitive to time changes. I know the change is minor, but it is a breaking change, and it feels like the documentation should acknowledge that there was a change.