masahi edited a comment on pull request #7796:
URL: https://github.com/apache/tvm/pull/7796#issuecomment-813639924


   > awesome. it would be great to also discuss the naming of the API(by 
checking with the naming in the existing libraries e.g. TF, Pytorch). e.g. 
shall we call it `combined_non_max_suppression` as per TF's naming convention?
   
   `combined_non_max_suppression` was also the name I started with, but I 
didn't particularly like it because it doesn't tell what exactly is "combined". 
   
   Moreover, in TF [the other NMS 
op](https://www.tensorflow.org/api_docs/python/tf/image/non_max_suppression) is 
 the standard, single class NMS, so for them combined or not is really single 
vs multiclass distinction. In contrast, our existing NMS also does multiclass 
NMS where a single box is associated with a single class. This variant of 
multiclass NMS has a different notion of "multiclass" and can support PyTorch 
and MXNet. So I don't think `combined_nms` is a good choice for us.
   
   `all_class_non_maximum_surpression` is also what TensorRT calls it 
https://github.com/NVIDIA/TensorRT/blob/master/plugin/common/kernels/allClassNMS.cu
 I don't have a better alternative than this.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to