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

NormalizeL2 transformation #1892

Merged
merged 14 commits into from
Aug 28, 2020

Conversation

mbencer
Copy link
Contributor

@mbencer mbencer commented Aug 21, 2020

No description provided.

@mbencer mbencer added the category: transformations OpenVINO Runtime library - Transformations label Aug 21, 2020
@mbencer mbencer requested review from a team August 21, 2020 08:45
@mbencer mbencer self-assigned this Aug 21, 2020
@openvino-pushbot openvino-pushbot added category: IE Tests OpenVINO Test: plugins and common category: Core OpenVINO Core (aka ngraph) labels Aug 21, 2020
@mbencer mbencer requested a review from lazarevevgeny August 21, 2020 15:12
return false;
}

if (shape_size(constant->get_shape()) > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the constant is empty? I mean if the size is equal to 0.

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 As I understand shape_size(constant->get_shape()) == 0 means that it is scalar

Copy link
Contributor

Choose a reason for hiding this comment

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

Scalar is only if shapes are empty.
But if one dimension is equal to 0, shape_size also returns 0, if I remember right.
But in any way it is a really strange case.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this check is not only for a scalar but constant with any shape like {1, 1, 1} that we can also cast to a single value.
So shape_size returns "Number of elements in spanned by a 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.

added self-explanatory variable (is_scalar_or_single_elem) + used is_scalar helper

@mbencer mbencer requested a review from lazarevevgeny August 21, 2020 20:20
return false;
}

if (shape_size(constant->get_shape()) > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Scalar is only if shapes are empty.
But if one dimension is equal to 0, shape_size also returns 0, if I remember right.
But in any way it is a really strange case.

auto reduce_sum = std::make_shared<ngraph::opset4::ReduceSum>(pow, axes);
auto sqrt = std::make_shared<ngraph::opset4::Sqrt>(reduce_sum);
auto eps_const = ngraph::pattern::wrap_type<ngraph::opset4::Constant>();
auto sqrt_add_eps = std::make_shared<ngraph::opset4::Add>(sqrt, eps_const);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can join these two passes if check sqrt_max_eps or sqrt_add_eps types in the callback.
@GlebKazantaev what do you think about it?

Copy link
Contributor

Choose a reason for hiding this comment

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

@mitruska mitruska self-assigned this Aug 25, 2020
@lazarevevgeny
Copy link
Contributor

@ilyachur , @GlebKazantaev , please, take a look.

@ilyachur
Copy link
Contributor

@ilyachur , @GlebKazantaev , please, take a look.

I asked @GlebKazantaev to look at this PR. I think we can join two passes from this PR.

@ilyachur
Copy link
Contributor

LGTM

@ilyachur ilyachur merged commit 55e4d56 into openvinotoolkit:master Aug 28, 2020
RomanZm pushed a commit to RomanZm/openvino that referenced this pull request Aug 28, 2020
* first version of implementation

* added unit tests

* changed multiply to pow

* doc + unit tests

* more unit tests

* code review remarks

* missing new line

* remarks

* review remarks

* Build fix - update constant check function in HSwishFusionWithClamp

Co-authored-by: mitruska <[email protected]>
ZlobinGM pushed a commit to ZlobinGM/openvino that referenced this pull request Sep 3, 2020
* first version of implementation

* added unit tests

* changed multiply to pow

* doc + unit tests

* more unit tests

* code review remarks

* missing new line

* remarks

* review remarks

* Build fix - update constant check function in HSwishFusionWithClamp

Co-authored-by: mitruska <[email protected]>
@mbencer mbencer deleted the mbencer/NormalizeL2Pass branch March 4, 2021 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: Core OpenVINO Core (aka ngraph) category: IE Tests OpenVINO Test: plugins and common category: transformations OpenVINO Runtime library - Transformations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants