Skip to content

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

Merged
merged 60 commits into from
Oct 14, 2021
Merged

changed PT and TF versions #806

merged 60 commits into from
Oct 14, 2021

Conversation

DvirDukhan
Copy link
Collaborator

@DvirDukhan DvirDukhan commented Jul 8, 2021

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.

@DvirDukhan DvirDukhan changed the title changed PT and TF versions - WIP DRAFT changed PT and TF versions Jul 11, 2021
@DvirDukhan DvirDukhan marked this pull request as ready for review July 11, 2021 08:56
@codecov
Copy link

codecov bot commented Jul 11, 2021

Codecov Report

Merging #806 (b8f34ec) into master (f3e815e) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            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              
Impacted Files Coverage Δ
...backends/libtorch_c/torch_extensions/torch_redis.h 90.90% <ø> (ø)
src/backends/libtorch_c/torch_c.cpp 64.20% <100.00%> (+0.17%) ⬆️
...ckends/libtorch_c/torch_extensions/torch_redis.cpp 94.94% <100.00%> (+0.21%) ⬆️
src/backends/torch.c 84.24% <100.00%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7aca1eb...b8f34ec. Read the comment docs.

chayim
chayim previously approved these changes Jul 11, 2021
alonre24
alonre24 previously approved these changes Jul 11, 2021
lantiga
lantiga previously approved these changes Jul 12, 2021
Copy link
Contributor

@lantiga lantiga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@chayim chayim dismissed stale reviews from lantiga, alonre24, and themself via 2b61d9f July 12, 2021 09:58
@alonre24 alonre24 added ci-test and removed ci-test labels Oct 12, 2021
@alonre24 alonre24 requested a review from chayim October 12, 2021 15:50
Copy link
Collaborator Author

@DvirDukhan DvirDukhan left a 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"
Copy link
Collaborator Author

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?

Copy link
Collaborator

@alonre24 alonre24 Oct 13, 2021

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" %}
Copy link
Contributor

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.

Copy link
Collaborator

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.


-------

## 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.
Copy link
Contributor

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.

sudo apt install python3 python3-dev make docker
python3 -m venv /path/to/venv
source /path/to/venv/bin/activate
pip install jinja
Copy link
Contributor

@chayim chayim Oct 13, 2021

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


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.
Copy link
Contributor

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.

Copy link
Contributor

@chayim chayim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestions for changes

@alonre24 alonre24 added ci-test and removed ci-test labels Oct 13, 2021
@alonre24 alonre24 added ci-test and removed ci-test labels Oct 14, 2021
@DvirDukhan DvirDukhan merged commit 8fc77bc into master Oct 14, 2021
@DvirDukhan DvirDukhan deleted the backends_update branch October 14, 2021 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants