-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Adding a naive implementation of various primitive tensor operations #91228
Conversation
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
Tagging subscribers to this area: @dotnet/area-system-numerics-tensors Issue Detailsnull
|
for (int i = 0; i < x.Length; i++) | ||
{ | ||
destination[i] = x[i] + y[i]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the plan is to get the functionality merged and to then follow-up with vectorization in time for 8.0 as well?
Let me know if you want to divvy that up at all or if you're going to tackle it all yourself. Presumably we'll end up wanting a Vector<T>
implementation for downlevel and a Vector128/256/512<T>
for .NET 8+.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The general plan is to get this first bit (covering element-wise operations) in.
Work on vectorizing them can then start while the vector-based tensor operations (Signmoid, Product, Sum, etc) get their naive implementations in.
@michaelgsharp and I will be splitting the work, but you're more than welcome to join in and help as well!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting the naive implementation in gives us both a scalar fallback and a baseline against which correctness of the SIMD implementation can be validated.
I should have tests up here in just a little bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michaelgsharp and I will be splitting the work, but you're more than welcome to join in and help as well!
Happy to help
src/libraries/System.Numerics.Tensors/src/System/Numerics/Tensors/TensorPrimitives.cs
Show resolved
Hide resolved
CC. @michaelgsharp, @luisquintanilla, @stephentoub this should be ready for review. As per the above this is just covering the simple "element-wise" functions and the naive implementations to give us a baseline. Once in, I'll move to the remaining "vector" functions and work on accelerating these using SIMD can happen as well, allowing more concurrent work. CC. @ViktorHofer would appreciate if you could look over the project settings to make sure they look correct. |
If the intent is to publicly ship this library going forward, you want to remove the properties in You would also need to update the README.md file which mentions that the library doesn't ship publicly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Probably still want another opinion though.
src/libraries/System.Numerics.Tensors/ref/System.Numerics.Tensors.csproj
Outdated
Show resolved
Hide resolved
src/libraries/System.Numerics.Tensors/src/System.Numerics.Tensors.csproj
Outdated
Show resolved
Hide resolved
{ | ||
public static class TensorPrimitivesTests | ||
{ | ||
private const int TensorSize = 512; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have any tests for boundary conditions, e.g. empty? Or error conditions? Or is the plan for those to come separately?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added tests for the exception cases already. I can add ones for empty as well.
For floating-point correctness cases (i.e. infinity
, nan
, -0
, etc), I think it'd be good to add handling for after we move off the "naive" implementation.
src/libraries/System.Numerics.Tensors/tests/TensorPrimitivesTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
Please respond to my above question before merging: #91228 (comment) |
Removed the file that marked it as non-shipping and adjusted the readme. |
|
||
Provides APIs for performing primitive operations over tensors represented by spans of memory. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to update this README to the format that other libraries use, i.e. https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.Extensions.Configuration.Abstractions/README.md. This can happen in a follow-up but please don't forget about it.
Also, as this package is now shipping officially, we should add a PACKAGE readme as well. I will add an entry in #91210 to track that work.
…otnet#91228) * Adding a naive implementation of various primitive tensor operations * Adding tests covering the new tensor primitives APIs * Adding tensor primitives APIs to the ref assembly * Allow .NET Framework to build/run * Sync TFMs between ref and src, csproj simplication and clean-up * Apply suggestions from code review Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> * Don't use var * Fix the S.N.Tensors readme and remove the file marking it as non-shipping --------- Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> Co-authored-by: Michael Sharp <51342856+michaelgsharp@users.noreply.github.com>
* added Bcl.Numerics * Adding a naive implementation of various primitive tensor operations (#91228) * Adding a naive implementation of various primitive tensor operations * Adding tests covering the new tensor primitives APIs * Adding tensor primitives APIs to the ref assembly * Allow .NET Framework to build/run * Sync TFMs between ref and src, csproj simplication and clean-up * Apply suggestions from code review Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> * Don't use var * Fix the S.N.Tensors readme and remove the file marking it as non-shipping --------- Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> Co-authored-by: Michael Sharp <51342856+michaelgsharp@users.noreply.github.com> * Start vectorizing TensorPrimitives (#91596) * Start vectorizing TensorPrimitives Just does two functions to establish the files into which the rest of the implementations can be moved. * 6 more naive methods for Tensor Primitives. (#92142) * 6 more naive methods * updates from pr comments * Add remaining set of TensorPrimitives APIs for .NET 8 (#92154) * Add remaining set of TensorPrimitives APIs for .NET 8 Adds non-vectorized implementations of: - Max - Min - MaxMagnitude - MinMagnitude - IndexOfMax - IndexOfMin - IndexOfMaxMagnitude - ConvertToHalf (only on .NET Core) - ConvertToSingle (only on .NET Core) - IndexOfMinMagnitude Adds vectorized implementations of: - Sum - SumOfSquares - SumOfMagnitudes - Product - ProductOfSums - ProductOfDifferences Also includes the helpers that'll make it trivial to vectorize Dot. Beyond vectorizing the non-vectorized ones, the vectorized implementations should be improved further, including: - Handling alignment better - Vectorizing the remainder that doesn't fit in a vector rather than falling back to scalar * Cleanup after previous PR, vectorize CosineSimilarity/Dot/L2Normalize/Distance, add tests * Address PR feedback, and fix a few other issues * Fix TensorPrimitives.CosineSimilarity to use vectorized implementations (#92204) * Fixed duplicated code from merge. * New Microsoft.BCL.Numerics package (#91074) * bcl numberics library added * bcl done * added explicit 2.1 target * Minor doc updates * Apply suggestions from code review Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> * fixes from PR comments * minor csproj fixes * fixed ref target frameworks * minor ref csproj updates * minor csproj updates --------- Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> * Microsoft.Bcl.Numerics.Tests: fix restore failure when DotNetBuildFromSource. (#91402) * Microsoft.Bcl.Numerics.Tests: fix restore failure when DotNetBuildFromSource. * Use NetCoreAppCurrent. * Try fix CI test failures. --------- Co-authored-by: Tanner Gooding <tagoo@outlook.com> Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com> Co-authored-by: Stephen Toub <stoub@microsoft.com> Co-authored-by: Tom Deseyn <tom.deseyn@gmail.com>
No description provided.