Add outputDataType to argmin/argmax#730
Conversation
huningxin
left a comment
There was a problem hiding this comment.
Returns: an {{MLOperand}}. The N-D tensor of the reduced shape. The values must be of type {{MLOperandDataType/"int64"}} in the range [0, N-1] where N is the size of the input dimension specified by axis.
This prose needs to be updated as well. What would be the range for int32 output value?
|
@huningxin i think it would still be [0, N-1] range? The dimensions are of type uint32_t so it could exceed int32_t range, but I don't think in reality we have tensors that big? this also exposes another issue - both CoreML and tflite backends actually use int32_t for tensor dimensions so if we have tensor that needs to use that extra bit from uint32_t they won't work on these backends. Should we actually consider use int32_t for dimensions? |
Yes, then would it be [0, min(N-1, MAX_INT)] range? And should we add a validation step to ensure the size of the dimension being reduced can be indexed by that range?
I haven't seen any tensor of a model has such big dimension.
Yes, AFAIK, the current Chromium TFLite prototype reports error when the dimension size is too large for int32: ToSignedDimensions().
FYI, #279 replaces the int32_t dimension with uint32_t because there are no use cases for negative values. We may want to reconsider that given it is not widely supported by currently targeting backends, CoreML / TFLite. |
While we closed issue #456, this table is still relevant: #456 (comment) (and the max for Core ML is 5D). The dimensions will always be non-negative and within |
Apologies, that table is referring to max rank while we're talking about the max dimension here
+1, though we should consider [0, |
### Description WebNN spec introduces a new option: `outputDataType` to `argMax` and `argMin` ops, it's default value is `int32`, we should explicitly set it to `int64` for WebNN EP. Spec CR: "Add outputDataType to argmin/argmax" webmachinelearning/webnn#730
|
@huningxin I've created a separate issue #734 for int32 vs uint32. |
SHA: 4a2b8ca Reason: push, by fdwr Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
### Description WebNN spec introduces a new option: `outputDataType` to `argMax` and `argMin` ops, it's default value is `int32`, we should explicitly set it to `int64` for WebNN EP. Spec CR: "Add outputDataType to argmin/argmax" webmachinelearning/webnn#730
### Description WebNN spec introduces a new option: `outputDataType` to `argMax` and `argMin` ops, it's default value is `int32`, we should explicitly set it to `int64` for WebNN EP. Spec CR: "Add outputDataType to argmin/argmax" webmachinelearning/webnn#730
fixes #653
Add outputDataType to
MLArgMinMaxOptionsand defaults toint32. GivenopSupportLimitsis not added to spec yet, no validation steps is done tooutputDataTyperight now.@fdwr @huningxin
Preview | Diff