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

Do not use ONNX reader if ONNX importer was disabled #683

Conversation

ilyachur
Copy link
Contributor

No description provided.

@ilyachur ilyachur requested a review from a team May 29, 2020 11:12
@ilyachur ilyachur added the bug Something isn't working label May 29, 2020
@ilyachur ilyachur added this to the 2020.4 milestone May 29, 2020
@ggladilov
Copy link
Contributor

ggladilov commented May 29, 2020

@ilyachur I still don't understand why PR that brings new library onnx reader passed the CI?
We see the issue locally and on our private CI. Should there be fixed somewhere else to prevent such issues in the future?

@ilyachur
Copy link
Contributor Author

ilyachur commented May 29, 2020

@ilyachur I still don't understand why PR that brings new library onnx reader passed the CI?
We see the issue locally and on our private CI. Should there be fixed somewhere else to prevent such issues in the future?

@gladilov-gleb Because ONNX importer is included in default build. It means that this library always exists in CI.

@ilya-lavrenov ilya-lavrenov added the category: IR FE OpenVINO IR v10 / v11 FrontEnd label May 29, 2020
@ilya-lavrenov ilya-lavrenov self-assigned this May 29, 2020
@ggladilov
Copy link
Contributor

@ilyachur I still don't understand why PR that brings new library onnx reader passed the CI?
We see the issue locally and on our private CI. Should there be fixed somewhere else to prevent such issues in the future?

@gladilov-gleb Because ONNX importer is included in default build. It means that this library always exists in CI.

Thanks for clarification! Sounds interesting, we need to take a look at the situation from myriad plugin side to understand should we change our CI pipeline or not.

@omaskovx
Copy link

@ilyachur Please add inference_engine_onnx_reader, depending on ie_dev_targets, then this will work for us

@ilyachur
Copy link
Contributor Author

@ilyachur Please add inference_engine_onnx_reader, depending on ie_dev_targets, then this will work for us

I think it won't work for your case. We try to find this library in runtime (as plugins). Please correct me if I am not right.

@ilya-lavrenov
Copy link
Contributor

@ilyachur Please add inference_engine_onnx_reader, depending on ie_dev_targets, then this will work for us

I think it won't work for your case. We try to find this library in runtime (as plugins). Please correct me if I am not right.

Depends on their problems.

@omaskovx what kind of error do you have?

@omaskovx
Copy link

inference_engine_onnx_reader, в зависимости от того ie_dev_targets

For our CI, this will work. I checked our CI with your branch, the current fix also breaks our CI because the library is still being searched. We need to build this library in our CI, then the problem will be fixed, and for this please add inference_engine_onnx_reader, depending on ie_dev_targets

@omaskovx
Copy link

@ilyachur Please add inference_engine_onnx_reader, depending on ie_dev_targets, then this will work for us

I think it won't work for your case. We try to find this library in runtime (as plugins). Please correct me if I am not right.

Depends on their problems.

@omaskovx what kind of error do you have?

which is described in 32398

@omaskovx
Copy link

@ilyachur Well, how is the progress?

@ilya-lavrenov ilya-lavrenov merged commit 11bd4f8 into openvinotoolkit:master May 29, 2020
@ilyachur ilyachur deleted the feature/ichuraev/optional_onnx_reader branch June 1, 2020 02:30
azhogov pushed a commit to azhogov/openvino that referenced this pull request Jun 1, 2020
@ilya-lavrenov ilya-lavrenov added the category: build OpenVINO cmake script / infra label Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working category: build OpenVINO cmake script / infra category: IR FE OpenVINO IR v10 / v11 FrontEnd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants