-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[nGraph][IE] Enable FP64 data type in IE and review nGraph Python tests #2563
[nGraph][IE] Enable FP64 data type in IE and review nGraph Python tests #2563
Conversation
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 is hard to review because you changed the code style for all file.
In general if you add a new precision to the Inference Engine please also add new tests.
You can use this PR as an example: https://github.com/openvinotoolkit/openvino/pull/1297/files
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.
please revert indentation changes to keep diff tidy.
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.
Please revert indentation changes. Otherwise looks good.
Should be openvino/inference-engine/ie_bridges/python/src/openvino/inference_engine/ie_api.pyx Line 1653 in 0c79804
@akuporos What do you think? |
e534e20
to
fa62920
Compare
inference-engine/ie_bridges/python/src/openvino/inference_engine/constants.pyx
Outdated
Show resolved
Hide resolved
@@ -382,6 +382,9 @@ CNNLayer::Ptr NodeConverter<ngraph::op::Convert>::createLayer(const std::shared_ | |||
case Precision::FP32: | |||
precision_str = "FP32"; | |||
break; | |||
case Precision::FP64: |
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 can be done separately.
I think we can replace this switch/case to one line: precision_str = p.name();
Please fix a CI. |
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'd be nice to add a test here
Please make CI green. |
No description provided.