Skip to content

Bump FAISS container base to Debian 12 and fix up various issues #73

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.

Sign up for GitHub

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
MostAwesomeDude merged 2 commits into from
Jul 22, 2023
Merged

Bump FAISS container base to Debian 12 and fix up various issues #73

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
170a8bc
Bump to Debian 12 and fix up various issues.
MostAwesomeDude Jul 16, 2023
de73268
Address review feedback.
MostAwesomeDude Jul 17, 2023
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
32 changes: 18 additions & 14 deletions platform/packaging/build/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,38 @@ ENV DEBIAN_FRONTEND=noninteractive
RUN apt-get update
RUN apt-get install -y --no-install-recommends \
python3-dev python3-numpy-dev swig python3-virtualenv \
cmake git gpg wget
cmake git gpg default-jre

# Phase 2: Install non-free components.
RUN sed -i -e 's/main/main contrib non-free/g' /etc/apt/sources.list.d/debian.sources

# Add nVidia CUDA repository.
RUN wget https://developer.download.nvidia.com/compute/cuda/repos/debian11/x86_64/cuda-keyring_1.1-1_all.deb
# Configure Intel repo and non-free/contrib Debian repos.
ADD debian.sources oneAPI.list /etc/apt/sources.list.d/

# Set up build directory.
RUN mkdir /build
WORKDIR /build

# Add nVidia CUDA repo and keyring.
ADD https://developer.download.nvidia.com/compute/cuda/repos/debian11/x86_64/cuda-keyring_1.1-1_all.deb /build/
RUN dpkg -i cuda-keyring_1.1-1_all.deb

# Add Intel repository.
RUN wget -O- https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB \
| gpg --dearmor | tee /usr/share/keyrings/oneapi-archive-keyring.gpg > /dev/null
RUN echo "deb [signed-by=/usr/share/keyrings/oneapi-archive-keyring.gpg] https://apt.repos.intel.com/oneapi all main" \
>> /etc/apt/sources.list.d/oneAPI.list
# Add Intel keyring.
ADD https://apt.repos.intel.com/intel-gpg-keys/GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB /build/
RUN < GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB gpg --dearmor > /usr/share/keyrings/oneapi-archive-keyring.gpg
Copy link
Member

@sdake sdake Jul 17, 2023

Choose a reason for hiding this comment

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

Does gpg --dearmor ./GPG-PUB-KEY-INTEL-SW-PRODUCTS.PUB > /usr/share/keyrings/oneapi-archive-keyring.gpg work here?


# Do a big install.
RUN apt-get update
RUN apt-get install -y --no-install-recommends \
cuda intel-basekit

# Phase 3: Prepare the build tree with CMake.
RUN mkdir /build
WORKDIR /build
ADD https://github.com/facebookresearch/faiss/archive/refs/tags/v1.7.4.tar.gz /build/
# XXX For whatever reason, Podman *doesn't* automatically unpack the source
Copy link
Member

@rstarmer rstarmer Jul 17, 2023

Choose a reason for hiding this comment

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

Unfortunate. You may want to wrap the tar in a test to ensure the file exists then, that way this should work with Docker (which I don't believe keeps the downloaded "archive" around) and in Podman, which apparently does.

Copy link
Member

@sdake sdake Jul 17, 2023

Choose a reason for hiding this comment

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

There is limited value in supporting multiple competing toolchains to produce our packaging.

Why should we do so?

# tarball. This is fine, but at odds with Docker's behavior.
RUN tar zxf v1.7.4.tar.gz

RUN virtualenv /build/local
Copy link
Member

@rstarmer rstarmer Jul 17, 2023

Choose a reason for hiding this comment

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

Do we need a virtualenv here? I know python complains about root owned files, but as this container is really a build container and not a runtime service, this may be overkill.

Copy link
Contributor Author

@MostAwesomeDude MostAwesomeDude Jul 17, 2023

Choose a reason for hiding this comment

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

Containers should be written to run with unprivileged users. I can't find it, but I could have sworn that it was official Debian policy to recommend virtualenv over system pip in all cases.

That said, I don't think that we need a virtualenv, and I primarily reached for it because Debian.

Copy link
Member

@sdake sdake Jul 17, 2023

Choose a reason for hiding this comment

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

I agree that containers with very limited capabilities are of strong value.

We are using the container to provide build-time isolation while generating faiss. We have no intent of running the container, so run-time isolation is of no value.

Maybe we should simplify and remove the container entirely from the equation.


RUN wget https://github.com/facebookresearch/faiss/archive/refs/tags/v1.7.4.tar.gz
RUN tar zxvf v1.7.4.tar.gz > /dev/null

ENV CUDACXX=/usr/local/cuda/bin/nvcc
COPY run-cmake.sh /build
RUN sh run-cmake.sh
Expand Down
2 changes: 1 addition & 1 deletion platform/packaging/build/run-cmake.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh

. /opt/intel/oneapi/setvars.sh
cmake -B build faiss-1.7.3 -DBUILD_SHARED_LIBS=ON -DFAISS_ENABLE_GPU=ON -DFAISS_ENABLE_PYTHON=ON -DFAISS_ENABLE_RAFT=OFF -DBUILD_TESTING=ON -DBUILD_SHARED_LIBS=ON -DFAISS_ENABLE_C_API=ON -DCMAKE_BUILD_TYPE=Release -DFAISS_OPT_LEVEL=avx2 -Wno-dev
cmake -B build faiss-1.7.4 -DBUILD_SHARED_LIBS=ON -DFAISS_ENABLE_GPU=ON -DFAISS_ENABLE_PYTHON=ON -DFAISS_ENABLE_RAFT=OFF -DBUILD_TESTING=ON -DBUILD_SHARED_LIBS=ON -DFAISS_ENABLE_C_API=ON -DCMAKE_BUILD_TYPE=Release -DFAISS_OPT_LEVEL=avx2 -Wno-dev
Copy link
Member

@rstarmer rstarmer Jul 17, 2023

Choose a reason for hiding this comment

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

Does this step work for you to build the avx2 .so file? It didn't in my previous Debian12 test.

To that end, can we at least add a validation in the build process to determine if the resultant libraries and python whl file exist on disk? That would be a necessary precursor to uploading them to an archive.

Copy link
Contributor Author

@MostAwesomeDude MostAwesomeDude Jul 17, 2023

Choose a reason for hiding this comment

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

Yeah, I'm still slowly working on building a Debian-style FAISS package. I am not good at Debian packaging, though, so it's taking me a while.