Skip to content

Conversation

@rok
Copy link
Member

@rok rok commented Oct 4, 2023

Rationale for this change

We want to add VariableShapeTensor extension type definition for arrays containing tensors with variable shapes.

What changes are included in this PR?

This adds a C++ implementation.

Are these changes tested?

Yes.

Are there any user-facing changes?

This adds a new extension type C++.

@AlenkaF
Copy link
Member

AlenkaF commented Nov 16, 2023

Hi @rok! It would be great to have this in 15.0.0 (added the milestone for visibility). Do you think you have enough bandwidth to work on it at the moment?

@rok
Copy link
Member Author

rok commented Nov 16, 2023

@AlenkaF yes I'd very much like to continue working on this, I was waiting for the release before chasing reviewers :D
I'll rebase tonight and would very much appreciate reviews afterwards.

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Nov 23, 2023
@github-actions github-actions bot added awaiting change review Awaiting change review awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Nov 28, 2023
@rok
Copy link
Member Author

rok commented Nov 12, 2025

As I understand it, the FixedShapeTensorArray has less need for a Large version, because the FixedSizeListType does not hold an offset int32 array like the ListType does. So 1 array can hold many tensors, as long as each is less than 2^31 elements. The VariableShapeTensorArray as it's currently documented can hold at most 2^31 elements in all tensors in the array combined, so that limit is hit earlier. Also, there doesn't seem to be a LargeFixedSizeListType, which has an int64 size?

@pimdh Yes, that seems right.
Now that some time has passed would you say there's still need for VariableShapeTensorArray with LargeList for data? We would need to amend the extension type or introduce LargeVariableShapeTensorArray.

@rok
Copy link
Member Author

rok commented Nov 12, 2025

Given that there is some interest in using VariableShapeTensor and that it was already implemented in rust it might make sense to merge this PR. @pitrou would you happen to have time to take another look here?

@pimdh
Copy link

pimdh commented Nov 12, 2025

@pimdh Yes, that seems right.
Now that some time has passed would you say there's still need for VariableShapeTensorArray with LargeList for data? We would need to amend the extension type or introduce LargeVariableShapeTensorArray.

No, as I didn't yet run into any limits of the int32 arrays, I have no need for the large version.
Appreciate your work on this! Would love to replace our internal version with an official one.
Also, then others, like the Ray library, can use the standardised version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++][Python] Add VariableShapeTensor implementation

6 participants