Skip to content
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

[API Proposal]: Tensor.LengthsAsInt/Long #111963

Open
michaelgsharp opened this issue Jan 29, 2025 · 3 comments
Open

[API Proposal]: Tensor.LengthsAsInt/Long #111963

michaelgsharp opened this issue Jan 29, 2025 · 3 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Numerics.Tensors untriaged New issue has not been triaged by the area owner

Comments

@michaelgsharp
Copy link
Member

Background and motivation

Our Tensor class has our Lengths stored as nints. This allows us to support multi-dimensional tensors that have more total elements than Int.MaxValue. Unfortunately, this also makes it harder to do integrations with other frameworks as most (if not all) other frameworks use either ints or longs to store the lengths of their dimensions. This means our users have to write their own conversion whenever they need to use this.

API Proposal

namespace System.Numerics.Tensors;

public interface IReadOnlyTensor<TSelf, T> : IEnumerable<T>
        where TSelf : IReadOnlyTensor<TSelf, T>
{
    ReadOnlySpan<int> IntLengths { get; }
    // Allocating version
    ReadOnlySpan<int> GetIntLengths();
    // Non-allocating version
    void GetIntLengths(scoped ReadOnlySpan<int> destination);

    ReadOnlySpan<long> LongLengths { get; }
    // Allocating version
    ReadOnlySpan<long> GetLongLengths();
    // Non-allocating version
    void GetLongLengths(scoped ReadOnlySpan<long> destination);
}

API Usage

Tensor<int> tensor = Tensor.Create<int>([1, 2, 3, 4], [2, 2]);
int[] intLengths = tensor.IntLengths;

long[] longLengths = tensor.LongLengths;

Alternative Designs

This could either be stored as actual values or computed each time. I think it would be better to compute it each time, especially in the case of TensorSpan where we are being stack memory conscience, rather than store the converted values.

Risks

This would be a new api in a preview object, so the risks are very minimal.

@michaelgsharp michaelgsharp added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 29, 2025
@michaelgsharp
Copy link
Member Author

@tannergooding

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 29, 2025
Copy link
Contributor

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

@tannergooding
Copy link
Member

nit: We use the BCL names, not language names, where relevant in most scenarios. So this should likely be Int32/Int64 not Int/Long (acknowledging that Array deviated and uses GetLongLengths())

It's also likely worth calling out that GetLongLengths() doesn't have to allocate on 64-bit, which is why it can be beneficial to return a ROSpan and not simply long[]. The same with GetIntLengths() on 32-bit. API review may decide that having an allocating version is better so that users aren't forced to double allocate if they want to store the data outside the scope of the method.

I expect we don't want the properties, since they may have non-trivial cost and that goes against the design guidelines. I expect the ones that take a destination span should be named bool TryGet and have an out int valuesWritten instead of simply returning void, to likewise match the typical pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Numerics.Tensors untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

2 participants