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

TensorPrimitives asymmetry between MultiplyAdd and MultiplyAddEstimate with trivial inputs #112989

Closed
HobbsCode opened this issue Feb 27, 2025 · 1 comment · Fixed by #113047
Closed
Labels
area-System.Numerics.Tensors bug in-pr There is an active PR which will close this issue when it is merged

Comments

@HobbsCode
Copy link

Description

I am seeing some wrong results from TensorPrimitives.MultiplyAddEstimate() when using 64-bit integral types.

Reproduction Steps

In the program below, various types are tested for symmetry of results between MultiplyAdd and MultiplyAddEstimate. On my machine, the 64-bit integral types are producing "4, 5, 6" for MultiplyAddEstimate. All results should be "11, 19, 27". Notably, the wrong results coincide with the values of the addends argument.

using System.Numerics.Tensors;

CompareMultiplyAddWithMultiplyAddEstimate<Half>();   
CompareMultiplyAddWithMultiplyAddEstimate<float>();  
CompareMultiplyAddWithMultiplyAddEstimate<double>(); 
CompareMultiplyAddWithMultiplyAddEstimate<NFloat>(); 
CompareMultiplyAddWithMultiplyAddEstimate<Complex>();
CompareMultiplyAddWithMultiplyAddEstimate<sbyte>();  
CompareMultiplyAddWithMultiplyAddEstimate<byte>();   
CompareMultiplyAddWithMultiplyAddEstimate<short>();  
CompareMultiplyAddWithMultiplyAddEstimate<ushort>(); 
CompareMultiplyAddWithMultiplyAddEstimate<int>();    
CompareMultiplyAddWithMultiplyAddEstimate<uint>();   
CompareMultiplyAddWithMultiplyAddEstimate<long>();   // problem
CompareMultiplyAddWithMultiplyAddEstimate<ulong>();  // problem
CompareMultiplyAddWithMultiplyAddEstimate<nint>();   // problem
CompareMultiplyAddWithMultiplyAddEstimate<nuint>();  // problem
CompareMultiplyAddWithMultiplyAddEstimate<Int128>(); 
CompareMultiplyAddWithMultiplyAddEstimate<UInt128>();

Console.WriteLine("\n\nPress any key...");
Console.ReadKey(true);
return;

static void CompareMultiplyAddWithMultiplyAddEstimate<TNum>() where TNum : INumberBase<TNum>
{
    TNum one = TNum.One;
    TNum two = one + one;
    TNum three = two + one;
    TNum four = three + one;
    TNum five = four + one;
    TNum six = five + one;
    TNum seven = six + one;

    ReadOnlySpan<TNum> initialValues = [one, two, three];
    ReadOnlySpan<TNum> addends = [four, five, six];
    TNum multiplier = seven;

    TNum[] resultsOfMultiplyAddEstimate = new TNum[3];
    TNum[] resultsOfMultiplyAdd = new TNum[3];

    TensorPrimitives.MultiplyAddEstimate(initialValues, multiplier, addends, resultsOfMultiplyAddEstimate);
    TensorPrimitives.MultiplyAdd(initialValues, multiplier, addends, resultsOfMultiplyAdd);

    Console.WriteLine($"Type: {typeof(TNum).Name}");

    Console.Write($"   MultiplyAddEstimate: "); 
    PrintValues(resultsOfMultiplyAddEstimate);
    Console.WriteLine();

    Console.Write($"           MultiplyAdd: ");
    PrintValues(resultsOfMultiplyAdd);
    Console.WriteLine();
    Console.WriteLine();
}

static void PrintValues<TNum>(TNum[] values)
{
    foreach (var item in values)
    {
        Console.Write($"{item}, ");
    }   
}

Expected behavior

All results should be "11, 19, 27".

Actual behavior

The 64-bit integral types are producing "4, 5, 6" for MultiplyAddEstimate.

Regression?

unknown

Known Workarounds

Use MultiplyAdd instead of MultiplyAddEstimate.

Configuration

Windows 11 (10.0.26100) x64
.NET 9.0.200

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 27, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Feb 27, 2025
@vcsjones vcsjones added area-System.Numerics.Tensors and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 27, 2025
Copy link
Contributor

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

@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Mar 2, 2025
@stephentoub stephentoub added bug and removed untriaged New issue has not been triaged by the area owner labels Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Numerics.Tensors bug in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants