Skip to content

Add manywheel2014-builder images #785

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 4 commits into from
Sep 24, 2021
Merged

Add manywheel2014-builder images #785

merged 4 commits into from
Sep 24, 2021

Conversation

mattip
Copy link
Contributor

@mattip mattip commented Jun 14, 2021

This PR

  • adds a Dockerfile_2014 that uses the pre-build manylinux2014 image as a base, meaning no need to build python, auditwheel, and patchelf
  • tries to remain true to the previous docker image so that building pytorch should JustWork
  • convert the build script into a loop for both docker image bases, which should create two sets of docker images

If this looks OK and actually uploads some images, I think the next iterative steps would be a PR to use the images (tagged with manylinux2014-builder:XXX) in pytorch/pytorch, and then to come back and fix the image failures.

@mattip
Copy link
Contributor Author

mattip commented Jun 14, 2021

A first step toward #670 which goes further toward enabling #520

"${TOPDIR}"
)
IMAGES=''
for DOCKER_NAME in manylinux manylinux2014; do
Copy link
Member

Choose a reason for hiding this comment

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

I think if we're going to do this it should just be an env variable that you pass through instead of iterating a for loop here, something like:

MANYLINUX_VERSION=2014 manywheel/build_docker.sh

Just so that we don't have to build all of the variants every single time.

This logic to build all of the variants can be added to manywheel/build_all_docker.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

adopted

@mattip
Copy link
Contributor Author

mattip commented Jun 20, 2021

A maintainer needs to approve the CI run

@mattip
Copy link
Contributor Author

mattip commented Jun 27, 2021

ping @seemethere

@mattip
Copy link
Contributor Author

mattip commented Jun 28, 2021

I updated the BASE_CUDA_VERSION to 10.2.

Should the update of cuda 10.1 to 10.2 also be done in the other places it appears:

libtorch/ubuntu16.04/Dockerfile
manywheel/Dockerfile
manywheel/build.sh

@mattip
Copy link
Contributor Author

mattip commented Jul 1, 2021

@malfet @seemethere any thing else needed?

@snnn
Copy link

snnn commented Jul 7, 2021

Not after long you will need to move to manylinux2 because the latest CUDA release requires libstdc++ from GCC 5.x but the one you got in manylinux2014 is based on GCC 4.x.

"CUDA Math Libraries toolchain uses C++11 features, and a C++11-compatible standard library (libstdc++ >= 20150422) is required on the host." from: https://docs.nvidia.com/cuda/cuda-toolkit-release-notes/index.html

ENV LD_LIBRARY_PATH=/opt/rh/devtoolset-7/root/usr/lib64:/opt/rh/devtoolset-7/root/usr/lib:$LD_LIBRARY_PATH

# cmake
RUN yum install -y cmake3 && \
Copy link

Choose a reason for hiding this comment

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

As you are using manylinux2014_x86_64 as the base, it should already have cmake and gcc. So do you need to install it again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it is already installed, yum will not install it again.

Copy link

Choose a reason for hiding this comment

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

manylinux installs cmake from pypi. yum doesn't know that.
And, they don't have gcc7, they installed gcc 9. So when you install GCC7, you will get two GCCs.

@mattip
Copy link
Contributor Author

mattip commented Jul 7, 2021

Not after long you will need to move to manylinux2_24 ...

... which apparently also will not be enough, see the discussion in issue #520. But this will be a first step in that direction.

@mattip
Copy link
Contributor Author

mattip commented Jul 7, 2021

Any ideas what happened to github actions?

@rgommers
Copy link
Contributor

rgommers commented Jul 8, 2021

Any ideas what happened to github actions?

That's not very useful CI output:

image

Never seen that before. Just restart it by close/open or pushing a new commit?

@mattip
Copy link
Contributor Author

mattip commented Jul 8, 2021

@seemethere @malfet could you start CI?

@mattip
Copy link
Contributor Author

mattip commented Jul 8, 2021

It crashed again. I rebased off master maybe that will solve it.
@seemethere @malfet please restart when you get a chance

@rgommers rgommers added the pypi PyPI, pip, wheel related issues label Jul 29, 2021
@malfet malfet merged commit 665e0d9 into pytorch:main Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla signed pypi PyPI, pip, wheel related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants