Closed dtrawins closed 6 months ago
libtbb.so
is being copied from a 3rd party directory instead of apt-get repository. So, my understanding is that it might not be backwards compatible.
We can conditionalize the install instructions over the version flag value to build for both the versions. However, I would like to avoid it.
@dtrawins Does ONNXRuntime OpenVINO execution provider support OpenVINO 2024.0 version? ref: https://github.com/triton-inference-server/server/blob/main/build.py#L66-L69
@tanmayv25 is there a different way to decouple the backend version upgrade from build.py
@nnshah1 There should be a way to properly separate OV symbols coming from openvino backend with those coming from openvino execution provider within ONNXruntime. We did try RTLD_LOCAL flag with dlopen however we still observed some conflicts. Would need to take a closer look into what's going wrong.
@nnshah1 There should be a way to properly separate OV symbols coming from openvino backend with those coming from openvino execution provider within ONNXruntime. We did try RTLD_LOCAL flag with dlopen however we still observed some conflicts. Would need to take a closer look into what's going wrong.
I'm also just wondering if we can check in a change to upgrade version to the backend without immediately affecting the server build - meaning can we stage them independently - or does the server build.py have to upgraded immediately at the same time?
@tanmayv25 This change should be backward compatible. I tested it both using openvino 2023.3 and 2024.0. tbb is compiled from sources and added to third-party directly in the build time when apt package is missing. This behavior was also in previous releases. The key change is for ngraph includes which are missing in 2024.0. For compatibility reasons there is used a wildcard so the copy will work with and without ngraph.
I guess ONNX needs to be validated. Would it be sufficient for build triton with both OV and ONNX backends?
@dtrawins , @tanmayv25 , @kthui
One general question: Is this change backwards compatible? Meaning if we merge this first before updating the version in tritonserver build.py repository will it still build / pass ci tests?
If not is there a way to modify it so that it can?
Yes, we should merge this PR before updating the version in the server repo. It can work with the current and new OV versions.
I'm also just wondering if we can check in a change to upgrade version to the backend without immediately affecting the server build - meaning can we stage them independently - or does the server build.py have to upgraded immediately at the same time?
build.py
configures the build parameters using cmake flags. Currently all the versions are held in server repository, these versions are directly supplied to the cmake configuration at the build time.
I think what you are proposing is that this version should come from the respective backends itself such that they are truly decoupled? We can play with default versions for this cmake options to build with specific version and build.py should not override these defaults. If the user is building a backend by themselves then they are free to provide any version.
Cons: There is no central place where you can go an update the library versions. We would have to visit each repsository to view/update the library versions.
@tanmayv25 It was not my intention. build.py should be still the central place for setting the versions. What I meant is the latest backend code might be not compatible with all possible OV versions from the past. Specifically there is one change with ngraph includes in 2024.0 which is incompatible with the current docker file building the backend. I made a change in the dockerfile to be compatible with 2023.3 and also 2024.0. It should be compatible also with a range of older version but I wasn't testing them all... I would assume the compatibility should at least with the current and the new OV version. The proposal is to merge that PR in the backed which would be still compatible with 2023.3 used in build.py and it would allow testing and merging the PR from server repo to update OV in the build.py.
Thanks @dtrawins ! I was responding to Neelay's question on the requirement of keeping build.py versions updated as soon as version in the backend was updated.
I made a change in the dockerfile to be compatible with 2023.3 and also 2024.0.
I believe this is what is needed for now. The changes in this PR will not impact our CI. It just enables us to upgrade to 2024.0 at some point.
@kthui is working on a running a pipeline with these changes to verify it. As soon as we get a clean pipeline. We will merge the change.
That being said, the windows build is failing with these changes:
Step 12/24 : RUN ren w_openvino_toolkit_windows_2024.0.0.14509.34caeefd078_x86_64 install
---> Running in adebabb58410
Removing intermediate container adebabb58410
---> ad7da4430460
Step 13/24 : WORKDIR /opt/openvino
---> Running in d686f2df2c40
Removing intermediate container d686f2df2c40
---> e809ec7fe529
Step 14/24 : RUN xcopy /I /E \workspace\install\docs\licensing LICENSE.openvino
---> Running in 08fa4e1bf7d5
\workspace\install\docs\licensing\Apache_license.txt
\workspace\install\docs\licensing\documentation-third-party-programs.txt
\workspace\install\docs\licensing\EULA.htm
\workspace\install\docs\licensing\EULA.rtf
\workspace\install\docs\licensing\EULA.txt
\workspace\install\docs\licensing\Intel_Software_Development_Products.rtf
\workspace\install\docs\licensing\Intel_Software_Development_Products.txt
\workspace\install\docs\licensing\LICENSE
\workspace\install\docs\licensing\onednn_third-party-programs.txt
\workspace\install\docs\licensing\onetbb_third-party-programs.txt
\workspace\install\docs\licensing\OpenVINOsupport.txt
\workspace\install\docs\licensing\readme.txt
\workspace\install\docs\licensing\redist.txt
\workspace\install\docs\licensing\runtime-third-party-programs.txt
14 File(s) copied
Removing intermediate container 08fa4e1bf7d5
---> ba97d8dd2f63
Step 15/24 : RUN mkdir include
---> Running in ba34ccda805f
Removing intermediate container ba34ccda805f
---> a19f57a9cfe9
Step 16/24 : RUN xcopy /I /E \workspace\install\runtime\include\ie include
---> Running in 9828fffab893
[91mFile not found - ie
[0m0 File(s) copied
The command 'cmd /S /C xcopy /I /E \workspace\install\runtime\include\ie include' returned a non-zero code: 4
17>C:\BuildTools\MSBuild\Microsoft\VC\v170\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'C:\tmp\tritonbuild\openvino\build\CMakeFiles\af8f2d9c639d0decc8b88db13fc2f5f6\openvino.lib.rule;C:\tmp\tritonbuild\openvino\build\CMakeFiles\294a404cc8abb0ec4b6304822fe5c224\ov_target.rule;C:\tmp\tritonbuild\openvino\CMakeLists.txt' exited with code 4. [C:\tmp\tritonbuild\openvino\build\ov_target.vcxproj]
17>Done Building Project "C:\tmp\tritonbuild\openvino\build\ov_target.vcxproj" (default targets) -- FAILED.
@dtrawins , @tanmayv25 , @kthui
One general question: Is this change backwards compatible? Meaning if we merge this first before updating the version in tritonserver build.py repository will it still build / pass ci tests?
If not is there a way to modify it so that it can?