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

Reference Implementation of ROIPooling op #2903

Merged
merged 55 commits into from
Nov 30, 2020

Conversation

ggalieroc-zz
Copy link

@ggalieroc-zz ggalieroc-zz commented Oct 29, 2020

Overview:

  • Update specification
  • Develop ROIPooling reference implementation
  • Add reference implementation to int_executable
  • Add unit tests
  • Create single layer test class
  • Instantiate single layer tests for CPU and Myriad plugins

ticket: 37530

@iefode
Copy link
Contributor

iefode commented Oct 30, 2020

Please, take Single Layer test from PR, refactor this if it will be necessary and instantiate for CPU plugin

docs/ops/detection/ROIPooling_1.md Outdated Show resolved Hide resolved
docs/ops/detection/ROIPooling_1.md Outdated Show resolved Hide resolved
ngraph/core/include/ngraph/op/roi_pooling.hpp Outdated Show resolved Hide resolved

const int num_rois = rois_shape[0];

for (unsigned int i = 0; i < shape_size(output_shape); i++)
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like std::fill_n.

@ggalieroc-zz ggalieroc-zz marked this pull request as ready for review November 24, 2020 12:35
@ggalieroc-zz ggalieroc-zz requested a review from a team as a code owner November 24, 2020 12:35
@ggalieroc-zz ggalieroc-zz requested review from a team, lazarevevgeny and pelszkow November 24, 2020 12:35
Copy link
Contributor

@lazarevevgeny lazarevevgeny left a comment

Choose a reason for hiding this comment

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

The main comment is that the shape inference function must be more specific and try to calculate output shape even if some of the input dimensions are dynamic.

Copy link
Contributor

@lazarevevgeny lazarevevgeny left a comment

Choose a reason for hiding this comment

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

The spec changes and partial inference function LGTM.

@jdanieck jdanieck self-requested a review November 26, 2020 08:57
@jdanieck jdanieck removed their request for review November 27, 2020 16:10
@ilyachur ilyachur merged commit ac8a39d into openvinotoolkit:master Nov 30, 2020
@ggalieroc-zz ggalieroc-zz deleted the roi_pooling_ref_impl branch November 30, 2020 08:34
evolosen pushed a commit to evolosen/openvino that referenced this pull request Dec 3, 2020
* ROIPooling: Specification and op class alignment

* ROIPooling: Add check to input tensor type to be aligned with spec

* ROIPooling: Corrected spec description for input tensor shape and box coordinates

* ROIPooling: Changed attributes pooled_h and pooled_w from Shape to plain int

* Revert "ROIPooling: Changed attributes pooled_h and pooled_w from Shape to plain int"

This reverts commit d49cfa8.

* ROIPooling: Further specification changes

* ROIPooling: Rename enum class ROIPoolingMethod methods

* Fix style

* ROIPooling: Draft reference implementation

* ROIPooling: Adjust feature map element type to float for attribute unit test

* ROIPooling: Add single layer test class

* ROIPooling: Corrected output index to iterate through output tensor elements

* ROIPooling: Added validation checks for input types in op constructor

* ROIPooling: Add unit tests

* ROIPooling: Attributes unit test changed to align with spec

* ROIPooling: Add check for batch id in reference implementation and unit test

* ROIPooling: Refactor single layer test class

* ROIPooling: Add test for invalid pooling method

* ROIPooling: Clean up unnecessary function declaration

* ROIPooling: Remove duplicated default ROIPooling method in op constructors

* ROIPooling: Add Infer method to generate suitable ROI data

* ROIPooling: CPU single layer test instantiation for max method

* ROIPooling: Remove enum class ROIPoolingMethod

* Revert "ROIPooling: Clean up unnecessary function declaration"

This reverts commit 074b540.

* ROIPooling: Refactor single layer tests after removing enum class ROIPoolingMethod

* ROIPooling: Add attribute checks in op constructor to align with spec and unit tests

* Resolve CI failure: clang could not resolve static conversion from uint64_t to size_t

* ROIPooling: Fix for output index calculation to loop through all ROIs

* ROIPooling: Add unit test for bilinear interpolation method

* ROIPooling: Add CPU single layer test instantiation for bilinear method

* ROIPooling: Clean up unnecessary enum class for pooling method

* ROIPooling: Add myriad single layer test instantiation

* ROIPooling: Add F16 precision single layer tests for CPU plugin

* ROIPooling: Add node validation check for string method attribute in constructor and unit tests

* ROIPooling: Spec changes to improve understanding of the operation

* ROIPooling: Fix for bilinear method when pooled size is 1x1

* ROIPooling: Add unit test for bilinear method and pooled size 1x1

* ROIPooling: Fix to broken format of specifications

* ROIPooling: Disable Myriad single layer tests

* ROIPooling: Handle dynamic dims and ranks for input tensors and unit tests

* ROIPooling: Code clean up

* ROIPooling: Address review comments

* ROIPooling: Changed location for makeROIPooling helper method

Co-authored-by: Kirill Molchanov <[email protected]>
mryzhov pushed a commit to mryzhov/openvino that referenced this pull request Dec 11, 2020
* ROIPooling: Specification and op class alignment

* ROIPooling: Add check to input tensor type to be aligned with spec

* ROIPooling: Corrected spec description for input tensor shape and box coordinates

* ROIPooling: Changed attributes pooled_h and pooled_w from Shape to plain int

* Revert "ROIPooling: Changed attributes pooled_h and pooled_w from Shape to plain int"

This reverts commit d49cfa8.

* ROIPooling: Further specification changes

* ROIPooling: Rename enum class ROIPoolingMethod methods

* Fix style

* ROIPooling: Draft reference implementation

* ROIPooling: Adjust feature map element type to float for attribute unit test

* ROIPooling: Add single layer test class

* ROIPooling: Corrected output index to iterate through output tensor elements

* ROIPooling: Added validation checks for input types in op constructor

* ROIPooling: Add unit tests

* ROIPooling: Attributes unit test changed to align with spec

* ROIPooling: Add check for batch id in reference implementation and unit test

* ROIPooling: Refactor single layer test class

* ROIPooling: Add test for invalid pooling method

* ROIPooling: Clean up unnecessary function declaration

* ROIPooling: Remove duplicated default ROIPooling method in op constructors

* ROIPooling: Add Infer method to generate suitable ROI data

* ROIPooling: CPU single layer test instantiation for max method

* ROIPooling: Remove enum class ROIPoolingMethod

* Revert "ROIPooling: Clean up unnecessary function declaration"

This reverts commit 074b540.

* ROIPooling: Refactor single layer tests after removing enum class ROIPoolingMethod

* ROIPooling: Add attribute checks in op constructor to align with spec and unit tests

* Resolve CI failure: clang could not resolve static conversion from uint64_t to size_t

* ROIPooling: Fix for output index calculation to loop through all ROIs

* ROIPooling: Add unit test for bilinear interpolation method

* ROIPooling: Add CPU single layer test instantiation for bilinear method

* ROIPooling: Clean up unnecessary enum class for pooling method

* ROIPooling: Add myriad single layer test instantiation

* ROIPooling: Add F16 precision single layer tests for CPU plugin

* ROIPooling: Add node validation check for string method attribute in constructor and unit tests

* ROIPooling: Spec changes to improve understanding of the operation

* ROIPooling: Fix for bilinear method when pooled size is 1x1

* ROIPooling: Add unit test for bilinear method and pooled size 1x1

* ROIPooling: Fix to broken format of specifications

* ROIPooling: Disable Myriad single layer tests

* ROIPooling: Handle dynamic dims and ranks for input tensors and unit tests

* ROIPooling: Code clean up

* ROIPooling: Address review comments

* ROIPooling: Changed location for makeROIPooling helper method

Co-authored-by: Kirill Molchanov <[email protected]>
mryzhov pushed a commit to mryzhov/openvino that referenced this pull request Dec 16, 2020
* ROIPooling: Specification and op class alignment

* ROIPooling: Add check to input tensor type to be aligned with spec

* ROIPooling: Corrected spec description for input tensor shape and box coordinates

* ROIPooling: Changed attributes pooled_h and pooled_w from Shape to plain int

* Revert "ROIPooling: Changed attributes pooled_h and pooled_w from Shape to plain int"

This reverts commit d49cfa8.

* ROIPooling: Further specification changes

* ROIPooling: Rename enum class ROIPoolingMethod methods

* Fix style

* ROIPooling: Draft reference implementation

* ROIPooling: Adjust feature map element type to float for attribute unit test

* ROIPooling: Add single layer test class

* ROIPooling: Corrected output index to iterate through output tensor elements

* ROIPooling: Added validation checks for input types in op constructor

* ROIPooling: Add unit tests

* ROIPooling: Attributes unit test changed to align with spec

* ROIPooling: Add check for batch id in reference implementation and unit test

* ROIPooling: Refactor single layer test class

* ROIPooling: Add test for invalid pooling method

* ROIPooling: Clean up unnecessary function declaration

* ROIPooling: Remove duplicated default ROIPooling method in op constructors

* ROIPooling: Add Infer method to generate suitable ROI data

* ROIPooling: CPU single layer test instantiation for max method

* ROIPooling: Remove enum class ROIPoolingMethod

* Revert "ROIPooling: Clean up unnecessary function declaration"

This reverts commit 074b540.

* ROIPooling: Refactor single layer tests after removing enum class ROIPoolingMethod

* ROIPooling: Add attribute checks in op constructor to align with spec and unit tests

* Resolve CI failure: clang could not resolve static conversion from uint64_t to size_t

* ROIPooling: Fix for output index calculation to loop through all ROIs

* ROIPooling: Add unit test for bilinear interpolation method

* ROIPooling: Add CPU single layer test instantiation for bilinear method

* ROIPooling: Clean up unnecessary enum class for pooling method

* ROIPooling: Add myriad single layer test instantiation

* ROIPooling: Add F16 precision single layer tests for CPU plugin

* ROIPooling: Add node validation check for string method attribute in constructor and unit tests

* ROIPooling: Spec changes to improve understanding of the operation

* ROIPooling: Fix for bilinear method when pooled size is 1x1

* ROIPooling: Add unit test for bilinear method and pooled size 1x1

* ROIPooling: Fix to broken format of specifications

* ROIPooling: Disable Myriad single layer tests

* ROIPooling: Handle dynamic dims and ranks for input tensors and unit tests

* ROIPooling: Code clean up

* ROIPooling: Address review comments

* ROIPooling: Changed location for makeROIPooling helper method

Co-authored-by: Kirill Molchanov <[email protected]>
mryzhov pushed a commit to mryzhov/openvino that referenced this pull request Jan 14, 2021
* ROIPooling: Specification and op class alignment

* ROIPooling: Add check to input tensor type to be aligned with spec

* ROIPooling: Corrected spec description for input tensor shape and box coordinates

* ROIPooling: Changed attributes pooled_h and pooled_w from Shape to plain int

* Revert "ROIPooling: Changed attributes pooled_h and pooled_w from Shape to plain int"

This reverts commit d49cfa8.

* ROIPooling: Further specification changes

* ROIPooling: Rename enum class ROIPoolingMethod methods

* Fix style

* ROIPooling: Draft reference implementation

* ROIPooling: Adjust feature map element type to float for attribute unit test

* ROIPooling: Add single layer test class

* ROIPooling: Corrected output index to iterate through output tensor elements

* ROIPooling: Added validation checks for input types in op constructor

* ROIPooling: Add unit tests

* ROIPooling: Attributes unit test changed to align with spec

* ROIPooling: Add check for batch id in reference implementation and unit test

* ROIPooling: Refactor single layer test class

* ROIPooling: Add test for invalid pooling method

* ROIPooling: Clean up unnecessary function declaration

* ROIPooling: Remove duplicated default ROIPooling method in op constructors

* ROIPooling: Add Infer method to generate suitable ROI data

* ROIPooling: CPU single layer test instantiation for max method

* ROIPooling: Remove enum class ROIPoolingMethod

* Revert "ROIPooling: Clean up unnecessary function declaration"

This reverts commit 074b540.

* ROIPooling: Refactor single layer tests after removing enum class ROIPoolingMethod

* ROIPooling: Add attribute checks in op constructor to align with spec and unit tests

* Resolve CI failure: clang could not resolve static conversion from uint64_t to size_t

* ROIPooling: Fix for output index calculation to loop through all ROIs

* ROIPooling: Add unit test for bilinear interpolation method

* ROIPooling: Add CPU single layer test instantiation for bilinear method

* ROIPooling: Clean up unnecessary enum class for pooling method

* ROIPooling: Add myriad single layer test instantiation

* ROIPooling: Add F16 precision single layer tests for CPU plugin

* ROIPooling: Add node validation check for string method attribute in constructor and unit tests

* ROIPooling: Spec changes to improve understanding of the operation

* ROIPooling: Fix for bilinear method when pooled size is 1x1

* ROIPooling: Add unit test for bilinear method and pooled size 1x1

* ROIPooling: Fix to broken format of specifications

* ROIPooling: Disable Myriad single layer tests

* ROIPooling: Handle dynamic dims and ranks for input tensors and unit tests

* ROIPooling: Code clean up

* ROIPooling: Address review comments

* ROIPooling: Changed location for makeROIPooling helper method

Co-authored-by: Kirill Molchanov <[email protected]>
jiwaszki pushed a commit to akuporos/openvino that referenced this pull request Jan 15, 2021
* ROIPooling: Specification and op class alignment

* ROIPooling: Add check to input tensor type to be aligned with spec

* ROIPooling: Corrected spec description for input tensor shape and box coordinates

* ROIPooling: Changed attributes pooled_h and pooled_w from Shape to plain int

* Revert "ROIPooling: Changed attributes pooled_h and pooled_w from Shape to plain int"

This reverts commit d49cfa8.

* ROIPooling: Further specification changes

* ROIPooling: Rename enum class ROIPoolingMethod methods

* Fix style

* ROIPooling: Draft reference implementation

* ROIPooling: Adjust feature map element type to float for attribute unit test

* ROIPooling: Add single layer test class

* ROIPooling: Corrected output index to iterate through output tensor elements

* ROIPooling: Added validation checks for input types in op constructor

* ROIPooling: Add unit tests

* ROIPooling: Attributes unit test changed to align with spec

* ROIPooling: Add check for batch id in reference implementation and unit test

* ROIPooling: Refactor single layer test class

* ROIPooling: Add test for invalid pooling method

* ROIPooling: Clean up unnecessary function declaration

* ROIPooling: Remove duplicated default ROIPooling method in op constructors

* ROIPooling: Add Infer method to generate suitable ROI data

* ROIPooling: CPU single layer test instantiation for max method

* ROIPooling: Remove enum class ROIPoolingMethod

* Revert "ROIPooling: Clean up unnecessary function declaration"

This reverts commit 074b540.

* ROIPooling: Refactor single layer tests after removing enum class ROIPoolingMethod

* ROIPooling: Add attribute checks in op constructor to align with spec and unit tests

* Resolve CI failure: clang could not resolve static conversion from uint64_t to size_t

* ROIPooling: Fix for output index calculation to loop through all ROIs

* ROIPooling: Add unit test for bilinear interpolation method

* ROIPooling: Add CPU single layer test instantiation for bilinear method

* ROIPooling: Clean up unnecessary enum class for pooling method

* ROIPooling: Add myriad single layer test instantiation

* ROIPooling: Add F16 precision single layer tests for CPU plugin

* ROIPooling: Add node validation check for string method attribute in constructor and unit tests

* ROIPooling: Spec changes to improve understanding of the operation

* ROIPooling: Fix for bilinear method when pooled size is 1x1

* ROIPooling: Add unit test for bilinear method and pooled size 1x1

* ROIPooling: Fix to broken format of specifications

* ROIPooling: Disable Myriad single layer tests

* ROIPooling: Handle dynamic dims and ranks for input tensors and unit tests

* ROIPooling: Code clean up

* ROIPooling: Address review comments

* ROIPooling: Changed location for makeROIPooling helper method

Co-authored-by: Kirill Molchanov <[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.

8 participants