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

MatrixNMS-8 spec. #5948

Merged

Conversation

ceciliapeng2011
Copy link
Contributor

Details:

  • Specification of a new operator called MatrixNMS.
  • This Op functionally equivalent to NonMaxSuppression-5, with an non max suppression algorithm called matrix_nms, and perform more post-processing phases, and lay out the detection outputs in the way of PaddlePaddle detection.

Tickets:

  • 55968

@ceciliapeng2011 ceciliapeng2011 requested a review from a team as a code owner June 1, 2021 09:06
@ceciliapeng2011 ceciliapeng2011 changed the title Specification of a new operator called MatrixNMS. MatrixNMS-8 spec. Jun 3, 2021
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
@lazarevevgeny
Copy link
Contributor

@openvinotoolkit/openvino-docs-maintainers , please, review this PR.

docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
@ceciliapeng2011
Copy link
Contributor Author

@zhangYiIntel welcome to review this spec, especially the operation algorithm description.

docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved

* *sort_result_across_batch*

* **Description**: *sort_result_across_batch* is a flag that specifies whenever it is necessary to sort selected boxes across batches or not.
Copy link
Contributor

Choose a reason for hiding this comment

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

PaddlePaddle has an attribute called normalized to specify whether the bounding boxes are normalized by the image width/height. Do we need to add this attribute ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't have to be in OV Op. We will handle it in our PDPD->OV conversion.

Copy link
Contributor

Choose a reason for hiding this comment

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

How do you know the image size if you want to handle it in PDPD->OV conversion? So I suggest to add this attribute.

Copy link
Contributor

Choose a reason for hiding this comment

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

During op conversion, it gets the attribute value True/False, but during calculation, they have different IOU calculation methods depends on the normalized value. how can we handle this during the Ops conversion ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This "normalize" attribute is PDPD specific, no need to use image size. It simply add a const offset to the original bbox. And as discussed with ZhangYi, we can handle this with a decomposition.

Please refer to Paddle2ONNX, which solve this by adding 2 Add operation before NMS.
https://github.com/PaddlePaddle/Paddle2ONNX/blob/e95695af98f1cf417d3ba2c0e81abf7b59948ab6/paddle2onnx/op_mapper/detection/multiclass_nms.py#L106

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lazarevevgeny I am so sorry I have to reopen this discussion. During test I found it cannot be solved in decomposition. Extra substract operation is required after NMS. It looks awkward. What's more, the extra substracts won't success as they are dynamic shape input.

This Op functionally equivalent to NonMaxSuppression-5, with an non max suppression algorithm called matrix_nms, and perform more post-processing phases, and lay out
the detection outputs in the way of PaddlePaddle detection.
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
docs/ops/sort/MatrixNMS_8.md Outdated Show resolved Hide resolved
@yuxu42 yuxu42 requested a review from lazarevevgeny June 17, 2021 06:50
ceciliapeng2011 and others added 7 commits June 18, 2021 09:45
@ceciliapeng2011 ceciliapeng2011 requested a review from avladimi June 18, 2021 03:00
@lazarevevgeny lazarevevgeny merged commit ac01777 into openvinotoolkit:master Jun 18, 2021
rnugmanx pushed a commit to rnugmanx/openvino that referenced this pull request Aug 26, 2021
* Specification of a new operator called MatrixNMS.

This Op functionally equivalent to NonMaxSuppression-5, with an non max suppression algorithm called matrix_nms, and perform more post-processing phases, and lay out
the detection outputs in the way of PaddlePaddle detection.

* Update docs/ops/sort/MatrixNMS_8.md

Co-authored-by: Anastasiya Ageeva <[email protected]>

* Update docs/ops/sort/MatrixNMS_8.md

Co-authored-by: Anastasiya Ageeva <[email protected]>

* Update docs/ops/sort/MatrixNMS_8.md

Co-authored-by: Anastasiya Ageeva <[email protected]>

* Update docs/ops/sort/MatrixNMS_8.md

Co-authored-by: Anastasiya Ageeva <[email protected]>

* Update docs/ops/sort/MatrixNMS_8.md

Co-authored-by: Anastasiya Ageeva <[email protected]>

* Update docs/ops/sort/MatrixNMS_8.md

Co-authored-by: Anastasiya Ageeva <[email protected]>

* Update docs/ops/sort/MatrixNMS_8.md

Co-authored-by: Anastasiya Ageeva <[email protected]>

* Update docs/ops/sort/MatrixNMS_8.md

Co-authored-by: Anastasiya Ageeva <[email protected]>

* Update MatrixNMS_8.md

* Update MatrixNMS_8.md

Co-authored-by: Anastasiya Ageeva <[email protected]>
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.

5 participants