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

MostAwesomeDude · 2023-07-16T19:08:08Z

Following up from #61. This is more work towards #49.

Following up from #61.

rstarmer

Still some missing bits here, but a good direction. See comments on changes and additional elements of this build container.

platform/packaging/build/Dockerfile

rstarmer · 2023-07-17T14:56:43Z

platform/packaging/build/Dockerfile

-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


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.

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

Why should we do so?

rstarmer · 2023-07-17T14:57:36Z

platform/packaging/build/Dockerfile

+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
+# tarball. This is fine, but at odds with Docker's behavior.
+RUN tar zxf v1.7.4.tar.gz

 RUN virtualenv /build/local


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.

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.

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.

rstarmer · 2023-07-17T14:58:44Z

platform/packaging/build/run-cmake.sh

@@ -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


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.

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.

sdake

This is shaping up. What we need are apt, whl, and deb artifacts. I am not clear how this build container outputs these to the user (us).

Any thoughts?

platform/packaging/build/Dockerfile

sdake · 2023-07-17T21:40:43Z

platform/packaging/build/Dockerfile

-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


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

Why should we do so?

sdake · 2023-07-17T21:49:15Z

platform/packaging/build/Dockerfile

+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
+# tarball. This is fine, but at odds with Docker's behavior.
+RUN tar zxf v1.7.4.tar.gz

 RUN virtualenv /build/local


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.

sdake · 2023-07-17T21:56:11Z

platform/packaging/build/Dockerfile

+RUN apt-get update && \
+    apt-get install -y --no-install-recommends \
+        python3-dev python3-numpy-dev swig python3-virtualenv \
+        cmake git gpg default-jre


unclear why JRE is needed, although not sure it matters.

ca-certificates appears to fail to configure during post-install without a JRE present.

sdake · 2023-07-17T22:01:43Z

platform/packaging/build/Dockerfile

-  >> /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


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

rstarmer · 2023-07-18T06:58:03Z

@MostAwesomeDude podman build . -t faiss completes, and the image does have the libfaiss.so, libfaiss_python_callbacks.so, and _swigfaiss.so libraries, along with the faiss-1.7.4-py3-none-any.whl. I note that the libfaiss-avx2.so is missing. It might be good to just extract those files (the .so files and the whl) and store them as resultant artifacts in an OCI S3 bucket. Adding a github action to build this, and save the results becomes something that can be accomplished in another pull request.

sdake · 2023-07-20T12:54:33Z

Please note current repo does not build.

#75

sdake · 2023-07-20T19:25:14Z

@MostAwesomeDude you had mentioned in this review that you are working on Debian packaging. Debian packages this upstream, although it may lack CUDA support.

The Debian installation is here:
git clone https://salsa.debian.org/deeplearning-team/faiss/

(the debian dir) contains the control files.

rstarmer

LGTM

Tested on a debian with docker-engine environment
Was able to extract the required .whl and .so/.so.2 libraries (libfaiss, faiss_python, _swigfaiss, and I believe we could grab the MKL libraries as well)
Tested pytorch-retro on an Ubuntu 22.04 env.

Bump to Debian 12 and fix up various issues.

170a8bc

Following up from #61.

MostAwesomeDude requested review from rstarmer and sdake as code owners July 16, 2023 19:08

cla-bot bot added the CLA CLA signed? label Jul 16, 2023

rstarmer requested changes Jul 17, 2023

View reviewed changes

Address review feedback.

de73268

sdake reviewed Jul 17, 2023

View reviewed changes

rstarmer mentioned this pull request Jul 20, 2023

faiss fails to build. #75

Open

rstarmer approved these changes Jul 21, 2023

View reviewed changes

MostAwesomeDude merged commit 5a5939f into main Jul 22, 2023

MostAwesomeDude deleted the faiss-debian12-fixup branch July 22, 2023 17:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

MostAwesomeDude commented Jul 16, 2023

rstarmer left a comment

rstarmer Jul 17, 2023

sdake Jul 17, 2023

rstarmer Jul 17, 2023

MostAwesomeDude Jul 17, 2023

sdake Jul 17, 2023

rstarmer Jul 17, 2023

MostAwesomeDude Jul 17, 2023

sdake left a comment

sdake Jul 17, 2023

sdake Jul 17, 2023

sdake Jul 17, 2023

MostAwesomeDude Jul 17, 2023

sdake Jul 17, 2023

rstarmer commented Jul 18, 2023

sdake commented Jul 20, 2023

sdake commented Jul 20, 2023

rstarmer left a comment

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

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

Conversation

MostAwesomeDude commented Jul 16, 2023

rstarmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdake left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rstarmer commented Jul 18, 2023

sdake commented Jul 20, 2023

sdake commented Jul 20, 2023

rstarmer left a comment

Choose a reason for hiding this comment