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

Add center_point_box=1 support in NonMaxSuppression. #3976

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

lpy
Copy link
Contributor

@lpy lpy commented Jan 20, 2025

When center_point_box=1, the supplied boxes come with a format of [x_center, y_center, width, height], this patch converts the format into [x1, y1, x2, y2] format before they are consumed.

The e2e test is added in nod-ai/SHARK-TestSuite#436

@lpy
Copy link
Contributor Author

lpy commented Jan 20, 2025

@vivekkhandelwal1 @jinchen62 Looks like I don't have permission to add reviewers, could you please take a look?

Copy link
Collaborator

@jinchen62 jinchen62 left a comment

Choose a reason for hiding this comment

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

Thanks for making this! LGTM except for some minor changes.

Could you test iree e2e test https://github.com/iree-org/iree-test-suites/tree/main/onnx_ops/onnx/node/generated/test_nonmaxsuppression_center_point_box_format? If you haven't used it, instruction is here https://github.com/iree-org/iree-test-suites/blob/main/onnx_ops/README.md.

lib/Conversion/TorchOnnxToTorch/DefaultDomainGtoP.cpp Outdated Show resolved Hide resolved
lib/Conversion/TorchOnnxToTorch/DefaultDomainGtoP.cpp Outdated Show resolved Hide resolved
lib/Conversion/TorchOnnxToTorch/DefaultDomainGtoP.cpp Outdated Show resolved Hide resolved
lib/Conversion/TorchOnnxToTorch/DefaultDomainGtoP.cpp Outdated Show resolved Hide resolved
lib/Conversion/TorchOnnxToTorch/DefaultDomainGtoP.cpp Outdated Show resolved Hide resolved
lib/Conversion/TorchOnnxToTorch/DefaultDomainGtoP.cpp Outdated Show resolved Hide resolved
When center_point_box=1, the supplied boxes come with a format of
[x_center, y_center, width, height], this patch converts the format into
[x1, y1, x2, y2] format before they are consumed.
@lpy lpy force-pushed the nonmaxsuppression branch from d7603ab to 7487082 Compare January 22, 2025 22:16
@lpy
Copy link
Contributor Author

lpy commented Jan 22, 2025

Thanks for the guidance. Comments addressed. I have also confirmed test_nonmaxsuppression_center_point_box_format in iree-test-suites when --iree-input-demote-f64-to-f32=false is removed.

PTAL!

Copy link
Collaborator

@jinchen62 jinchen62 left a comment

Choose a reason for hiding this comment

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

Cheers!

@jinchen62 jinchen62 merged commit 2564d7a into llvm:main Jan 22, 2025
3 checks passed
@lpy lpy deleted the nonmaxsuppression branch January 23, 2025 00:00
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.

2 participants