Repository navigation
Fix tensor shape consistency and native-width traversal - #135060
tannergooding merged 1 commit into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Added When you commit this breaking change:
Tagging @dotnet/compat for awareness of the breaking change. |
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/area-system-numerics-tensors |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The broad behavioral compatibility changes and unsafe native-width memory traversal require final maintainer review despite substantial regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Standardizes tensor shape semantics for rank-zero empties and leading singleton dimensions while enabling safe native-width traversal.
Changes:
- Aligns broadcasting, reshape, stack, concatenate, equality, slicing, and axis behavior.
- Adds native-width copying and min/max index traversal with overlap and SIMD semantics preserved.
- Expands regression coverage and documents shape/storage contracts.
| File | Description |
|---|---|
TensorTests.cs |
Adds extensive shape, traversal, overlap, and boundary tests. |
TensorSpan_1.cs |
Uses validated view-shape construction. |
TensorShape.cs |
Centralizes normalization, shape equivalence, and empty-view handling. |
TensorOperation.cs |
Adds native-width copy/reduction paths and fixes Any kernels. |
TensorDimensionSpan_1.cs |
Supports effective rank for default empties. |
Tensor.cs |
Applies consistent semantics across tensor operations. |
ReadOnlyTensorSpan_1.cs |
Uses validated view-shape construction. |
ReadOnlyTensorDimensionSpan_1.cs |
Supports effective rank for read-only empty views. |
TensorPrimitives.IIndexOfMinMaxOperator.cs |
Exposes internals needed for shared reduction dispatch. |
README.md |
Documents shape, storage, and native-width behavior. |
Breaking Change DocumentationA breaking change draft has been prepared for this PR. 👉 Click here to create the issue in dotnet/docs The issue body is available in the After creating the issue, please email a link to it to the .NET Breaking Change Notifications alias (dotnetbcn@microsoft.com). /cc @tannergooding Note This documentation was generated with AI assistance from Copilot.
|
#135281) Fixes Issue <!-- Issue Number --> [#133459](#133459), [#134691](#134691), [#128555](#128555), and [#121463](#121463). main PR <!-- Link to PR if any that fixed this in the main branch. --> [#134914](#134914) and [#135060](#135060). # Description <!-- Give a brief summary of the issue and how the pull request is fixing it. --> Backport both merged Tensor fixes to `release/11.0` in their original order: - `137a6701515dd7427e78bc46bcdd520c8d6e3a76`: fix array element-type validation, slice pinning, shape/storage validation, broadcasting, overlap handling, resize zero-fill, enumeration, and numerical operations. Restore efficient dense and contiguous-slice dispatch, including the tangent range-reduction accuracy correction needed by that dispatch. - `be7b579e4230520b5c47cd956513147e13f737a6`: make shape alignment consistent across operations, treat default rank-zero empties as effective shape `[0]` without changing their stored metadata, fix all ten internal `Any` span kernels, and preserve native-width counts, overlap-safe copy ordering, and valid empty-view origins. Both cherry-picks applied without conflicts or backport-specific source changes. The combined implementation, regression tests, README, and API remarks match the merged changes; release-specific compatibility suppressions are left unchanged. No public API is added. # Customer Impact <!-- What is the impact to customers of not taking this fix? --> Without these fixes, incompatible `System.Array` storage can be interpreted as the wrong element type and read beyond its bounds. Pinning a tensor slice can pass the parent's data, rather than the slice's data, to native consumers such as inference engines. Other affected operations can produce incorrect results, mishandle empty or broadcast shapes, omit the required resize zero-fill, or corrupt overlapping output. The existing cropped/gapped `FlattenTo` performance regression and per-element overhead in dense and contiguous-slice operations also remain. Native-backed spans with more than `int.MaxValue` logical elements retain paths that prematurely narrow counts or offsets. Taking both fixes together keeps the initial correctness/dispatch changes paired with the subsequent shape-consistency and native-width corrections. # Regression <!-- Is this fixing a problem that was introduced in the most recent release, ie., fixing a regression? --> Yes, in part, but these are not all newly introduced .NET 11 regressions. The array type-safety issue and cropped `FlattenTo` regression were introduced by the .NET 10 Tensor rewrite in [#114927](#114927) and remain in `release/11.0`. Slice pinning was reproduced with the 10.0.9 package. The remaining changes address existing correctness and consistency defects rather than one recent servicing regression. # Testing <!-- What kind of testing has been done with the fix. --> Local Windows x64 validation on the actual `release/11.0` backport: - `.\build.cmd clr+libs -rc release -lc release` on the unmodified release baseline: passed, zero warnings/errors. - `.\dotnet.cmd build src\libraries\System.Numerics.Tensors\System.Numerics.Tensors.slnx -c Release /p:RuntimeConfiguration=Release` after both cherry-picks: passed, zero warnings/errors. This built the configured `net11.0`, `net10.0`, `netstandard2.0`, and `net462` implementations and both test targets. - `.\dotnet.cmd build src\libraries\System.Numerics.Tensors\tests\System.Numerics.Tensors.Tests.csproj -t:Test -c Release /p:RuntimeConfiguration=Release`: passed in the default configuration. - The same full test command with `/p:testnobuild=true --no-restore` was rerun under each environment override below: all passed. Fresh XML results were checked for the full counts, with no failures, errors, or skips. | Configuration | Environment override | net11.0 passed | net481 passed | | --- | --- | ---: | ---: | | Default intrinsics | None | 6,266 | 200 | | 256-bit vectors | `DOTNET_PreferredVectorBitWidth=256` | 6,266 | 200 | | 128-bit vectors | `DOTNET_PreferredVectorBitWidth=128` | 6,266 | 200 | | AVX2/FMA disabled | `DOTNET_EnableAVX2=0` | 6,266 | 200 | | All intrinsics disabled | `DOTNET_EnableHWIntrinsic=0` | 6,266 | 200 | Coverage includes incompatible arrays, pinning, dense/strided/broadcast layouts, rejection before writes, empty shapes/views, high ranks, native-width logical counts, forced copy/reduction chunk boundaries, NaNs/ties/signed zero, and protected-memory boundaries. Local BenchmarkDotNet ShortRun comparisons used the same built Release runtime with distinct baseline/backport Tensor assemblies on a Ryzen 9 7950X. Assembly loading and the active vector/FMA configuration were verified. The 10 default-intrinsic jobs and four non-FMA tangent jobs all produced measured results: | Scenario | Release baseline | Combined backport | | --- | ---: | ---: | | Dense float Add, 64x16 | 16.37 us | 0.114 us | | Gapped-row float Add, 64x16, strides `[32, 1]` | 16.25 us | 2.05 us | | Gapped-row float FlattenTo, same layout | 7.92 us | 0.875 us | | Float Tan, 4,096 values in `[-1, 1]`, FMA | 1.80 us | 1.72 us | | Float Tan, 4,096 values in `[-50, 50]`, FMA | 1.63 us | 1.53 us | | Float Tan, same small range, no FMA | 4.98 us | 5.08 us | | Float Tan, same wide range, no FMA | 5.60 us | 9.19 us | No managed allocation was measured in these samples. Small timing differences are not claimed as precise gains or proof of performance neutrality; the non-FMA wide-range slowdown is an accuracy tradeoff, discussed below. These checks do not replace the more extensive measurements in the main PRs. Linux/macOS, Arm64, Mono, NativeAOT, and browser/WASM were not locally validated. # Risk <!-- Please assess the risk of taking this fix. Provide details backing up your assessment. --> Low-to-moderate risk. The changes are consistent backports of two fixes already merged on main, applied without conflicts or release-specific source adaptations. The extensive regression tests passed in all five local configurations, and the scope is confined to `System.Numerics.Tensors`. Remaining risk is in the intentional behavior changes and edge cases described below, rather than the patch size. Intentional behavior changes include accepting/rejecting shape combinations consistently, interpreting default empties as effective shape `[0]`, rejecting incompatible array element types and unsupported overlapping layouts before writes, and zero-filling newly exposed resized elements. Consumers relying on the previous inconsistent behavior can be affected. There is no compatibility switch. On hardware without FMA, cancellation-sensitive tangent vectors now use scalar evaluation to avoid inaccurate results. The wide-range local sample increased from 5.60 to 9.19 us, approximately 64%; this is an acknowledged throughput cost for correctness, not a performance-neutral change. FMA-capable hardware retains fused vector range reduction. Breaking-change documentation is already tracked by [dotnet/docs#56290](dotnet/docs#56290) and [dotnet/docs#56305](dotnet/docs#56305). Both currently name .NET 12 Preview 1; their shipment-version metadata needs to be aligned with the approved .NET 11 delivery if this backport is accepted. # Package authoring no longer needed in .NET 9 IMPORTANT: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version. Keep in mind that we still need package authoring in .NET 8 and older versions. > [!NOTE] > This pull request description was generated with assistance from GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
Make tensor shape handling consistent across broadcasting, elementwise operations, copies, equality, reshaping, axis operations, and views:
[0], without changing their storedRank,Lengths, orStrides. Only redundant leading singleton axes can be added or removed; explicit zero axes and non-leading singleton axes remain significant. Stack and concatenate interpret axes using the first input's effective shape.Anyspan kernels to traverse the input length and handle empty inputs correctly.TensorPrimitivesfor dense spans, dense suffixes, and bounded gathered blocks, preserving first-NaN, tie, signed-zero, and magnitude semantics.This includes behavioral compatibility changes, not new public API. Some formerly inconsistent shape combinations are now accepted or rejected according to the shared rules. For example, default empty values can explicitly broadcast to
[2, 0], while binary operations with[0, 2]reject the incompatible trailing dimension. Unsupported overlapping layouts and self-overlapping output layouts throw before writing. The README and affected API remarks document the shape and storage contracts; breaking-change documentation is required after merge.Validation
Performance
Local BenchmarkDotNet comparisons used Windows x64, a Ryzen 9 7950X, and the .NET 11 RC SDK/runtime.
For a native-width broadcast view with shape
[nint.MaxValue / 1024, 1024], strides[0, 1], and a NaN at the end of the first dense suffix,IndexOfMaxmeasured 295 ns with dense-suffix primitive dispatch versus 4,050 ns with an elementwise native-width fallback. This compares dispatch strategies and early NaN termination, not full traversal of a multi-gigabyte allocation or throughput against upstream's length-narrowing path.Concatenation, stack, and dispatch comparisons retained the same allocation counts. Separate-process timings varied substantially and some apparent regressions reversed with run order. A same-process comparison using separately loaded assemblies measured row dispatch at 326 -> 309 ns for rank 2 and 351 -> 356 ns for rank 6, within measurement variability. No general throughput improvement is claimed for the validation simplification or metadata snapshots.
Note
This pull request was authored with assistance from GitHub Copilot.