Closed JesseFarebro closed 1 year ago
Hi @JesseFarebro, yeah, we should definitely also publish 3.10 wheels. I'm really busy this week, but it shouldn't be a major problem I think (at least I don't see any obvious technical issues blocking it). Cheers!
@kenjitoyama Hey is there any update on this?
Hi! Sorry for the massive delay, I forgot about this. I'll take a look.... the annoying part is ensuring that it plays nice with TFDS's glibc
version, protobuf's version, pybind11_protobuf
's version and libstdc++
's version. I'll take a look at this and update this bug.
@kenjitoyama Thanks, I actually did some digging. I think there are a few updates that need to be done for the third-party packages. This probably needs some coordination with riegeli's dependencies as well as I believe that the master branch currently breaks with TF 2.12.0 (the latest release version).
Not sure if it's at all helpful, here are some things which I found to be useful when attempting to create PR (which I didn't manage to finish because coordinating a few repos outside google3 is painful and bazel doesn't quite work with my pyenv setup.) Updating some dependencies get me to compile envlogger with py3.10. I did this with Bazel 6.1.0 instead of <4 as I thought maybe we can update the bazel version so that's it's not too out-of-sync.
# Probably a good idea to be in sync with the TF release?
http_archive(
name = "com_google_absl",
sha256 = "54707f411cb62a26a776dad5fd60829098c181700edcd022ea5c2ca49e9b7ef1",
strip_prefix = "abseil-cpp-20220623.1",
urls = ["https://github.com/abseil/abseil-cpp/archive/refs/tags/20220623.1.zip"], # 2022-06-23
)
# This seems to be the version of protobuf as of TF 2.12.0
http_archive(
name = "com_google_protobuf",
# patch_args = ["-p1"],
# patches = ["//third_party:protobuf.patch"],
# sha256 = "cfcba2df10feec52a84208693937c17a4b5df7775e1635c1e3baffc487b24c9b",
sha256 = "f66073dee0bc159157b0bd7f502d7d1ee0bc76b3c1eac9836927511bdc4b3fc1",
strip_prefix = "protobuf-3.21.9",
urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.21.9.zip"],
)
load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
protobuf_deps()
http_archive(
name = "com_google_googletest",
# sha256 = "ff7a82736e158c077e76188232eac77913a15dac0b22508c390ab3f88e6d6d86",
sha256 = "81964fe578e9bd7c94dfdb09c8e4d6e6759e19967e397dbea48d1c10e45d0df2",
# strip_prefix = "googletest-b6cd405286ed8635ece71c72f118e659f4ade3fb",
strip_prefix = "googletest-release-1.12.1",
urls = [
# "https://storage.googleapis.com/mirror.tensorflow.org/github.com/google/googletest/archive/b6cd405286ed8635ece71c72f118e659f4ade3fb.zip",
# "https://github.com/google/googletest/archive/b6cd405286ed8635ece71c72f118e659f4ade3fb.zip",
"https://github.com/google/googletest/archive/refs/tags/release-1.12.1.tar.gz"
],
)
http_archive(
name = "pybind11_bazel",
sha256 = "b72c5b44135b90d1ffaba51e08240be0b91707ac60bea08bb4d84b47316211bb",
strip_prefix = "pybind11_bazel-b162c7c88a253e3f6b673df0c621aca27596ce6b",
urls = ["https://github.com/pybind/pybind11_bazel/archive/b162c7c88a253e3f6b673df0c621aca27596ce6b.zip"],
)
# We still require the pybind library.
http_archive(
name = "pybind11",
build_file = "@pybind11_bazel//:pybind11.BUILD",
strip_prefix = "pybind11-2.10.4",
urls = ["https://github.com/pybind/pybind11/archive/v2.10.4.tar.gz"],
)
load("@pybind11_bazel//:python_configure.bzl", "python_configure")
Let me know if I could be of more help! I want to say thanks again for the effort of maintaining this as it is a really useful library to have working with the other DM RL libraries.
Hi @ethanluoyc! Yeah, it won't be so easy. Coordinating all of these deps is super annoying.
I think our best bet is to start from ubuntu:20.04
(which is the image used by tensorflow
), upgrade deps like the protobuf lib, pybind11 etc and then try to get it to build.
@kenjitoyama Yeah that was needed for py310. I thought maybe working with the TF build image https://hub.docker.com/r/tensorflow/build is another way given we want ABI compatibility with TF.
Totally agree. I think in general tooling around python packages built by bazel is a mess. Same goes for launchpad and reverb which all have separate ways for managing the complexity.
I'm already getting some errors trying to build a docker image with 3.10
:
Traceback (most recent call last):
File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
exec(code, run_globals)
File "/usr/lib/python3/dist-packages/pip/__main__.py", line 16, in <module>
from pip._internal.cli.main import main as _main # isort:skip # noqa
File "/usr/lib/python3/dist-packages/pip/_internal/cli/main.py", line 10, in <module>
from pip._internal.cli.autocompletion import autocomplete
File "/usr/lib/python3/dist-packages/pip/_internal/cli/autocompletion.py", line 9, in <module>
from pip._internal.cli.main_parser import create_main_parser
File "/usr/lib/python3/dist-packages/pip/_internal/cli/main_parser.py", line 7, in <module>
from pip._internal.cli import cmdoptions
File "/usr/lib/python3/dist-packages/pip/_internal/cli/cmdoptions.py", line 19, in <module>
from distutils.util import strtobool
ModuleNotFoundError: No module named 'distutils.util'
pip
is still using the deprecated distutils
module. :(
@ethanluoyc, how far did you get in the process?
pip
's error should be fixed with https://github.com/pypa/pip/commit/6739f56351a88add7a9e09e4eec25f691f79ec79, but the version in ubuntu:20.04
is older than that.
@kenjitoyama I created a PR which captures what I have right now. No guarantees that it would work and I also wasn't using the docker image :)
Roughly the updates I applied.
I remember getting this to work with most tests and the failing tests I got are mostly due to bazel not picking up the correct python.
Maybe once we figure this out we can think about setting up some GitHub CI? I can certainly help with that.
Hi @ethanluoyc ! Thanks for the PR, it's helpful to see your work. Are you able to build the wheels for Python 3.10? The command would be something like python3 envlogger/setup.py bdist_wheel --plat manylinux2014_x86_64
. After building the wheels you can try them yourself to see if everything works as expected.
WRT Github CI we already have an internal CI system that run tests on the docker image (using Dockerfile
as the "source"). I'm not opposed to having a GitHub Action that complements that system though, and in fact that's what we do with AndroidEnv and it works really well. Having the docker image nailed first will make any such CI attempts 10x easier.
Yeah I will give that a try. Would you want me to create an entire PR so you can review the changes and run the internal tests or you would want to update things on your end (I was hoping it was easier updating on your end but I could be wrong :D)
I can update the things on our end once we find a golden configuration. ;)
Cool. I will let you know of any update.
@kenjitoyama made some attempts
Hi @ethanluoyc , I gave up on trying to use pip_parse()
in Bazel and have instead did what you did by just installing the pip
dependencies directly into the system. I've got everything working, except the cross language test which I didn't look into (it's not a huge issue though since we do test that internally to make sure everything is working) and that we can take a look later since it's unrelated to bazel, pip and Python.
The new PyPi packages are available here: https://pypi.org/project/envlogger/1.1/
I'll prepare an internal change and hopefully it'll be submitted by the end of this week.
Thanks a lot for the investigation!
Daniel
@kenjitoyama Good to know!
I have encountered the issue with cross-language test and it's got to do with py_test not finding the correct python interpreter in bazel with a non-hermetic python toolchain configuration. (you need to pass the host PATH to bazel test in e.g. bazelrc for test to get the correct interpreter) I have some hacks which can fix that issue but I think it's too much hassle to test that OSS as long as you have it working internally.
Hi!
Would it be possible to publish Python 3.10 wheels? Other DeepMind projects are already publishing for 3.10 and it would be really nice to have envlogger.
Thanks!