Skip to content

Eager mode: Argmax and fixup max and min.#11861

Merged
WilBrady merged 3 commits intomicrosoft:masterfrom
WilBrady:wilbrady/investigating
Jun 23, 2022
Merged

Eager mode: Argmax and fixup max and min.#11861
WilBrady merged 3 commits intomicrosoft:masterfrom
WilBrady:wilbrady/investigating

Conversation

@WilBrady
Copy link
Contributor

@WilBrady WilBrady commented Jun 15, 2022

Description: Adding argmax support to eager mode, correcting min and max behavior, and adding tests.

Motivation and Context

  • Working to expand eager mode support. In this case running a very simple model showed argmax support was needed so added that. In the testing process noticed min and max functions returned the wrong dimensions so corrected that behavior and updated the test.

@WilBrady WilBrady force-pushed the wilbrady/investigating branch from 9477c3b to f77c3ce Compare June 15, 2022 21:03
@WilBrady WilBrady force-pushed the wilbrady/investigating branch from c303f81 to 0a920e5 Compare June 16, 2022 21:12
@WilBrady WilBrady force-pushed the wilbrady/investigating branch 2 times, most recently from ca8a35c to ee2c025 Compare June 22, 2022 14:22
@WilBrady WilBrady marked this pull request as ready for review June 22, 2022 18:02
@WilBrady WilBrady force-pushed the wilbrady/investigating branch from ee2c025 to 29e7acc Compare June 22, 2022 18:11
@WilBrady WilBrady force-pushed the wilbrady/investigating branch from 29e7acc to b887221 Compare June 22, 2022 18:17
@WilBrady
Copy link
Contributor Author

@souptc - please review and approve or provide feedback. Thank you.


// generator also needs to do this to handle the out param!
out = aten_tensor_from_ort(
std::move(ort_outputs_0_ArgMax[0]),
Copy link
Member

Choose a reason for hiding this comment

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

nit: could we check the indentation here (and some of the places above)?

Copy link
Member

@jamill jamill left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just left a comment about whether we can improve the indentation for the manual code. Thanks!

@WilBrady WilBrady merged commit fa7f80c into microsoft:master Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants