-
Notifications
You must be signed in to change notification settings - Fork 106
changed PT and TF versions #806
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #806 +/- ##
==========================================
+ Coverage 81.33% 81.34% +0.01%
==========================================
Files 56 56
Lines 8185 8192 +7
==========================================
+ Hits 6657 6664 +7
Misses 1528 1528
Continue to review full report at Codecov.
|
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.
Looks good!
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.
looks good
small comment
###### SET VERSIONS ###### | ||
|
||
ORT_VERSION="1.9.0" | ||
DLPACK_VERSION="v0.5_RAI" |
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.
are we on 0.5 or 0.6?
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.
dlack released 0.6 about 3 months ago but nothing has changed except for the version number. Our fork is up to date (except for our additions), so I don't see a reason to upgrade our fork at this point...
@@ -30,8 +30,8 @@ ARG ONNXRUNTIME_VER={{REDIS_ONNX_VERSION}} | |||
# build | |||
WORKDIR /build | |||
{% if REDIS_GPU is defined %} | |||
{% set BUILDTYPE="MinSizeRel" %} | |||
{% set BUILDARGS="--use_cuda --cudnn_home /usr/local/cuda --cuda_home /usr/local/cuda" %} | |||
{% set BUILDTYPE="Release" %} |
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.
Are you sure about this? I admit that I don't know - but literally lifted it from the old docker builds.
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's OK, I checked it.
docs/developer-backends.md
Outdated
|
||
------- | ||
|
||
## Backends | ||
|
||
### onnxruntime | ||
|
||
We build Onnxruntime library with DISABLE_EXTERNAL_INITIALIZERS=ON build flag. This means that loading ONNX models that use external files to store the initial (usually very large) values of the model's operations, is invalid. That is, initializers values must be part of the serialized model, which is also the standard use case. |
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.
We build the onnxruntime library with the DISABLE_EXTERNAL_INITIALIZERS=ON build flag. As a result, loading ONNX models that use external files to store the initial (usually very large) values of the model's operations, is invalid. Hence, initialization values must be part of the serialized model, which is also the standard use case.
docs/developer-backends.md
Outdated
sudo apt install python3 python3-dev make docker | ||
python3 -m venv /path/to/venv | ||
source /path/to/venv/bin/activate | ||
pip install jinja |
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.
sudo apt install python3 python3-dev make docker
python3 -m venv /path/to/venv
source /path/to/venv/bin/activate
pip install jinja2
docs/developer-backends.md
Outdated
|
||
GNU Make is used as a runner for the dockerfile generator. Python is the language used for the generator script, and jinja is the templating library used to create the docker file from the template. | ||
GNU Make is used as a runner for the dockerfile generator. Python is the language used for the generator script, and jinja is the templating library used to create the docker file from a template *dockerfile.tmpl* that can be found in the directory of a given backend listed 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.
GNU Make is used as a runner for the dockerfile generator. Python is the language used for the generator script, and jinja is the templating library used to create the docker file from the template *dockerfile.tmpl*, located in the directory for the backends listed 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.
Suggestions for changes
Upgrade to TF 2.6, PT 1.9.
TF requires CUDA 11.2, so we update CI image for testing with GPU.
In one test, error message is changed in GPU version of TF due to the upgrade.
Here we also remove all the build (and fetch) logic for OS and architectures that are not in used for our backends currently (also update documentation accordingly). The work that been done so far is in #851 draft PR, from which we can continue in the future for adding such support.