-
Notifications
You must be signed in to change notification settings - Fork 329
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
Build light weight PyRuntime without llvm or onnx-mlir #3044
Conversation
Signed-off-by: Chen Tong <[email protected]>
Signed-off-by: Chen Tong <[email protected]>
Signed-off-by: Chen Tong <[email protected]>
Signed-off-by: Chen Tong <[email protected]>
Signed-off-by: Chen Tong <[email protected]>
Signed-off-by: Chen Tong <[email protected]>
Signed-off-by: Chen Tong <[email protected]>
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.
Still very early review; let me get the overall picture.
How would the new code base change if we were to adapt it for PyTorch as well? Namely, what is common to any "docker compilation + pyruntime_lit" vs what is specialized for "ORT" and what would be specialized for PyTorch?
Do you mind giving me this high level info, and then I will be able to better understand the overall approach. Thanks
CMakeLists.txt
Outdated
@@ -11,6 +11,7 @@ option(ONNX_MLIR_ENABLE_STABLEHLO "Enable StableHLO support." ON) | |||
option(ONNX_MLIR_ENABLE_WERROR "Enable warnings as errors." OFF) | |||
option(ONNX_MLIR_SUPPRESS_THIRD_PARTY_WARNINGS "Suppress warning in third_party code." ON) | |||
option(ONNX_MLIR_ENABLE_JAVA "Set to ON for building the Java runtime, tools, and tests" ON) | |||
option(ONNX_MLIR_ENABLE_PYRUNTIME_LIT "Set to ON for building Python driver of running the compiled model without llvm-project." OFF) |
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.
Maybe the name should be explained: if off, then no pyruntime is build? Or its build anyway, but when on, then it's only the pyruntime? Or when off, pyruntime is build one way, but when off, its build another way?
Maybe the name could be a bit more explicit, depending on what the answer is from the question above.
minor question: _LIT
is it for "_LIGHT"?
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.
When this option is off, the pyruntime is built with onnx-mlir and llvm-project, as it was previously.
When this option is on, only the pyruntime is built without llvm-project.
Yes, LIT for LIGHT.
CMakeLists.txt
Outdated
add_subdirectory(src) | ||
add_subdirectory(docs) | ||
add_subdirectory(test) | ||
if (ONNX_MLIR_ENABLE_PYRUNTIME_LIT) |
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.
I see this (and above): there are some dir that added on both path. Is it that the order of them is important?
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.
Only the src
is added on both path. I do not think the order of add_subdirectory matters. Just to keep the original add_subdirectory together.
) | ||
endif() | ||
if (ONNX_MLIR_ENABLE_PYRUNTIME_LIT) | ||
function(llvm_update_compile_flags name) |
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.
Can you add a one liner comment on why this function is defined here.
@@ -0,0 +1,151 @@ | |||
import numpy as np |
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.
That file is totally new, is that right?
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.
Yes, it is the file to use docker package.
This docker compilation + pyruntime_lit provides the basic functionality for compilation and run. The interface for ORT or PyTorch will be in the python package built on top of them. This PR contains only the package, onnxmlir, with ORT interface. In future, another package for PyTorch interface will be provided. |
Signed-off-by: Chen Tong <[email protected]>
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.
LGTM as a first pass, will wait for additional feedback from the team as we develop this further. Thanks for taking this great first step @chentong319
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.
LGTM!
Hope that the environment variable ENABLE_PYRUNTIME_LIT
will be removed completely in next PRs.
LIT in ONNX_MLIR_ENABLE_PYRUNTIME_LIT
is confusing with LIT tests. It would be better to use other word, e.g. LIGHT or to remove it.
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.
LGTM!
The definition, ENABLE_PYRUNTIME_LIT, will be away finally. Current version is the transition from the previous one and the new one. |
Signed-off-by: Chen Tong <[email protected]>
Jenkins Linux s390x Build #16181 [push] Build light weight PyRun... started at 13:38 |
Jenkins Linux amd64 Build #16180 [push] Build light weight PyRun... started at 12:39 |
Jenkins Linux ppc64le Build #15209 [push] Build light weight PyRun... started at 13:58 |
Jenkins Linux amd64 Build #16180 [push] Build light weight PyRun... passed after 1 hr 30 min |
Jenkins Linux s390x Build #16181 [push] Build light weight PyRun... passed after 1 hr 47 min |
Jenkins Linux ppc64le Build #15209 [push] Build light weight PyRun... passed after 2 hr 54 min |
* pass test Signed-off-by: Chen Tong <[email protected]> * package Signed-off-by: Chen Tong <[email protected]> * clean makefile Signed-off-by: Chen Tong <[email protected]> * document Signed-off-by: Chen Tong <[email protected]> * fix MLIR.cmake Signed-off-by: Chen Tong <[email protected]> * fix script Signed-off-by: Chen Tong <[email protected]> * fix Signed-off-by: Chen Tong <[email protected]> * add comments Signed-off-by: Chen Tong <[email protected]> * LIGHT Signed-off-by: Chen Tong <[email protected]> --------- Signed-off-by: Chen Tong <[email protected]>
Motivation
Python driver is needed to run the compiled model. Currently, the driver is built with onnx-mlir and can only be run in the env where onnx-mlir is built, typically inside the onnx-mlir docker image. When the compilation can be done by calling the onnx-mlir docker image, we'd like to run the compiled .so with python driver in the local env, so that all the packages installed in the local env can be used, rather than installing them on top of docker.
In order to reach this goal, the PR tried to remove the unnecessary dependencies of pyruntime if the light-weight pyruntime is the target. Otherwise, there is no change in how onnx-mlir is built and used.
Details can be found in docs/build-pyruntime-lit.md.
I tried the build on a z16 machine: it takes less than 2 minutes.
Components in this PR
ONNX_MLIR_ENABLE_PYRUNTIME_LIT
is used to control Cmake, and consequently a compile definitionENABLE_PYRUNTIME_LIT
issued to control the source code.Test
Run successfully with utils/BuildPyRuntimeLit.sh.
Future works: