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

[release/9.0-staging] Fix TensorPrimitives.MultiplyAddEstimate for integers #113094

Open
wants to merge 3 commits into
base: release/9.0-staging
Choose a base branch
from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Mar 3, 2025

Backport of #113047 to release/9.0-staging

/cc @stephentoub

Customer Impact

  • Customer reported
  • Found internally

TensorPrimitives.MultiplyAddEstimate produces the wrong answers when used with integer types, e.g.

using System.Numerics.Tensors;

ReadOnlySpan<long> x = [1, 2, 3];
ReadOnlySpan<long> y = [4, 5, 6];
ReadOnlySpan<long> addend = [7, 8, 9];
long[] destination = new long[3];
TensorPrimitives.MultiplyAddEstimate(x, y, addend, destination);

Console.WriteLine(string.Join(", ", destination));

That should print 11, 18, 27 (i.e. 1*4 + 7, 2*5 + 8, 3*6 + 9), but it prints 7, 8, 9.

Customers using this method will get the wrong numerical results.

Regression

  • Yes
  • No

Testing

Added new tests to the System.Numerics.Tensors test suite. Previously there were only tests for floating-point types, and this issue was specific to integer types.

Risk

Low. It's a very isolated change specifically for the MultiplyAddEstimate implementation, where previously there was a structure like:

if (typeof(T) == typeof(double)) { ... /* optimize for double */ }
else { ... /* optimize for float, but oops if T is an integer */ }

and this fixes it to be:

if (typeof(T) == typeof(double)) { ... /* optimize for double */ }
else if (typeof(T) == typeof(float)) { ... /* optimize for float */ }
else { ... /* handle generically */ { ... }

There's no reasonable way someone could have taken a dependency on the previous behavior. This method was also only introduced in .NET 9.

Copy link
Contributor

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

@stephentoub stephentoub added the Servicing-consider Issue for next servicing release review label Mar 3, 2025
@stephentoub
Copy link
Member

cc: @jeffhandley, @artl93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Numerics Servicing-consider Issue for next servicing release review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants