Skip to content

Consider changing output type of ArgMax/Argmin to int32, or allow passing output_type #653

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 Apr 23, 2024 · 2 comments · Fixed by #730
Closed

Comments

@philloooo
Copy link
Contributor

philloooo commented Apr 23, 2024

Currently WebNN specifies ArgMax/Min returns int64.

  • CoreML supported return type is int32 or uint16.
  • tflite supports int32, int64.
  • dml supports int32, int64, uint32, uint64.

Returning int64 can't be emulated on CoreML:

  • the cast operator only supports casting to fp16, fp32, i32, bool.
  • None of the operations support int64 as input(so the output of argMax/argMin can't be connected to the next layer if it's int64)
  • Graph output doesn't support i64. Only fp16, fp32, i32 is supported.

Given int32 is the intersection across these backends, I see two options to make it work:
a. Update argMin/Max to always output int32
b. On the spec level, allow passing a param of output_type. And allow probing the supported data types for this param.

@philloooo philloooo changed the title Consider changing output type of ArgMax/Argmin to int32, or allow passing output_type for ArgMax/ArgMin Consider changing output type of ArgMax/Argmin to int32, or allow passing output_type Apr 23, 2024
@philloooo
Copy link
Contributor Author

Following up from the working group meeting, I am happy to explore option 2 if @fdwr can provide examples on when int64 is useful.

I didn't fully understand your gather example, because I actually don't understand why indices allow both int64 and uint32. The indices should point to valid indices that's within MLOperand's dimensions right? And the dimensions are uint32...

@fdwr
Copy link
Collaborator

fdwr commented Jul 18, 2024

Mirroring this table here too for historical reference:

API uint32 int32 uint64 int64 Link
DML uint32 int32 uint64 int64 DML_ARGMIN_OPERATOR_DESC
TF output_type - int32 - int64 tf.math.argmin
CoreML - int32 - - reduction.reduce_argmin
MLIR TOSA uint32 - - - tosa.argmax "signless integer"
ONNX - - - int64 ArgMin
PyTorch - - - int64 torch.argmin
NumPy - - - int64 numpy.argmin

# 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.

4 participants