-
Notifications
You must be signed in to change notification settings - Fork 229
Fast TensorAccessor #1396
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
base: main
Are you sure you want to change the base?
Fast TensorAccessor #1396
Changes from 11 commits
ce679e2
958a187
abe9990
0b20f13
7df8e46
d6865a6
d2857bf
f5e43d7
6235075
1ab3891
19effd7
9ff7866
9e6ba01
46fe73a
d24c709
63aa144
c29592b
c031c61
0fe2f96
ac06718
1ea4266
5d50ebe
e55f01c
d0e5db8
4304c3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -273,3 +273,5 @@ packages/ | |
| /.idea | ||
| /test/TorchSharpTest/exportsd.py | ||
| .vscode/settings.json | ||
| /TestClear | ||
| TestClear/ | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Collections.Generic; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Diagnostics; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Linq; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using System.Runtime.InteropServices; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| using static TorchSharp.PInvoke.NativeMethods; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| namespace TorchSharp.Utils | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -46,10 +47,72 @@ public T[] ToArray() | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (_tensor.ndim < 2) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return (T[])ToNDArray(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| long Cnt = Count; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (_tensor.is_contiguous()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (Cnt == 0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Exception("Invalid"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Exception("Invalid"); | |
| return Array.Empty<T>(); |
Copilot
AI
Feb 13, 2026
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.
CopyContiguous() only handles a handful of element types via Marshal.Copy and silently does nothing for other unmanaged T (e.g., sbyte, ushort, uint, ulong, bool), leaving the destination array unchanged. This should either use a generic unmanaged copy (e.g., Span<T>/MemoryMarshal/Buffer.MemoryCopy) or throw a clear NotSupportedException for unsupported element types.
| if (array is byte[] ba) | |
| Marshal.Copy(_tensor_data_ptr, ba, index, count); | |
| if (array is short[] sa) | |
| Marshal.Copy(_tensor_data_ptr, sa, index, count); | |
| if(array is char[] ca) | |
| Marshal.Copy(_tensor_data_ptr, ca, index, count); | |
| if (array is long[] la) | |
| Marshal.Copy(_tensor_data_ptr, la, index, count); | |
| if (array is float[] fa) | |
| Marshal.Copy(_tensor_data_ptr, fa, index, count); | |
| if (array is int[] ia) | |
| Marshal.Copy(_tensor_data_ptr, ia, index, count); | |
| if (array is double[] da) | |
| Marshal.Copy(_tensor_data_ptr, da, index, count); | |
| bool copied = false; | |
| if (array is byte[] ba) { | |
| Marshal.Copy(_tensor_data_ptr, ba, index, count); | |
| copied = true; | |
| } | |
| if (array is short[] sa) { | |
| Marshal.Copy(_tensor_data_ptr, sa, index, count); | |
| copied = true; | |
| } | |
| if (array is char[] ca) { | |
| Marshal.Copy(_tensor_data_ptr, ca, index, count); | |
| copied = true; | |
| } | |
| if (array is long[] la) { | |
| Marshal.Copy(_tensor_data_ptr, la, index, count); | |
| copied = true; | |
| } | |
| if (array is float[] fa) { | |
| Marshal.Copy(_tensor_data_ptr, fa, index, count); | |
| copied = true; | |
| } | |
| if (array is int[] ia) { | |
| Marshal.Copy(_tensor_data_ptr, ia, index, count); | |
| copied = true; | |
| } | |
| if (array is double[] da) { | |
| Marshal.Copy(_tensor_data_ptr, da, index, count); | |
| copied = true; | |
| } | |
| if (!copied) { | |
| throw new NotSupportedException($"CopyContiguous does not support element type '{typeof(T)}' with array type '{array.GetType()}'."); | |
| } |
Copilot
AI
Feb 13, 2026
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.
CopyTo(T[] array, int arrayIndex = 0, long tensorIndex = 0) no longer honors tensorIndex (it always copies from the start of the tensor). This is a behavioral change from the previous implementation and will produce wrong results for callers that request copying from a non-zero tensor offset.
Copilot
AI
Feb 13, 2026
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.
CopyTo(Span<T> array, int arrayIndex = 0, long tensorIndex = 0) ignores both arrayIndex and tensorIndex (it calls ToArray().CopyTo(array)), and the contiguous fast path still allocates a full intermediate array. This should copy directly from the tensor starting at tensorIndex into array.Slice(arrayIndex, ...) without allocating.
| if (_tensor.is_contiguous()) { | |
| ToArray().CopyTo(array); | |
| return; | |
| } | |
| ToArray().CopyTo(array); | |
| if (arrayIndex < 0 || arrayIndex > array.Length) | |
| throw new ArgumentOutOfRangeException(nameof(arrayIndex)); | |
| long count = Count; | |
| if (tensorIndex < 0 || tensorIndex > count) | |
| throw new ArgumentOutOfRangeException(nameof(tensorIndex)); | |
| if (_tensor.is_contiguous()) { | |
| long availableFromTensor = count - tensorIndex; | |
| int availableInArray = array.Length - arrayIndex; | |
| int maxCount = Math.Min((int)availableFromTensor, availableInArray); | |
| if (maxCount <= 0) | |
| return; | |
| unsafe { | |
| // Source pointer: start at tensorIndex | |
| T* srcPtr = (T*)_tensor_data_ptr; | |
| srcPtr += tensorIndex; | |
| // Destination: slice the span to the desired region | |
| var destSlice = array.Slice(arrayIndex, maxCount); | |
| fixed (T* destPtr = &MemoryMarshal.GetReference(destSlice)) { | |
| long byteCount = (long)maxCount * sizeof(T); | |
| Buffer.MemoryCopy(srcPtr, destPtr, byteCount, byteCount); | |
| } | |
| } | |
| return; | |
| } | |
| // Non-contiguous tensor: copy element-by-element without allocating an intermediate array. | |
| if (tensorIndex >= count || arrayIndex >= array.Length) | |
| return; | |
| int written = 0; | |
| long skipped = 0; | |
| foreach (var value in this) { | |
| if (skipped < tensorIndex) { | |
| skipped++; | |
| continue; | |
| } | |
| int targetIndex = arrayIndex + written; | |
| if (targetIndex >= array.Length) | |
| break; | |
| array[targetIndex] = value; | |
| written++; | |
| if (written >= array.Length - arrayIndex) | |
| break; | |
| } |
Copilot
AI
Feb 13, 2026
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.
CopyFrom(T[] array, int arrayIndex = 0, long tensorIndex = 0) passes arrayIndex into SetValueTensor() as if it were the starting tensor index, and ignores tensorIndex. This changes semantics vs. the old implementation (array offset != tensor offset) and can write into the wrong tensor positions.
Copilot
AI
Feb 13, 2026
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.
CopyFrom(ReadOnlySpan<T> array, int arrayIndex = 0, long tensorIndex = 0) uses the loop variable index as both the tensor linear index and the source span index (array[(int)index]), which will go out of range whenever Count > array.Length and also ignores tensorIndex. It should map source index src = index - tensorIndex + arrayIndex and limit the loop to the min of source/destination remaining lengths.
| for (long index = arrayIndex; index < count; index++) { | |
| long offset = index; | |
| long ptrIndex = 0; | |
| for (long d = shape.Length - 1; d >= 0; d--) // Traverse dimensions in reverse order | |
| { | |
| long i = offset % shape[d]; // Current index in dimension d | |
| ptrIndex += i * strides[d]; // Calculate ptrIndex using strides | |
| offset /= shape[d]; // Move to the next dimension | |
| } | |
| ptr[ptrIndex] = array[(int)index]; | |
| if (arrayIndex < 0 || arrayIndex > array.Length) | |
| throw new ArgumentOutOfRangeException(nameof(arrayIndex)); | |
| if (tensorIndex < 0 || tensorIndex > count) | |
| throw new ArgumentOutOfRangeException(nameof(tensorIndex)); | |
| long maxSrc = array.Length - arrayIndex; | |
| long maxDst = count - tensorIndex; | |
| long elementsToCopy = Math.Min(maxSrc, maxDst); | |
| for (long i = 0; i < elementsToCopy; i++) { | |
| long linearIndex = tensorIndex + i; | |
| long offset = linearIndex; | |
| long ptrIndex = 0; | |
| for (long d = shape.Length - 1; d >= 0; d--) // Traverse dimensions in reverse order | |
| { | |
| long dimIndex = offset % shape[d]; // Current index in dimension d | |
| ptrIndex += dimIndex * strides[d]; // Calculate ptrIndex using strides | |
| offset /= shape[d]; // Move to the next dimension | |
| } | |
| int srcIndex = arrayIndex + (int)i; | |
| ptr[ptrIndex] = array[srcIndex]; |
|
haytham2597 marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||
| // Copyright (c) .NET Foundation and Contributors. All Rights Reserved. See LICENSE in the project root for license information. | ||||
| using System; | ||||
| using System.Collections.Generic; | ||||
| using System.Diagnostics; | ||||
|
||||
| using System.Diagnostics; |
Uh oh!
There was an error while loading. Please reload this page.