search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.05k stars 4.69k forks source link

Investigate Tensor's Helpers.IsEqualWithTolerance helper #107934

Open stephentoub opened 2 weeks ago

stephentoub commented 2 weeks ago

The System.Numerics.Tensors test suite has a Helpers.IsEqualWithTolerance helper method. This is letting through as equal some values which should not be considered equal, in particular when one of the values is infinity or negative infinity. This is one of the reasons we didn't notice the bug cited by https://github.com/dotnet/runtime/issues/107838, because a value close to 0 and negative infinity were being considered equal. But when updating that helper to accomodate, other existing tests started failing. It's possible those are false positives, but it's also possible we have further edge case bugs to be fixed. We need to investigate.

stephentoub commented 2 weeks ago

@jeffhandley, this is what I was referring to earlier today offline.

dotnet-policy-service[bot] commented 2 weeks ago

Tagging subscribers to this area: @dotnet/area-system-numerics-tensors See info in area-owners.md if you want to be subscribed.

jeffhandley commented 2 weeks ago

Thanks, @stephentoub. We should investigate this during the .NET 9 GA milestone and verify correctness of test cases that were slipping through coverage before.

tannergooding commented 1 week ago

A valid implementation would be https://github.com/dotnet/runtime/blob/main/src/libraries/Common/tests/TestUtilities/System/AssertExtensions.cs#L818-L1037

This ensures that values such as inf and nan are checked exactly, while finite values are within an expected variance amount. It does so in a way that minimizes additional error and which can account for per input result differences.