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

Eliminate reader tests #13409

Merged
merged 8 commits into from
Oct 27, 2022
Merged

Eliminate reader tests #13409

merged 8 commits into from
Oct 27, 2022

Conversation

olpipi
Copy link
Contributor

@olpipi olpipi commented Oct 10, 2022 •
edited
Loading

Details:

  • Remove ngraph reader test
  • Extend IR frontend and Core tests to cover gaps

Tickets:

@olpipi olpipi force-pushed the eliminate_reader_tests branch from 4bf6d39 to 0ea51dc Compare October 10, 2022 18:26
@olpipi olpipi force-pushed the eliminate_reader_tests branch from 0ea51dc to 7e26569 Compare October 12, 2022 12:29
@olpipi olpipi force-pushed the eliminate_reader_tests branch from 7e26569 to ad88f65 Compare October 14, 2022 14:34
@olpipi olpipi force-pushed the eliminate_reader_tests branch from dc3f8a4 to 9a85397 Compare October 21, 2022 11:05
@olpipi
Copy link
Contributor Author

olpipi commented Oct 21, 2022 •
edited
Loading

@ilyachur please start review. and clarify about these 2 files:
src/tests/functional/inference_engine/ngraph_reader/linear_ops_tests.cpp
src/tests/functional/inference_engine/ngraph_reader/reduce_to_pooling_tests.cpp

There are small fixes for coverage improvement and codestyle to be uploaded today.

@ilyachur
Copy link
Contributor

@ilyachur please start review. and clarify about these 2 files: src/tests/functional/inference_engine/ngraph_reader/linear_ops_tests.cpp src/tests/functional/inference_engine/ngraph_reader/reduce_to_pooling_tests.cpp

There are small fixes for coverage improvement and codestyle to be uploaded today.

@itikhono Could you please advise? Should we move these tests to transformations? Or can we remove these files?

@itikhono
Copy link
Contributor

itikhono commented Oct 24, 2022 •
edited
Loading

@olpipi @ilyachur as I can see, we already have such tests for transformations
MulAddToScaleShift or Power:
openvino/src/tests/functional/inference_engine/transformations/legacy/mul_add_conversion_test.cpp
openvino/src/common/legacy/src/transformations/convert_opset1_to_legacy/convert_mul_add_to_scaleshift_or_power.cpp

ReduceToPooling: openvino/src/tests/functional/inference_engine/transformations/op_conversions/convert_reduce_to_pooling_test.cpp

it looks like we can delete these ngraph reader_tests

@olpipi olpipi force-pushed the eliminate_reader_tests branch from 9a85397 to b7ec61b Compare October 25, 2022 10:23
@olpipi olpipi marked this pull request as ready for review October 25, 2022 16:50
@olpipi olpipi requested review from a team as code owners October 25, 2022 16:50
@olpipi olpipi force-pushed the eliminate_reader_tests branch from f51ddbe to e06c150 Compare October 25, 2022 17:03
ilyachur
ilyachur approved these changes Oct 27, 2022
View reviewed changes
Copy link
Contributor

@ilyachur ilyachur left a comment

Choose a reason for hiding this comment

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

Please restore custom constant op tests. It can be done in next PR

src/core/tests/visitors/op/abs.cpp

#include "unary_ops.hpp"

using Type = ::testing::Types<UnaryOperatorType<ngraph::op::Abs, ngraph::element::f32>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use new API :) It can be fixed separatelly.

@ilyachur ilyachur enabled auto-merge (squash) October 27, 2022 06:32
@ilyachur ilyachur self-assigned this Oct 27, 2022
@ilyachur ilyachur added this to the 2022.3 milestone Oct 27, 2022
@ilyachur ilyachur merged commit ad1c824 into openvinotoolkit:master Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilyachur ilyachur ilyachur approved these changes

Assignees

@ilyachur ilyachur

Labels
None yet
Projects
None yet
Milestone
2022.3
Development

Successfully merging this pull request may close these issues.
3 participants
@olpipi @ilyachur @itikhono