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

OpenVino add build_shared_lib flag in the build command #6560

Merged
merged 4 commits into from
Feb 5, 2021

Conversation

sfatimar
Copy link
Contributor

@sfatimar sfatimar commented Feb 4, 2021

We were not able to build C++ samples without build_shared_lib flag in the build command.

Motivation and Context

  • This changes was required in dockerfiles to enable c++ sample build
  • If it fixes an open issue, please link to the issue here.

sfatimar and others added 4 commits February 2, 2021 12:59
2021_1 indendation changes
add build shared lib flag
add build shared lib flag
@sfatimar sfatimar requested a review from a team as a code owner February 4, 2021 02:33
@snnn snnn added the ep:OpenVINO issues related to OpenVINO execution provider label Feb 4, 2021
@HectorSVC
Copy link
Contributor

/azp run Linux OpenVINO CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@HectorSVC HectorSVC changed the title Sahar/indentation fix OpenVino add build_shared_lib flag in the build command Feb 5, 2021
@jywu-msft
Copy link
Member

is it really necessary to change the dockerfiles? (which are for building python bindings/package)
the build_shared_lib option is for onnxruntime lib.
openvino ep gets built as a shared lib regardless of whether build_shared_lib option is enabled.

@sfatimar
Copy link
Contributor Author

sfatimar commented Feb 5, 2021

We have added build_shared_lib to build libonnxruntime.so as some developers found that it was easier to build C++ samples this way.

@MaajidKhan
Copy link
Contributor

is it really necessary to change the dockerfiles? (which are for building python bindings/package)
the build_shared_lib option is for onnxruntime lib.
openvino ep gets built as a shared lib regardless of whether build_shared_lib option is enabled.

Hello George. To build the CPP samples that are already available here https://github.com/microsoft/onnxruntime/tree/master/samples/c_cxx, we need the libonnxruntime.so shared lib to be linked while compiling along with two other shared libs (libonnxruntime_providers_shared.so and libonnxruntime_providers_openvino.so).

Without this libonnxruntime.so lib, when we try compiling, it doesn't get compiled with this error.
/tmp/cctSSMOx.o: In function __static_initialization_and_destruction_0(int, int)': squeezenet_cpp_app.cpp:(.text+0x1c03): undefined reference to OrtGetApiBase'
collect2: error: ld returned 1 exit status

so, we thought of adding --build_shared_lib flag as default while compiling OpenVINO-EP, so that the developers won't have issues building CPP samples.

we added the same flag inside the docker files which are for building python bindings/package because anyways the onnxruntime dir gets deleted as part of optimization at this line

cd ${MY_ROOT}/ && rm -rf onnxruntime && cd /opt && rm -rf v1.0.22.zip && cd ${MY_ROOT} &&\
.

so, if some user wants to use the same docker file to build CPP samples, all he has to do is remove this step.

@HectorSVC
Copy link
Contributor

/azp run Linux CPU CI Pipeline,Linux CPU x64 NoContribops CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,MacOS CI Pipeline,MacOS NoContribops CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@jywu-msft
Copy link
Member

jywu-msft commented Feb 5, 2021

is it really necessary to change the dockerfiles? (which are for building python bindings/package)
the build_shared_lib option is for onnxruntime lib.
openvino ep gets built as a shared lib regardless of whether build_shared_lib option is enabled.

Hello George. To build the CPP samples that are already available here https://github.com/microsoft/onnxruntime/tree/master/samples/c_cxx, we need the libonnxruntime.so shared lib to be linked while compiling along with two other shared libs (libonnxruntime_providers_shared.so and libonnxruntime_providers_openvino.so).

Without this libonnxruntime.so lib, when we try compiling, it doesn't get compiled with this error.
/tmp/cctSSMOx.o: In function __static_initialization_and_destruction_0(int, int)': squeezenet_cpp_app.cpp:(.text+0x1c03): undefined reference to OrtGetApiBase' collect2: error: ld returned 1 exit status

so, we thought of adding --build_shared_lib flag as default while compiling OpenVINO-EP, so that the developers won't have issues building CPP samples.

we added the same flag inside the docker files which are for building python bindings/package because anyways the onnxruntime dir gets deleted as part of optimization at this line

cd ${MY_ROOT}/ && rm -rf onnxruntime && cd /opt && rm -rf v1.0.22.zip && cd ${MY_ROOT} &&\

.
so, if some user wants to use the same docker file to build CPP samples, all he has to do is remove this step.

yes, understood that the CPP sample needs the libonnxruntime.so
On the other hand, I was questioning the usefulness of adding the build_shared_lib option in the dockerfile.openvino (whose main purpose is to build and install the python package).
As you mentioned, the build directory gets deleted so the libonnxruntime.so would also get removed.

@jywu-msft
Copy link
Member

jywu-msft commented Feb 5, 2021

Azure Pipelines successfully started running 9 pipeline(s).

is it really necessary to change the dockerfiles? (which are for building python bindings/package)
the build_shared_lib option is for onnxruntime lib.
openvino ep gets built as a shared lib regardless of whether build_shared_lib option is enabled.

Hello George. To build the CPP samples that are already available here https://github.com/microsoft/onnxruntime/tree/master/samples/c_cxx, we need the libonnxruntime.so shared lib to be linked while compiling along with two other shared libs (libonnxruntime_providers_shared.so and libonnxruntime_providers_openvino.so).
Without this libonnxruntime.so lib, when we try compiling, it doesn't get compiled with this error.
/tmp/cctSSMOx.o: In function __static_initialization_and_destruction_0(int, int)': squeezenet_cpp_app.cpp:(.text+0x1c03): undefined reference to OrtGetApiBase' collect2: error: ld returned 1 exit status
so, we thought of adding --build_shared_lib flag as default while compiling OpenVINO-EP, so that the developers won't have issues building CPP samples.
we added the same flag inside the docker files which are for building python bindings/package because anyways the onnxruntime dir gets deleted as part of optimization at this line

cd ${MY_ROOT}/ && rm -rf onnxruntime && cd /opt && rm -rf v1.0.22.zip && cd ${MY_ROOT} &&\

.
so, if some user wants to use the same docker file to build CPP samples, all he has to do is remove this step.

yes, understood that the CPP sample needs the libonnxruntime.so
On the other hand, I was questioning the usefulness of adding the build_shared_lib option in the dockerfile.openvino (whose main purpose is to build and install the python package).
As you mentioned, the build directory gets deleted so the libonnxruntime.so would also get removed.

ok i missed your comment that the user would need to manually leave out the step which removes the build directory.
yes, i suppose that is true. you might explicitly add a comment explaining that.
otherwise i doubt the user would know this up front.

@HectorSVC
Copy link
Contributor

is it really necessary to change the dockerfiles? (which are for building python bindings/package)
the build_shared_lib option is for onnxruntime lib.
openvino ep gets built as a shared lib regardless of whether build_shared_lib option is enabled.

Hello George. To build the CPP samples that are already available here https://github.com/microsoft/onnxruntime/tree/master/samples/c_cxx, we need the libonnxruntime.so shared lib to be linked while compiling along with two other shared libs (libonnxruntime_providers_shared.so and libonnxruntime_providers_openvino.so).
Without this libonnxruntime.so lib, when we try compiling, it doesn't get compiled with this error.
/tmp/cctSSMOx.o: In function __static_initialization_and_destruction_0(int, int)': squeezenet_cpp_app.cpp:(.text+0x1c03): undefined reference to OrtGetApiBase' collect2: error: ld returned 1 exit status
so, we thought of adding --build_shared_lib flag as default while compiling OpenVINO-EP, so that the developers won't have issues building CPP samples.
we added the same flag inside the docker files which are for building python bindings/package because anyways the onnxruntime dir gets deleted as part of optimization at this line

cd ${MY_ROOT}/ && rm -rf onnxruntime && cd /opt && rm -rf v1.0.22.zip && cd ${MY_ROOT} &&\

.
so, if some user wants to use the same docker file to build CPP samples, all he has to do is remove this step.

yes, understood that the CPP sample needs the libonnxruntime.so
On the other hand, I was questioning the usefulness of adding the build_shared_lib option in the dockerfile.openvino (whose main purpose is to build and install the python package).
As you mentioned, the build directory gets deleted so the libonnxruntime.so would also get removed.

I think what they mean is that some user reuse same docker file for development, the developer will comment out the line which delete the build directory. For normal user build docker image directly from the original docker file, they won't aware of the libonnxruntime.so file since it will be deleted anyway.

@HectorSVC
Copy link
Contributor

/azp run orttraining-linux-ci-pipeline,orttraining-mac-ci-pipeline,orttraining-linux-gpu-ci-pipeline,centos7_cpu,Linux CPU Minimal Build E2E CI Pipeline,Linux Nuphar CI Pipeline,MacOS NoContribops CI Pipeline,orttraining-distributed

@azure-pipelines
Copy link

Azure Pipelines successfully started running 8 pipeline(s).

@MaajidKhan
Copy link
Contributor

is it really necessary to change the dockerfiles? (which are for building python bindings/package)
the build_shared_lib option is for onnxruntime lib.
openvino ep gets built as a shared lib regardless of whether build_shared_lib option is enabled.

Hello George. To build the CPP samples that are already available here https://github.com/microsoft/onnxruntime/tree/master/samples/c_cxx, we need the libonnxruntime.so shared lib to be linked while compiling along with two other shared libs (libonnxruntime_providers_shared.so and libonnxruntime_providers_openvino.so).
Without this libonnxruntime.so lib, when we try compiling, it doesn't get compiled with this error.
/tmp/cctSSMOx.o: In function __static_initialization_and_destruction_0(int, int)': squeezenet_cpp_app.cpp:(.text+0x1c03): undefined reference to OrtGetApiBase' collect2: error: ld returned 1 exit status
so, we thought of adding --build_shared_lib flag as default while compiling OpenVINO-EP, so that the developers won't have issues building CPP samples.
we added the same flag inside the docker files which are for building python bindings/package because anyways the onnxruntime dir gets deleted as part of optimization at this line

cd ${MY_ROOT}/ && rm -rf onnxruntime && cd /opt && rm -rf v1.0.22.zip && cd ${MY_ROOT} &&\

.
so, if some user wants to use the same docker file to build CPP samples, all he has to do is remove this step.

yes, understood that the CPP sample needs the libonnxruntime.so
On the other hand, I was questioning the usefulness of adding the build_shared_lib option in the dockerfile.openvino (whose main purpose is to build and install the python package).
As you mentioned, the build directory gets deleted so the libonnxruntime.so would also get removed.

I think what they mean is that some user reuse same docker file for development, the developer will comment out the line which delete the build directory. For normal user build docker image directly from the original docker file, they won't aware of the libonnxruntime.so file since it will be deleted anyway.

Right Hector. This is the main reason, we added the option in the docker file. so, that we don't have to explicitly tell the user every time that they need to add this flag themselves if they want to work with CPP samples.

@HectorSVC HectorSVC merged commit 973c391 into microsoft:master Feb 5, 2021
@preetha-intel preetha-intel deleted the sahar/indentation_fix branch December 29, 2022 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ep:OpenVINO issues related to OpenVINO execution provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants