Skip to content
This repository has been archived by the owner on Apr 18, 2023. It is now read-only.

[doc] Add ARGMAX op. #960

Merged
merged 3 commits into from Oct 23, 2019
Merged

[doc] Add ARGMAX op. #960

merged 3 commits into from Oct 23, 2019

Conversation

BruceDai
Copy link
Contributor

@BruceDai BruceDai commented Sep 3, 2019

@huningxin @fujunwei These two new ops were referring to ARGMAX and ARGMIN of NN API, please take a review, thanks

docs/api.md Outdated
@@ -62,6 +62,8 @@ interface NeuralNetworkContext {
const long MAXIMUM = 65;
const long ATROUS_CONV_2D = 10003;
const long ATROUS_DEPTHWISE_CONV_2D = 10004;
const long ARGMAX = 10005;
const long ARGMIN = 10006;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please align the constant value to NNAPI?

const long ARGMAX = 39;
const long ARGMIN = 40;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated them by the new patch.

src/nn/Enums.js Outdated
* Outputs:
* * 0: An (n - 1)-D TENSOR_INT32 tensor.
*/
ARGMAX: 10005,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

src/nn/Enums.js Outdated
* Outputs:
* * 0: An (n - 1)-D TENSOR_INT32 tensor.
*/
ARGMIN: 10006,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@BruceDai
Copy link
Contributor Author

BruceDai commented Sep 3, 2019

ONNX also supports ARGMAX and ARGMIN .

@huningxin
Copy link
Contributor

ONNX also supports ARGMAX and ARGMIN .

Good pointers. Could you please open an issue and document the difference between NNAPI and ONNX for argmax and argmin? Thanks.

@BruceDai BruceDai changed the title [doc] Add ARGMAX and ARGMIN ops. [doc] Add ARGMAX op. Oct 22, 2019
@huningxin
Copy link
Contributor

Looks good to me. Thanks much!

@huningxin huningxin merged commit 56a62bb into intel:master Oct 23, 2019
@BruceDai BruceDai mentioned this pull request Mar 17, 2020
@BruceDai BruceDai deleted the add_argmax branch June 4, 2020 02:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants