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.

Sign up for GitHub

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

Jump to bottom

Common test infrastructure suggestions #14343

Merged
merged 3 commits into from
Jan 11, 2023
Merged

Common test infrastructure suggestions #14343

merged 3 commits into from
Jan 11, 2023

Conversation

IvanNovoselov
Copy link
Contributor

Details:

  • Added static_shapes_to_test_representation but for partial shapes. This will be used in snippets tests: PR#14327
  • I noticed that data generate method could generate inputs in a very narrow range of values. A fix is proposed.

@IvanNovoselov IvanNovoselov requested review from a team as code owners November 30, 2022 17:28
@IvanNovoselov IvanNovoselov mentioned this pull request Nov 30, 2022
Merged
4 tasks
@IvanNovoselov
Copy link
Contributor Author

@iefode could you take a look please?

@IvanNovoselov IvanNovoselov added the category: IE Tests OpenVINO Test: plugins and common label Nov 30, 2022
iefode
iefode reviewed Dec 6, 2022
View reviewed changes
src/tests/functional/shared_test_classes/include/shared_test_classes/base/ov_subgraph.hpp Outdated
@@ -61,6 +61,16 @@ class SubgraphBaseTest : public CommonTestUtils::TestsCommon {
virtual std::vector<ov::Tensor> get_plugin_outputs();
};

inline std::vector<InputShape> dynamic_shapes_to_test_representation(const std::vector<ov::PartialShape>& shapes) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a example of usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, please have a look here or here.

src/tests/functional/shared_test_classes/include/shared_test_classes/base/ov_subgraph.hpp Outdated
std::vector<InputShape> result;
for (const auto& staticShape : shapes) {
if (staticShape.is_dynamic())
throw std::runtime_error("dynamic_shapes_to_test_representation can process only static shapes");
Copy link
Contributor

Choose a reason for hiding this comment

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

Not clear to understand why we can work with static shapes only in dynamic_shapes_to_test_representation. What should do this function? Please provide3 more applicable name..
And:
result.push_back({{staticShape}, {staticShape.get_shape()}}); ?? why do use such construction? if shapes are static? We can extend static_shapes_to_test_representation for the case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, static_shapes_to_test_representation is used to transform std::vector<ov::Shape> -> std::vector<InputShape>. However, shapes are represented as std::vector<ov::PartialShape> in dynamic tests, so I thought it would be nice to have a similar function that transforms std::vector<ov::PartialShape> -> std::vector<InputShape>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, I can add static_shapes_to_test_representation overload that takes std::vector<ov::PartialShape> as an argument.

iefode
iefode reviewed Dec 20, 2022
View reviewed changes
src/tests/functional/shared_test_classes/include/shared_test_classes/base/ov_subgraph.hpp Outdated
@@ -61,6 +61,16 @@ class SubgraphBaseTest : public CommonTestUtils::TestsCommon {
virtual std::vector<ov::Tensor> get_plugin_outputs();
};

inline std::vector<InputShape> static_shapes_to_test_representation(const std::vector<ov::PartialShape>& shapes) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If i am correct, Shape is inherited from Partial shape

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That'll be too easy for us)
Shape is derived from vector<size_t> and PartialShape is not derived from anything.

@IvanNovoselov
Copy link
Contributor Author

Ok, so static_shapes_to_test_representation overloading doesn't work because bracket-initialization is used heavily in the code, and the compiler fails to determine which overload should be used in such cases.
So, we have to handle partial shapes in a function with a different name. My suggestion is static_partial_shapes_to_test_representation

@iefode iefode enabled auto-merge (squash) January 10, 2023 13:49
@iefode iefode merged commit 1d59a5a into openvinotoolkit:master Jan 11, 2023
@IvanNovoselov IvanNovoselov deleted the fix_common_tests branch October 2, 2023 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iefode iefode iefode approved these changes

Assignees

@iefode iefode

Labels
category: IE Tests OpenVINO Test: plugins and common
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@IvanNovoselov @iefode