-
Notifications
You must be signed in to change notification settings - Fork 6.7k
[v1.x] ONNX contrib_box_nms #19755
[v1.x] ONNX contrib_box_nms #19755
Conversation
|
Hey @Zha0q1 , Thanks for submitting the PR
CI supported jobs: [edge, clang, miscellaneous, windows-cpu, windows-gpu, website, centos-cpu, unix-gpu, unix-cpu, sanity, centos-gpu] Note: |
| out_names = list() | ||
| for name in sym.list_outputs(): | ||
| if name.endswith('_output'): | ||
| if re.search('.*_output$', name): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use a regex here? endswith would be more efficient.
| if name.endswith('_output'): | ||
| if re.search('.*_output$', name): | ||
| out_names.append(name[:-len('_output')]) | ||
| elif re.search('.*_output[0-9]$', name): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using a regex here, you could just check for output in the string, like:
if name[-len('output0'):-1] == 'output':
|
@josephevans thanks for reviewing, will change. Additionally I need to fix the graph linking issue when a node have > 1 output, thus adding [wip] |
josephevans
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
box nms op
currently does not support
id_indexandbackground_id-- we can add those later if requested