Skip to content
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

Clean up ray-ml requirements #23325

Merged
merged 18 commits into from
Mar 25, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions docker/ray-ml/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,26 @@ RUN sudo apt-get update \
libgl1-mesa-dev \
unzip \
unrar \
&& $HOME/anaconda3/bin/pip install -U pip \
&& $HOME/anaconda3/bin/pip --use-deprecated=legacy-resolver --no-cache-dir install -r requirements.txt \
&& $HOME/anaconda3/bin/pip --no-cache-dir install -U -r requirements_rllib.txt \
&& $HOME/anaconda3/bin/pip --no-cache-dir install -U -r requirements_train.txt \
&& $HOME/anaconda3/bin/pip --no-cache-dir install -U -r requirements_tune.txt \
&& $HOME/anaconda3/bin/pip --no-cache-dir install -U -r requirements_ml_docker.txt \
&& $HOME/anaconda3/bin/pip --no-cache-dir install -U -r requirements_upstream.txt \
&& $HOME/anaconda3/bin/pip --no-cache-dir install -U pip \
&& $HOME/anaconda3/bin/pip --no-cache-dir install -U \
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice thanks for doing this!

-r requirements.txt \
-r requirements_rllib.txt \
-r requirements_train.txt \
-r requirements_tune.txt \
-r requirements_upstream.txt \
# explicitly install (overwrite) pytorch with CUDA support
&& $HOME/anaconda3/bin/pip --no-cache-dir install -U \
-r requirements_ml_docker.txt \
# Remove dataclasses & typing because they are included in Python > 3.6
&& if [ $(python -c 'import sys; print(sys.version_info.minor)') != "6" ]; then \
$HOME/anaconda3/bin/pip uninstall dataclasses typing -y; fi \
&& sudo rm requirements.txt && sudo rm requirements_ml_docker.txt \
&& sudo rm requirements_tune.txt && sudo rm requirements_rllib.txt \
&& sudo rm requirements_train.txt \
&& sudo rm requirements_upstream.txt \
&& sudo rm \
requirements.txt \
requirements_ml_docker.txt \
requirements_tune.txt \
requirements_rllib.txt \
requirements_train.txt \
requirements_upstream.txt \
&& sudo apt-get clean

# Make sure tfp is installed correctly and matches tf version.
Expand Down
2 changes: 1 addition & 1 deletion python/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pytest-lazy-fixture
pytest-timeout
pytest-virtualenv
scikit-learn==0.22.2
tensorflow==2.5.1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

does this change need to be documented anywhere?

tensorflow
ddelange marked this conversation as resolved.
Show resolved Hide resolved
testfixtures
werkzeug
xlrd
Expand Down
15 changes: 5 additions & 10 deletions python/requirements/ml/requirements_dl.txt
Original file line number Diff line number Diff line change
@@ -1,20 +1,15 @@
# These requirements are used for the CI and CPU-only Docker images so we install CPU only versions of torch.
# For GPU Docker images, you should install requirements_ml_docker.txt instead.
# For GPU Docker images, you should install requirements_ml_docker.txt afterwards.

tensorflow~=2.6.2
Copy link
Contributor Author

@ddelange ddelange Mar 18, 2022

Choose a reason for hiding this comment

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

maybe remove here and only have it only in python/requirements.txt?

Copy link
Contributor Author

@ddelange ddelange Mar 18, 2022

Choose a reason for hiding this comment

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

Anything against allowing tensorflow 2.7 or 2.8 btw? Probably one of the breaking changes in 2.7? Or is it pinned to sync with tensorflow-probability releases?

Copy link
Contributor

Choose a reason for hiding this comment

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

Usually it's a breaking change but cc @sven1977 for more details

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this sync up is necessary indeed. There's a note in their docs:

Note: Since TensorFlow is not included as a dependency of the TensorFlow Probability package (in setup.py), you must explicitly install the TensorFlow package (tensorflow or tensorflow-gpu). This allows us to maintain one package instead of separate packages for CPU and GPU-enabled TensorFlow.

Which is actually outdated as tensorflow-gpu is deprecated. They could simply have the correct tensorflow requirement range specified in the probability package requirements, and there would be no need to manually sync up here anymore (i.e. simply only specify tensorflow-probability==XXX in python/requirements.txt).

tensorflow-probability~=0.14.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Is keras installed as a dependency in tensorflow automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exactly. There was a small problem back in the day (#20032) for a single version, but that's fixed and so tensorflow will request the right version of keras

ddelange marked this conversation as resolved.
Show resolved Hide resolved

# If you make changes to this file, please also change `requirements_ml_docker.txt`!


# TODO(amogkam): Remove after https://github.com/tensorflow/tensorflow/issues/52922 is fixed.
keras==2.6.0
tensorflow==2.6.0
tensorflow-probability==0.14.0
# If you make changes below this line, please also change `requirements_ml_docker.txt`!

torch==1.9.0;sys_platform=="darwin"
torchvision==0.10.0;sys_platform=="darwin"

# On non-OSX machines only install CPU version of torch and torchvision
-f https://download.pytorch.org/whl/torch_stable.html
torch==1.9.0+cpu;sys_platform!="darwin"
-f https://download.pytorch.org/whl/torch_stable.html
torchvision==0.10.0+cpu;sys_platform!="darwin"
torchvision==0.10.0+cpu;sys_platform!="darwin"
2 changes: 1 addition & 1 deletion python/requirements/ml/requirements_rllib.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ higher==0.2.1
pyglet==1.5.15
imageio-ffmpeg==0.4.5
# For JSON reader/writer.
ddelange marked this conversation as resolved.
Show resolved Hide resolved
smart_open==5.0.0
smart_open~=5.0
ddelange marked this conversation as resolved.
Show resolved Hide resolved
# Ray Serve example
starlette==0.16.0
# ONNX
Expand Down
7 changes: 0 additions & 7 deletions python/requirements_ml_docker.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,6 @@ tblib

# If you make changes to anything below this line, please also change `requirements_dl.txt`!

# TODO(amogkam): Remove after https://github.com/tensorflow/tensorflow/issues/52922 is fixed.
keras==2.6.0
# In TF >v2, GPU support is included in the base package.
tensorflow==2.6.0
tensorflow-probability==0.14.0

-f https://download.pytorch.org/whl/torch_stable.html
torch==1.9.0+cu111
-f https://download.pytorch.org/whl/torch_stable.html
torchvision==0.10.0+cu111