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 4 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
17 changes: 10 additions & 7 deletions docker/ray-ml/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@ 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
-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 \
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
7 changes: 2 additions & 5 deletions python/requirements/ml/requirements_dl.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
# 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~=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.0

torch==1.9.0;sys_platform=="darwin"
Expand All @@ -16,5 +14,4 @@ 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"
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