Skip to content

why gather indices only accept uint32 or int64? #675

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
philloooo opened this issue May 3, 2024 · 2 comments · Fixed by #685
Closed

why gather indices only accept uint32 or int64? #675

philloooo opened this issue May 3, 2024 · 2 comments · Fixed by #685

Comments

@philloooo
Copy link
Contributor

Is there a reason that it doesn't accept int32?

Also I am not sure why it needs int64, we won't have dimensions that large?

Asking because CoreML only supports int32, int16, uint16, int8, uint8 . And at model boundary, it only supports int32, float32, float16. So there is no way to create a builder.input with uint32 or int64.

@huningxin
Copy link
Contributor

huningxin commented May 10, 2024

Thanks @philloooo for CoreML investigation.

Asking because CoreML only supports int32, int16, uint16, int8, uint8 .

DML gather indices supports INT64, INT32, UINT64, UINT32

TFLite gather supports int32 and int64.

int32 is the intersection. I think WebNN spec should support it.

Also I am not sure why it needs int64, we won't have dimensions that large?

I guess the reason is to support the [-N, 0) range of the [-N, N) range that the spec mentions the indices value:

must be in the range -N (inclusive) to N (exclusive) where N is the size of the input dimension indexed by options.axis, and a negative index means indexing from the end of the dimension.

@fdwr , any insights?

@fdwr
Copy link
Collaborator

fdwr commented May 10, 2024

Is there a reason that it doesn't accept int32?

Oversight? Adding int32 makes sense to me.

Also I am not sure why it needs int64

Primarily because multiple of the transformer models we looked at used that as the native data type for indices (even if, yes, they didn't access more than 4 billion elements), and that option offered the least friction to enable existing models without a bunch of inefficient casts on the caller. When possible, routing data through without copies/casts is 👍👍.

I wouldn't hold ourselves to the lowest common denominator of all possible backends when considering data type support, just as the lack of ASTC support on various hardware in didn't stop WebGPU from adding ASTC as a pixel format. I think we'll need some optionality (not too much, because then the spec loses its value and is hard to test, but some at broadly granular levels), and when guiding WebNN, we should consider not just the current limitations of backends (where CoreML curiously lacks int64, whereas every other newer Apple ML library has it: BNNSDataTypeInt64, MPSDataType.int64, mx.int64) but also what we'd like to see and what the future may hold to shape the ecosystem.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants