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

[IE TEST] LRN tests fixed params #743

Merged
merged 3 commits into from
Jul 1, 2020

Conversation

l-bat
Copy link
Contributor

@l-bat l-bat commented Jun 3, 2020

Now in LRN tests using size_t beta, bias, but ngraph::op::LRN takes double beta, bias. We also need to add type of normalization channel algorithm Output<Node> axes.

LRN(const Output<Node>& arg,
       const Output<Node>& axes
       double alpha,
       double beta,
       double bias,
       size_t size);

please, take a look @iefode

@l-bat l-bat requested a review from a team June 3, 2020 11:30
@l-bat l-bat added the category: IE Tests OpenVINO Test: plugins and common label Jun 3, 2020

auto ngPrc = FuncTestUtils::PrecisionUtils::convertIE2nGraphPrc(netPrecision);
auto params = ngraph::builder::makeParams(ngPrc, {inputShapes});
auto paramIn =
ngraph::helpers::convert2OutputVector(ngraph::helpers::castOps2Nodes<ngraph::op::Parameter>(params));

auto lrn = std::make_shared<ngraph::opset1::LRN>(paramIn[0], alpha, beta, bias, size);
std::vector<int64_t> axes;
if (region == "same") {

Choose a reason for hiding this comment

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

I propose to parameterize the test directly with the axes parameter instead of some key-word which defines certain behavior of the test.

There are two advantages to such an approach. First of all, it is to be aligned with opset specification which describes LRN parameters and inputs. And also to make it possible to validate cases different from currently possible axes values {1} and {2,3,4,...}

@l-bat l-bat requested a review from mikhail-treskin June 15, 2020 08:53
@l-bat l-bat force-pushed the lbat/fix_lrn_ie_tests branch 2 times, most recently from d14045b to 204ec85 Compare June 22, 2020 05:56
@l-bat
Copy link
Contributor Author

l-bat commented Jun 22, 2020

please, take a look @mikhail-treskin

@l-bat l-bat force-pushed the lbat/fix_lrn_ie_tests branch from e7b54e5 to 7dd0faf Compare June 23, 2020 06:48
@iefode
Copy link
Contributor

iefode commented Jun 23, 2020

LGTM

@l-bat l-bat added this to the 2020.4 milestone Jun 28, 2020
@iefode
Copy link
Contributor

iefode commented Jun 29, 2020

@mikhail-treskin Please review changes again

@l-bat l-bat force-pushed the lbat/fix_lrn_ie_tests branch from 7dd0faf to e77d79e Compare June 30, 2020 11:59
@l-bat l-bat force-pushed the lbat/fix_lrn_ie_tests branch from e77d79e to 7e9d07d Compare July 1, 2020 03:46
@l-bat l-bat force-pushed the lbat/fix_lrn_ie_tests branch from 7e9d07d to 91f0437 Compare July 1, 2020 05:04
@iefode iefode merged commit cff39c3 into openvinotoolkit:master Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: IE Tests OpenVINO Test: plugins and common
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants