-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[GNA] Depth-wise separable convolution support #7281
Conversation
05e48e2
to
2d6cdaa
Compare
inference-engine/src/gna_plugin/transformations/convert_dwsc_to_scaleshifts.cpp
Outdated
Show resolved
Hide resolved
inference-engine/src/gna_plugin/transformations/convert_dwsc_to_scaleshifts.cpp
Outdated
Show resolved
Hide resolved
inference-engine/src/gna_plugin/transformations/convert_dwsc_to_scaleshifts.cpp
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
#include "transformations/convert_dwsc_to_scaleshifts.hpp" | ||
#include "common_test_utils/ngraph_test_utils.hpp" | ||
#include <ngraph/function.hpp> | ||
#include <ngraph/opsets/opset7.hpp> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it ok to use opset7 directelly?
isn't there sth like "opset_newest" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion by ngraph developers was to use opset7, so I believe this is the way to go for now.
auto bias_size = shape_size(add_const->get_shape()); | ||
auto conv_filter_count = conv->get_output_shape(0)[1]; | ||
if (bias_size == conv_filter_count) | ||
return add_const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider to make more strict const shape comparison,
i.e.: not only compare total shape size but check that each dimension matches expected one by one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's assumed that bias const is flat and should be broadcasted, but I'll take notice of this suggestion in next PR.
@@ -72,4 +73,29 @@ std::shared_ptr<ngraph::opset7::StridedSlice> FlatCrop(ngraph::Output<ngraph::No | |||
std::vector<int64_t>{1, 0}); // end mask | |||
} | |||
|
|||
std::shared_ptr<ngraph::Node> VerifyBiasGetConst(std::shared_ptr<ngraph::Node> conv, std::shared_ptr<ngraph::Node> bias) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider not to pass conv
.
probaby conv filter count
could be derived from bias->input(1)->shape[1]
.
consider to rename VerifyBiasGetConst() into GetVerifiedConstBiasPerFilter()
consider to rename bias into add (or addToVerify)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If bias->input(1)->shape[1]
is compared instead of conv_filter_count
then it will be compared with itself.
* [GNA] Add support for DWSC, other fixes and code refactoring. * [GNA] Change supported layout to NHWC * [GNA] Detect bias const only on second position, move verification of dwsc to matcher
* [GNA] Add support for DWSC, other fixes and code refactoring. * [GNA] Change supported layout to NHWC * [GNA] Detect bias const only on second position, move verification of dwsc to matcher
Details:
Tickets: