-
Notifications
You must be signed in to change notification settings - Fork 320
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
Modify RunONNXModel.py for pip install #2857
Conversation
@chentong319 Can you add a reference to the ORT interface that this PR is imitating. In your first example
was the |
Yes, onnx_model is not needed if the compiled .so is provided. By the way, the two onnxmilrrun.RunONNXModel.onnxmlirrun commands are two independent ways to do inference. |
Got it, so we can provide a onnx file or a pre-compiled binary, smart, thanks. |
utils/RunONNXModel.py
Outdated
if onnx_model : | ||
args.model = onnx_model | ||
if compiled_so : | ||
args.load_so = compiled_so |
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.
Should we check if both onnx_model
and compiled_so
are not given?
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.
Make sense, and maybe if none are given, an error?
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 added the check whether both onnx_mdel and compiled_so are given or not give (XOR).
My two cents here. For the package name (the first onnxmlirrun), just Anyway, we have several python utilities, perhaps it's time to reorganize them into a single python package. |
utils/onnxmlir/VERSION_NUMBER
Outdated
@@ -0,0 +1 @@ | |||
1.0.0 |
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.
Version number here is different than onnx-mlir/VERSION_NUMBER
. Is that on purpose?
Also the number here is different than the version listed in the file below.
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.
Fixed. I used a version number different from the onnx-mlir. The version for onnxmlir is for upstream onnxmlir to pip repo. It could be viewed as a pre-required package.
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.
May I suggest a 0.1.0
and move to a 1.0.0
once we have the installation of the compiler with it at least one z?
@@ -0,0 +1,959 @@ | |||
#!/usr/bin/env python3 |
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.
Is this an identical copy? If so, at a minimum there should be a comment stating that they should stay in sync. Or maybe we could have a single version and we could "build" the pip structure by copying files in the right place. That would be my preference.
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.
Added a target in CMake to check the 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.
Check error message if file does not exist.
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.
TODO: let's try to have parallel default and/or as set in onnxruntime.
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.
TODO: sync also with #2899 with respect to catching errors/crasshes
onnx-mlir will report the error if file does not exist. Leave this to the TODO list to catch all the errors gracefully. |
utils/onnxmlir/VERSION_NUMBER
Outdated
@@ -0,0 +1 @@ | |||
1.0.0 |
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.
May I suggest a 0.1.0
and move to a 1.0.0
once we have the installation of the compiler with it at least one z?
utils/onnxmlir/pyproject.toml
Outdated
build-backend = "hatchling.build" | ||
[project] | ||
name = "onnxmlir" | ||
version = "1.0.0" |
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.
Thanks for having it in sync.
Signed-off-by: chentong319 <chentong@us.ibm.com>
@jenkins-droid test it! |
@jenkins-droid test this please |
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, thanks for the updates
Signed-off-by: chentong319 <chentong@us.ibm.com>
Jenkins Linux ppc64le Build #14467 [push] Modify RunONNXModel.py f... started at 11:56 |
Jenkins Linux amd64 Build #15437 [push] Modify RunONNXModel.py f... started at 10:46 |
Jenkins Linux s390x Build #15444 [push] Modify RunONNXModel.py f... started at 11:46 |
Jenkins Linux amd64 Build #15437 [push] Modify RunONNXModel.py f... passed after 1 hr 7 min |
Jenkins Linux s390x Build #15444 [push] Modify RunONNXModel.py f... passed after 1 hr 29 min |
Jenkins Linux ppc64le Build #14467 [push] Modify RunONNXModel.py f... passed after 2 hr 1 min |
The goal is to create a python package so that we can run an inference like onnxruntime in python script. Since the RunONNXModel.py provides most of the necessary functionality, I added extra support in that script. I tested with a local pip install. We can upload the package later.
Major changes:
Example:
We may have different name for the functions and the package. Comments are welcome.