Skip to content

[Relax][ONNX] Set max_output_boxes_per_class default value to 0 for NonMaxSuppression#19547

Open
cchung100m wants to merge 3 commits into
apache:mainfrom
cchung100m:issue-19544
Open

[Relax][ONNX] Set max_output_boxes_per_class default value to 0 for NonMaxSuppression#19547
cchung100m wants to merge 3 commits into
apache:mainfrom
cchung100m:issue-19544

Conversation

@cchung100m
Copy link
Copy Markdown
Contributor

Hi Committers,

This PR is trying to fix issues #19544. Any suggestions would be appreciated if you are available.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the default value for max_output_boxes_per_class from 100 to 0 in the ONNX frontend to align with the ONNX specification. Feedback suggests that AllClassNMS._impl_v1 should also be updated to use 0 as the default value to maintain consistency across the codebase.

Comment on lines +4760 to +4762
max_output_boxes_per_class = 0 # Default value
else:
max_output_boxes_per_class = 100 # Default value
max_output_boxes_per_class = 0 # Default value
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The default value for max_output_boxes_per_class is updated to 0 here to align with the ONNX specification. However, AllClassNMS._impl_v1 (lines 4849 and 4851) still uses 100 as the default value. To maintain consistency across the ONNX frontend, AllClassNMS should also be updated to use 0 as the default value.

@cchung100m cchung100m marked this pull request as ready for review May 12, 2026 16:35
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.

1 participant