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

Add support for python3.11 #6288

Merged
merged 21 commits into from Oct 23, 2023
Merged

Conversation

OlivierLDff
Copy link
Contributor

@OlivierLDff OlivierLDff commented Jul 28, 2023

Type

  • Bug fix (non-breaking change which fixes an issue): Fixes #
  • New feature (non-breaking change which adds functionality). Resolves #
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

Fix #5620

Checklist:

  • I have run python util/check_style.py --apply to apply Open3D code style
    to my code.
  • This PR changes Open3D behavior or adds new functionality.
    • Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
      updated accordingly.
    • I have added or updated C++ and / or Python unit tests OR included test
      results
      (e.g. screenshots or numbers) here.
  • I will follow up and update the code if CI fails.
  • For fork PRs, I have selected Allow edits from maintainers.

Description

  • Update the linux/mac/windows CI to handle python 3.11
  • Update pybind11

I guess some documentation will also be required to be updated somewhere. I am not familiar with the workspace, I just happened to have python3.11 on my machine so wanted to give a go to the update. Maybe there more stuff to update than what I did. Let's see what the CI have to say.

Companion Open3D-ML PR: isl-org/Open3D-ML#619


This change is Reviewable

@update-docs
Copy link

update-docs bot commented Jul 28, 2023

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

@OlivierLDff OlivierLDff marked this pull request as ready for review July 31, 2023 07:39
@ssheorey ssheorey self-requested a review August 9, 2023 04:04
@ssheorey
Copy link
Member

ssheorey commented Aug 9, 2023

Hi @OlivierLDff thanks for this PR! There is some background info and more tasks required for this PR. The reason we were hesitating with adding Python 3.11 support is an incompatibility between PyTorch and Tensorflow dependencies, that we would also need to update. But I think we can bite the bullet and proceed now. Here are the details:

  • Python 3.11 is supported by TF 2.12+ and PyTorch 2.0+ only.
  • PyTorch still uses the old CXX11_ABI in Linux (issue), while TF switched to the new CXX11_ABI since 2.9.
  • We need to pick one CXX11_ABI for Open3D - we have been using the old CXX11_ABI till now and we should continue that till PyTorch also supports the new ABI. Unfortunately, this means temporarily dropping support for TensorFlow.

As a result, please make these additional changes in this PR for proper Python 3.11 support:

  • Update PyTorch to version 2.0.1 that supports Python 3.11
  • Comment out tensorflow dependencies from requirements files. Update the CI to not install / build with tensorflow.

We can also drop support for Python 3.7 in the same PR. Python 3.7 is now EOL.

Also, we need to keep the companion Open3D-ML repo in sync - we will need a companion PR for that repo. Let me know if you want some help in preparing a PR for that.

@OlivierLDff
Copy link
Contributor Author

Ok thanks for the guidance. I'm currently away from any computer for a week, but I will continue this PR next week!
It doesn't seems urgent to update ATM. But if someone wants/need to finalize this this week do not hesitate to superseed this PR.
Have a nice, I will com back with more questions if required when I will get back to it.

To drop python 3.7 is there more than the file I already touched?

@OlivierLDff
Copy link
Contributor Author

Coming back to this PR I now realize it might be harder than what I was expecting, since I'm not familiar with the codebase.

I guess pytorch version has to be updated there:

Open3D/util/ci_utils.sh

Lines 45 to 50 in 1012819

TORCH_CPU_GLNX_VER="1.13.1+cpu"
TORCH_CUDA_GLNX_VER="1.13.1+cu116"
PYTHON_VER=$(python -c 'import sys; ver=f"{sys.version_info.major}{sys.version_info.minor}"; print(f"cp{ver}-cp{ver}{sys.abiflags}")' 2>/dev/null || true)
# TORCH_CUDA_GLNX_URL="https://github.com/isl-org/open3d_downloads/releases/download/torch1.8.2/torch-1.8.2-${PYTHON_VER}-linux_x86_64.whl"
TORCH_MACOS_VER="1.13.1"
TORCH_REPO_URL="https://download.pytorch.org/whl/torch/"

But then should cuda version be updated too?

Open3D/util/ci_utils.sh

Lines 30 to 33 in 1012819

CUDA_VERSION=("11-6" "11.6")
CUDNN_MAJOR_VERSION=8
CUDNN_VERSION="8.4.1.50_cuda11.6"
GCC_MAX_VER=9

11.7 or 11.8 ?

For tensorflow, should it just be commented here:

BUILD_TENSORFLOW_OPS: ${{ matrix.MLOPS }}

@ssheorey
Copy link
Member

Coming back to this PR I now realize it might be harder than what I was expecting, since I'm not familiar with the codebase.

No worries, we'll help as much as required. Thanks for working on this task!

I guess pytorch version has to be updated there:

Open3D/util/ci_utils.sh

Lines 45 to 50 in 1012819

TORCH_CPU_GLNX_VER="1.13.1+cpu"
TORCH_CUDA_GLNX_VER="1.13.1+cu116"
PYTHON_VER=$(python -c 'import sys; ver=f"{sys.version_info.major}{sys.version_info.minor}"; print(f"cp{ver}-cp{ver}{sys.abiflags}")' 2>/dev/null || true)
# TORCH_CUDA_GLNX_URL="https://github.com/isl-org/open3d_downloads/releases/download/torch1.8.2/torch-1.8.2-${PYTHON_VER}-linux_x86_64.whl"
TORCH_MACOS_VER="1.13.1"
TORCH_REPO_URL="https://download.pytorch.org/whl/torch/"

Yes.

But then should cuda version be updated too?

Open3D/util/ci_utils.sh

Lines 30 to 33 in 1012819

CUDA_VERSION=("11-6" "11.6")
CUDNN_MAJOR_VERSION=8
CUDNN_VERSION="8.4.1.50_cuda11.6"
GCC_MAX_VER=9

11.7 or 11.8 ?

Update CUDA to the oldest supported by our target PyTorch version. For 2.0, this is 11.7. Update this and the corresponding cuDNN version in all dockerfiles in the docker folder.

For tensorflow, should it just be commented here:

BUILD_TENSORFLOW_OPS: ${{ matrix.MLOPS }}

This needs to be changed in a few different places (Linux CI is all in docker):

  • docker/Dockerfile.wheel is used to build the Open3D wheel. Set this to OFF here:
    && export BUILD_TENSORFLOW_OPS=ON \
  • Disable TF for various CI configurations. These are all run in docker controlled by the file: docker/docker_build.sh Turn all of them OFF there and a comment such as OFF due to cxx11_abi to each one.
  • Finally, in ubuntu.yml, the BUILD_TENSORFLOW_OPS and BUILD_PYTORCH_OPS settings are unused (leftovers from earlier changes). Go ahead and delete them.

We provide both cxx11_abi and pre cxx11_abi C++ binary builds for Open3D, so it is possible to support TF through C++. However, for simplicity, lets disable it completely in Linux.

For macOS, the ABI is not a problem. Lets upgrade Tensorflow to the latest there (2.12). This ensures that our TF code continues to be maintained. This will also need changes to util/ci_utils.sh (near where you update the PyTorch version).

@OlivierLDff
Copy link
Contributor Author

Would it be possible to approve workflow run so I can see what is working/failing?

@OlivierLDff
Copy link
Contributor Author

Forget what I said, it seems everything is working when I PR against my own repo

@OlivierLDff OlivierLDff force-pushed the support-python-311 branch 2 times, most recently from 02fd1bc to 36531b4 Compare August 17, 2023 07:15
@OlivierLDff OlivierLDff force-pushed the support-python-311 branch 8 times, most recently from e55988b to 41ab5a0 Compare August 17, 2023 12:07
@OlivierLDff
Copy link
Contributor Author

Ok so I updated a bunch of stuff, they are kept in different commit for easier review/understanding of what I did.
I'm running into multiple issues and I would gladly receive some insight before investing some times.

MacOS

tensorflow framework

So it seems tensorflow framework is no longer available, but I'm not familiar with that library. I just commented out things in the CMakeLists to ignore that for now. I guess this is the kind of issue that will blow up at linking time, that I haven't reached yet.

e55988b

Do you have any insight if the tensorflow_framework disappeared? Couldn't find anything, but didn't look for long.

Macos min version

It seems that min macos version is 10.15, but I'm running into issues with tensorflow

  • that don't include the right header (for example #include <optional> and stuff.
  • libusb was compiled with min version 12.x.

You can checks logs here: https://github.com/OlivierLDff/Open3D/actions/runs/5889809346/job/15973684524?pr=1

I guess by updating min macos version to 12.x, things will go away. Is it something you wish to do?

set (CMAKE_OSX_DEPLOYMENT_TARGET "10.15" CACHE STRING
"Minimum OS X deployment version" FORCE)
endif()

Ubuntu

Tensorflow

I have this really strange error:

Downloading tensorflow-gpu-2.12.0.tar.gz (2.6 kB)
#24 35.23     ERROR: Command errored out with exit status 1:
#24 35.23      command: /root/miniconda3/envs/open3d/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-install-dp_5t5al/tensorflow-gpu_22a7e4fc178b45c3a700cbb994608eb0/setup.py'"'"'; __file__='"'"'/tmp/pip-install-dp_5t5al/tensorflow-gpu_22a7e4fc178b45c3a700cbb994608eb0/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-pip-egg-info-rhpu13wg
#24 35.23          cwd: /tmp/pip-install-dp_5t5al/tensorflow-gpu_22a7e4fc178b45c3a700cbb994608eb0/
#24 35.23     Complete output (38 lines):
#24 35.23     Traceback (most recent call last):
#24 35.23       File "/root/miniconda3/envs/open3d/lib/python3.10/site-packages/setuptools/_vendor/packaging/requirements.py", line 35, in __init__
#24 35.23         parsed = parse_requirement(requirement_string)
#24 35.23       File "/root/miniconda3/envs/open3d/lib/python3.10/site-packages/setuptools/_vendor/packaging/_parser.py", line 64, in parse_requirement
#24 35.23         return _parse_requirement(Tokenizer(source, rules=DEFAULT_RULES))
#24 35.23       File "/root/miniconda3/envs/open3d/lib/python3.10/site-packages/setuptools/_vendor/packaging/_parser.py", line 82, in _parse_requirement
#24 35.23         url, specifier, marker = _parse_requirement_details(tokenizer)
#24 35.23       File "/root/miniconda3/envs/open3d/lib/python3.10/site-packages/setuptools/_vendor/packaging/_parser.py", line 126, in _parse_requirement_details
#24 35.23         marker = _parse_requirement_marker(
#24 35.23       File "/root/miniconda3/envs/open3d/lib/python3.10/site-packages/setuptools/_vendor/packaging/_parser.py", line 147, in _parse_requirement_marker
#24 35.23         tokenizer.raise_syntax_error(
#24 35.23       File "/root/miniconda3/envs/open3d/lib/python3.10/site-packages/setuptools/_vendor/packaging/_tokenizer.py", line 163, in raise_syntax_error
#24 35.23         raise ParserSyntaxError(
#24 35.23     setuptools.extern.packaging._tokenizer.ParserSyntaxError: Expected end or semicolon (after name and no valid version specifier)
#24 35.23         python_version>"3.7"

I'm not sure where is it coming from or how to track that. This seems to come from tensorflow which is anyway not in used. Have you ever heard of this problem?

Headless docs (ubuntu)

I'm running in a missing symbol, but this seems to be because I'm using the wrong repo for open3d-ML

Traceback (most recent call last):
  File "generate_tf_ops_wrapper.py", line 110, in <module>
    sys.exit(main())
  File "generate_tf_ops_wrapper.py", line 61, in main
    oplib = tf.load_op_library(args.lib)
  File "/opt/hostedtoolcache/Python/3.8.17/x64/lib/python3.8/site-packages/tensorflow/python/framework/load_library.py", line 54, in load_op_library
    lib_handle = py_tf.TF_LoadLibrary(library_filename)
tensorflow.python.framework.errors_impl.NotFoundError: /home/runner/work/Open3D/Open3D/build/lib/Release/cpu/open3d_tf_ops.so: undefined symbol: _ZTIN10tensorflow8OpKernelE

https://github.com/OlivierLDff/Open3D/actions/runs/5889809363/job/15973675060?pr=1

@ssheorey
Copy link
Member

Looks like build is failing due to this line? https://github.com/isl-org/Open3D/actions/runs/6399541003/job/17371755134?pr=6288#step:4:13726

#29 2234.8 make: *** No rule to make target 'open3d_tf_ops'.  Stop.

Probably need to guard these make targets behind the env var

make VERBOSE=1 -j"$NPROC" pybind open3d_tf_ops open3d_torch_ops

Thanks @igozali you have sharp eyes...

@johnthagen
Copy link
Contributor

Enable users to build wheel with tfops locally with docker container.
@ssheorey
Copy link
Member

@ssheorey In case it helps, I noticed that Pytorch recently uploaded cpu.cxx11.abi wheels to their custom wheel repo:

* https://download.pytorch.org/whl/torch/

For example:

* https://download.pytorch.org/whl/cpu-cxx11-abi/torch-2.1.0%2Bcpu.cxx11.abi-cp311-cp311-linux_x86_64.whl#sha256=0aafe9ff268e209831d38acd3e0b39a95ae76ff45a88ec0e48d7019cf476dc21

Thanks, that is useful. Hope they finish the migration to the new cxx11_abi quickly. To keep a handle on complexity, we'll go with the old cxx11_abi for now and without tf ops on Linux. We'll switch completely to the new cxx11_abi with both torch and tf in a future PR.

I'll add instructions for users to build an Open3D wheel with tf ops and the new cxx11_abi in the meantime.

@johnthagen johnthagen mentioned this pull request Oct 17, 2023
3 tasks
Copy link
Contributor

@benjaminum benjaminum left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 39 files reviewed, all discussions resolved

@ssheorey ssheorey merged commit a2401d0 into isl-org:master Oct 23, 2023
62 of 66 checks passed
@johnthagen
Copy link
Contributor

@ssheorey I was curious if the team was planning a 0.18 release to support Python 3.11? We are excited to try it out. ❤️

@ssheorey
Copy link
Member

Yes, we will have a 0.18 release soon. :-)

ssheorey added a commit to isl-org/Open3D-ML that referenced this pull request Oct 24, 2023
Companion Open3D PR: isl-org/Open3D#6288
- Add support for python3.11.
-️ Upgrade pytorch to 2.0.1 with cuda 11.7
- Upgrade tensorflow / tensorboard to 2.13. 
- Disable building tensorflow ops on Linux. Keep old cxx11_abi.
- Update CI to use requirements files instead of manually specified versions.
- Add instructions to build with new cxx11_abi with tensorflow ops support.
@thomasegriffith
Copy link

Yes, we will have a 0.18 release soon. :-)

Really looking forward to this! Do you have a rough idea on the timeline?

@thomasegriffith
Copy link

@ssheorey any idea when the 0.18 release will happen? TIA!

@sharon-br
Copy link
Contributor

@ssheorey Any updates on the 0.18 release?

@johnthagen
Copy link
Contributor

♫ All I want for Christmas is a new Open3D ♬

😄 ❤️

@ssheorey
Copy link
Member

♫ All I want for Christmas is a new Open3D ♬

😄 ❤️

Working on it... Hope to have a new year present for all...

@johnthagen
Copy link
Contributor

johnthagen commented Jan 5, 2024

It's out 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request to add Python 3.11 support?
7 participants