Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[ONNX] Fix bugs for the onnx.OneHot operator #3334

Merged
merged 1 commit into from
May 22, 2024

Conversation

angelz913
Copy link
Collaborator

@angelz913 angelz913 commented May 13, 2024

This commit fixes the bugs for the onnx.OneHot operator by:

  1. Converting negative indices to non-negative indices
  2. Handling both int and float types for off and on values
  3. Using the correct result type

It also includes a new unit test.

@angelz913 angelz913 requested review from rsuderman, vivekkhandelwal1 and renxida and removed request for rsuderman and vivekkhandelwal1 May 13, 2024 19:56
@angelz913 angelz913 force-pushed the one-hot-remove-negative-indices branch from 7716da9 to 254a3bd Compare May 14, 2024 14:17
@angelz913 angelz913 force-pushed the one-hot-remove-negative-indices branch 3 times, most recently from 07ce073 to ddda063 Compare May 21, 2024 20:12
@angelz913 angelz913 changed the title [ONNX] Convert negative indices to non-negative for onnx.OneHot operator [ONNX] Fix bugs for the onnx.OneHot operator May 21, 2024
@angelz913 angelz913 force-pushed the one-hot-remove-negative-indices branch 3 times, most recently from 80fa182 to 2e95d1b Compare May 21, 2024 20:37
This commit fixes bugs for the onnx.OneHot operator by:

1. Converting negative indices to non-negative indices
2. Handling both int and float types for off and on values
3. Using the correct result type

It also includes a new unit test.
@angelz913 angelz913 force-pushed the one-hot-remove-negative-indices branch from 2e95d1b to d520770 Compare May 21, 2024 22:49
Copy link
Collaborator

@vivekkhandelwal1 vivekkhandelwal1 left a comment

Choose a reason for hiding this comment

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

LGTM

@kuhar kuhar merged commit 52be4bd into llvm:main May 22, 2024
3 checks passed
kuhar pushed a commit that referenced this pull request May 22, 2024
This PR fixes the bugs for `Torch::AtenOneHotOp` by:

1) Using `Torch::kUnknownSize` as the default value for `numClasses` in
   the pattern matching stage in `DecomposeAtenOneHotOp`
2) Adding `AtenIntScalarOp` to the patterns in `TorchToArith`
3) Handling both `int` and `float` types for `off` and `on` values in
`TorchOnnxToTorch` conversion

It also includes:

1) A new test in `TorchToArith/basic.mlir`, for `torch.aten.Int.Scalar`,
and
2) A new test in `decompose-complex-ops.mlir`, for `torch.aten.one_hot`

**Dependencies**

This PR is dependent on #3334.
BaneTrifa pushed a commit to BaneTrifa/torch-mlir that referenced this pull request May 24, 2024
This commit fixes the bugs for the `onnx.OneHot` operator by:

1) Converting negative indices to non-negative indices
2) Handling both `int` and `float` types for `off` and `on` values
3) Using the correct result type

It also includes a new unit test.
BaneTrifa pushed a commit to BaneTrifa/torch-mlir that referenced this pull request May 24, 2024
This PR fixes the bugs for `Torch::AtenOneHotOp` by:

1) Using `Torch::kUnknownSize` as the default value for `numClasses` in
   the pattern matching stage in `DecomposeAtenOneHotOp`
2) Adding `AtenIntScalarOp` to the patterns in `TorchToArith`
3) Handling both `int` and `float` types for `off` and `on` values in
`TorchOnnxToTorch` conversion

It also includes:

1) A new test in `TorchToArith/basic.mlir`, for `torch.aten.Int.Scalar`,
and
2) A new test in `decompose-complex-ops.mlir`, for `torch.aten.one_hot`

**Dependencies**

This PR is dependent on llvm#3334.
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.

None yet

4 participants