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

@copilot do other methods have the same or similar problem? e.g. IndexOf*, ResizeTo, Fill*, etc.? Please investigate and fix any other similar problems in the whole library.

@copilot do other methods have the same or similar problem? e.g. IndexOf*, ResizeTo, Fill*, etc.? Please investigate and fix any other similar problems in the whole library.

Investigated all methods in the library that use MemoryMarshal.CreateSpan/CreateReadOnlySpan with LinearLength without density checks. Found and fixed the following in commit 99ba364:

• FillGaussianNormalDistribution / FillUniformDistribution — wrote to raw buffer including gaps; now iterates logical elements via enumerator for non-dense spans
• Resize (Tensor overload) — read from raw buffer of non-dense sliced tensors; now enumerates logical elements
• ResizeTo (ReadOnlyTensorSpan overload) — read/wrote raw buffer for both source and destination; now uses element-by-element copy for non-dense spans
• IndexOfMax / IndexOfMin / IndexOfMaxMagnitude / IndexOfMinMagnitude — searched raw buffer including gaps; now flattens to temp array for non-dense spans

Methods using TensorOperation.Invoke (like Sum, Product, Average, Equals, element-wise ops) are already correct — they iterate via AdjustToNextIndex. Also noted ConcatenateOnDimension has a similar issue with non-dense destinations but that requires a larger rewrite; proposed as follow-up work.

@copilot invoke the code-review skill and post your analysis/results as a comment on this PR

@copilot invoke the code-review skill and post your analysis/results as a comment on this PR

🤖 Copilot Code Review — PR #123681

Holistic Assessment

Motivation: The PR fixes a real correctness bug. Multiple Tensor methods used MemoryMarshal.CreateSpan(ref _reference, (int)_shape.LinearLength) which includes gap/padding elements for non-dense (strided) tensor spans. This caused SequenceEqual to return false for logically-equal non-dense tensors, Fill* to write to gap positions, IndexOf* to search gap positions, and Resize* to copy gap data. The original tests for SequenceEqual were commented out, confirming this was a known gap.

Approach: The IsDense check with enumerator fallback is the right pattern — it's consistent with how TensorOperation.Invoke already handles non-dense iteration throughout the library. Dense fast paths are preserved. The IndexOf* methods use FlattenTo + temp array, which is correct and simple.

Summary: ⚠️ Needs Human Review. The core code changes are correct, but I have two concerns a human reviewer should evaluate: (1) a behavioral inconsistency in SequenceEqual between the dense and non-dense paths, and (2) whether ConcatenateOnDimension (line 240) should be addressed in this PR or is truly safe to defer.

Detailed Findings

⚠️ SequenceEqual — EqualityComparer<T>.Default vs IEquatable<T>.Equals behavioral difference

In SequenceEqual (line 1787), the non-dense path uses EqualityComparer<T>.Default.Equals(enumerator1.Current, enumerator2.Current), while the dense path delegates to Span<T>.SequenceEqual which calls IEquatable<T>.Equals directly. For T : IEquatable<T>?, these should behave the same for concrete types, but there's a subtle difference: EqualityComparer<T>.Default goes through a virtual dispatch that Span.SequenceEqual avoids. More importantly, if T is null (since the constraint allows IEquatable<T>?), the behavior may differ — EqualityComparer<T>.Default.Equals(null, null) returns true, while Span.SequenceEqual uses IEquatable<T>.Equals which would NRE on null.

This is likely not a practical issue since tensor element types won't be null in practice, but for consistency and to avoid the virtual dispatch overhead, consider using EqualityComparer<T>.Default.Equals in the non-dense path or document why the difference is acceptable. Low confidence concern — a human should evaluate whether this matters.

✅ Correctness — All 10 LinearLength sites fixed

Verified by grep: no remaining MemoryMarshal.Create*Span calls use _shape.LinearLength. All MemoryMarshal span creations in the changed methods now use FlattenedLength inside IsDense guards. The remaining LinearLength usages are in constructor calls (passing linear length as a size parameter to TensorSpan/ReadOnlyTensorSpan constructors), which is correct — those need the full buffer extent.

✅ Correctness — SequenceEqual logic is sound

The precondition check (FlattenedLength match + Lengths.SequenceEqual) correctly handles shape mismatches. The dense fast path via Span.SequenceEqual is correct and unchanged in behavior for dense tensors. The non-dense Enumerator path correctly walks only logical elements. The TensorSpan overload correctly delegates via cast.

✅ Correctness — Fill, Resize, IndexOf methods

• FillGaussianNormalDistribution / FillUniformDistribution: Correctly iterate only logical elements for non-dense destinations. Gap positions remain untouched.
• Resize: Non-dense Tensor inputs correctly flattened via enumerator. Output is always a fresh dense array, so no destination density issue.
• ResizeTo: Both source and destination density checked. Non-dense path copies via dual enumerators.
• IndexOf*: Non-dense fallback uses FlattenTo + TensorPrimitives.IndexOf*, returning a flattened logical index. Correct.

⚠️ ConcatenateOnDimension (line 240) — still uses totalLength with raw _reference

ConcatenateOnDimension creates MemoryMarshal.CreateSpan(ref destination._reference, (int)totalLength) where totalLength is the sum of source FlattenedLength values. If the destination is non-dense, this span length doesn't account for the destination's stride gaps, so writes via TensorOperation.CopyTo into dstSpan would write to incorrect positions. The PR description notes this as a follow-up, but since the inputs are Tensor<T> (always backed by a contiguous array), the sources are always dense. However, the destination TensorSpan<T> can be non-dense. A human should confirm whether this is exploitable in practice or safe to defer.

✅ Style — Braces added

All one-line for/while/if/else bodies have braces as requested.

💡 Tests — Reasonable coverage, minor duplication

The NonDenseTensorData() provider covers 4 configurations (2×2, 2×3, 3×2, 1D with varying strides/gaps). The tests verify core scenarios: same logical elements with different gaps, non-dense vs dense, differing elements, and the TensorSpan overload. The Fill* tests verify gap positions remain zero. The IndexOf* tests verify correct logical indices.

The offset-computation logic is duplicated across FillGaussianNormalDistributionNonDenseTests and FillUniformDistributionNonDenseTests (~15 lines each). This could be extracted to a helper, but it's a minor concern for test code.

💡 IndexOf* fallbacks — Allocation of T[FlattenedLength]

The IndexOf*Fallback methods allocate new T[x.FlattenedLength] which could be large for big tensors. ArrayPool<T>.Shared would be more appropriate for BCL code. Not blocking since correctness is the priority here and the pattern is simple, but worth noting for follow-up.

⚠️ ConcatenateOnDimension (line 240) — still uses totalLength with raw _reference

ConcatenateOnDimension creates MemoryMarshal.CreateSpan(ref destination._reference, (int)totalLength) where totalLength is the sum of source FlattenedLength values. If the destination is non-dense, this span length doesn't account for the destination's stride gaps, so writes via TensorOperation.CopyTo into dstSpan would write to incorrect positions. The PR description notes this as a follow-up, but since the inputs are Tensor<T> (always backed by a contiguous array), the sources are always dense. However, the destination TensorSpan<T> can be non-dense. A human should confirm whether this is exploitable in practice or safe to defer.

This is still an issue. ConcatenateOnDimension will write to "unobserved" elements in ud in demonstration:

var u0 = Tensor.CreateFromShape<int>([2, 3]);
var ud = u0.AsTensorSpan().Slice([0..1, 1..2]);
var u1 = Tensor.Create([1, 2]);
var u2 = Tensor.Create([3, 4]);
Tensor.ConcatenateOnDimension(-1, [u1, u2], ud);
Console.WriteLine($"[{u0[0, 0]} {u0[0, 1]} {u0[0, 2]}]");
Console.WriteLine($"[{u0[1, 0]} {u0[1, 1]} {u0[1, 2]}]");

@copilot please address the review feedback in #124225 (comment)

@copilot try again

@copilot try again

Implemented the follow-up fixes and retried with fresh validation in ec7f3ab. I added a new regression test for ConcatenateOnDimension with a non-dense destination span (the remaining gap from prior review feedback), then reran ./build.sh clr+libs -rc release and dotnet build /t:test src/libraries/System.Numerics.Tensors/tests/System.Numerics.Tensors.Tests.csproj (5401 tests, 0 failures). I also reran the code-review pass on the updated head and it reported no review comments.