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

[GPU]get data type of conv weights from node.weights() when network is internal #12232

Merged
merged 6 commits into from
Aug 3, 2022
Merged

[GPU]get data type of conv weights from node.weights() when network is internal #12232

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f948ba3
get data type of convolution weights from node.weights() when network…
wilson-seok Jul 20, 2022
56c33b5
use only instance.node.weights().get_output_layout().data_type
wilson-seok Jul 25, 2022
749f15c
fix typo
wilson-seok Jul 25, 2022
e439670
Merge branch 'openvinotoolkit:master' into fix_convolution_weights_da…
wilson-seok Aug 1, 2022
93c7813
add unit test for the case
wilson-seok Aug 2, 2022
1dfdadb
Merge branch 'master' into fix_convolution_weights_datatype_check
wilson-seok Aug 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/plugins/intel_gpu/src/graph/impls/ocl/convolution.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,19 @@ struct convolution_impl : typed_primitive_impl_ocl<convolution> {

auto outer_id = _outer.id();
auto data_type = instance.node.input().get_output_layout().data_type;
cldnn::data_types weights_data_type;
if (instance.get_network().is_internal()) {
weights_data_type = instance.node.weights().get_output_layout().data_type;
} else {
weights_data_type = instance.weights_memory(0)->get_layout().data_type;
}

// Integer signed/unsigned is ok for convoluiton
Copy link
Contributor

Choose a reason for hiding this comment

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

please create unit tests for that 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.

@vladimir-paramuzov So unit test for getting instance.node.weights().get_output_layout().data_type? Do we have similar test case in clDNN unit test?

Copy link
Contributor

Choose a reason for hiding this comment

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

The unit test should be for convolution with weights located in input_layout primitive, not for getting something from node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vladimir-paramuzov Convolution unit test already includes the case of weights located in input_layout primitive. https://github.com/wilson-seok/openvino/blob/749f15cf8b7c9db5a50b5bda4c458f883412e529/src/plugins/intel_gpu/tests/test_cases/convolution_gpu_test.cpp#L1298
Do you mean this kind of test or another one?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, something like that, but new test case should cover fixed scenario, i.e. it must fail w/o your fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vladimir-paramuzov Added unit test for the case. Thanks for the comments!

CLDNN_ERROR_DATA_TYPES_MISMATCH_IGNORE_SIGN(outer_id,
"Input memory",
data_type,
"filter memory",
instance.weights_memory(0)->get_layout().data_type,
weights_data_type,
"");

return res;
Expand Down