Conversation
Using `IEnumerable<T>` and associated LINQ extensions for numerical processing is not appropriate at this level of the stack. The only remaining use is where the input provided by the user is already an `IEnumerable<T>` and `ToArray` is used to get the required `T[]`, which is fine.
ghost
added
the
|
Tagging subscribers to this area: @dotnet/area-system-numerics-tensors |
|
@tannergooding or @michaelgsharp, could one of you help review this? Thanks. |
| array == null ? | ||
| [0] : | ||
| TensorSpanHelpers.FillLengths(array.Rank <= TensorShape.MaxInlineRank ? | ||
| stackalloc nint[array.Rank] : |
There was a problem hiding this comment.
nit: stack-allocating dynamic lengths has poor performance due to additional checks and guards required. It would be much better to do stackalloc nint[MaxInlineRank].Slice(array.Rank)
There was a problem hiding this comment.
Same general comment applies to other stackallocs as well.
There was a problem hiding this comment.
nit: stack-allocating dynamic lengths has poor performance due to additional checks and guards required. It would be much better to do stackalloc nint[MaxInlineRank].Slice(array.Rank)
There are dozens of such existing stackallocs across the library. If we want to replace them with that pattern (with Slice(0, array.Rank), let's do it separately.
| /// Get a string representation of the tensor. | ||
| /// </summary> | ||
| private string ToMetadataString() | ||
| private void ToMetadataString(StringBuilder sb) |
There was a problem hiding this comment.
Not really for this PR, but would it be a good idea to have this setup around an interpolated string handler instead long term?
There was a problem hiding this comment.
We could use something like a ValueStringBuilder or DefaultInterpolatedStringHandler to avoid the heap allocation.
Using `IEnumerable<T>` and associated LINQ extensions for numerical processing is not appropriate at this level of the stack. The only remaining use is where the input provided by the user is already an `IEnumerable<T>` and `ToArray` is used to get the required `T[]`, which is fine.
2 participants
Using
IEnumerable<T>and associated LINQ extensions for numerical processing is not appropriate at this level of the stack. The only remaining use after this PR is where the input provided by the user is already anIEnumerable<T>andToArrayis used to get the requiredT[], which is fine.