add package/faiss #25272

simshi · 2024-09-15T10:07:33Z

Summary

Add new recipe: faiss (https://github.com/facebookresearch/faiss)

Motivation

Faiss is a library for efficient similarity search and clustering of dense vectors.

Details

Limitations

Because there is no available CUDAToolkit/system and OpenMP/system, consumers must express dependencies on those dependencies by themself. Please refer to test_package/CMakeLists.txt as example.
- Solved cuda dependency, add a cmake build module file, by @valgur
Faiss prefers MKL, but no conan package, now, using OpenBLAS without an option to choose.
OpenBLAS recipe lacks of options like USE_OPENMP, NUM_THREADS, faiss openmp code calls into openblas without openmp supporting may cause issues.

Requires

openmp/system: new recipe #22360

Read the contributing guidelines
Checked that this PR is not a duplicate: list of PRs by recipe
Tested locally with at least one configuration using a recent version of Conan

CLAassistant · 2024-09-15T10:07:38Z

All committers have signed the CLA.

valgur · 2024-09-19T13:25:37Z

recipes/faiss/all/conanfile.py

+
+    def requirements(self):
+        self.requires("openblas/0.3.27")
+        self.test_requires("gtest/1.12.1")


self.test_requires() be placed under build_requirements() instead.

Besides that, following the value of the tools.build:skip_test config option on CCI is discouraged as it can lead to unexpected test failures for consumers and CCI maintainers try to avoid the additional potential burden from bug reports related to these.

valgur · 2024-09-19T13:27:04Z

recipes/faiss/all/conanfile.py

+        if not (
+            self.options.with_opt_generic or
+            self.options.get_safe("with_opt_avx2", False) or
+            self.options.get_safe("with_opt_avx512", False)
+        ):
+            raise ConanInvalidConfiguration("""Must configure at least one of following optimizations:
+    - generic: no optimization
+    - avx2: with x86 AVX2 instruction set
+    - avx512 (only availible >=1.8.0): with x86 AVX512 instruction set
+""")


I would suggest keeping the recipe slightly simpler and always enabling FAISS_WITH_OPT_GENERIC for non-debug builds instead. What do you think?

valgur · 2024-09-19T13:28:11Z

recipes/faiss/all/conanfile.py

+    - avx512 (only availible >=1.8.0): with x86 AVX512 instruction set
+""")
+        if not self.dependencies["openblas"].options.get_safe("use_openmp"):
+            self.output.warning("Prefers openbals with use_openmp")


Suggested change

self.output.warning("Prefers openbals with use_openmp")

self.output.warning("-o openblas/*:use_openmp is recommended but is not available.")

valgur · 2024-09-19T13:32:18Z

recipes/faiss/all/patches/fix-cmake-1.7.4.patch

+-  target_compile_options(faiss_avx2 PRIVATE $<$<COMPILE_LANGUAGE:CXX>:/arch:AVX2>)
+-  # we need bigobj for the swig wrapper
+-  add_compile_options(/bigobj)
+find_package(OpenMP REQUIRED)


This will fail on AppleClang and on Clang provided by most Linux package managers.

Related:

llvm-openmp: fix incompatibilities with official FindOpenMP, add v18.1.8, simplify patches, add MSVC support #24584

openmp/system: new recipe #22360

valgur · 2024-09-19T13:33:14Z

recipes/faiss/config.yml

+  "1.7.4":
+    folder: all


I would only keep the latest version unless you specifically need the older one for some reason.

valgur · 2024-09-19T13:45:34Z

Since v1.8.0, faiss has a separated gpu library, linked to cpu lib by whole-archive, but conan itself has no solution for it

The libtorch recipe had a similar issue and has a workaround for it: #24759. Probably useful here as well.

valgur

Needs some work, but looks good overall. Thank you!

valgur · 2024-09-19T19:14:18Z

I simplified the recipe a bit by avoiding most of the patching, improved OpenMP and CUDA support and filled in a few other gaps on my personal fork: https://github.com/valgur/conan-center-index/blob/dev/recipes/faiss/all/conanfile.py

simshi · 2024-09-20T15:57:58Z

Thank you, that's great!

conan-center-bot · 2024-09-20T16:00:24Z

Conan v1 pipeline ❌

Failure in build 3 (8a03b4b815f1a2542bd2a700f39c930878f9f669):

faiss/1.8.0:
Error running command conan info faiss/1.8.0@#7d1189f594ffd764b3a019ad57a090d1 --json {jsonName} --dry-build -pr {profileName}:

[settings]
arch=x86_64
build_type=Release
compiler=gcc
compiler.libcxx=libstdc++11
compiler.version=5
os=Linux
[options]
faiss:shared=False

...
ERROR: Failed requirement 'openmp/system' from 'faiss/1.8.0'
ERROR: Unable to find 'openmp/system' in remotes

^{Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.}

Conan v2 pipeline ❌

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

The v2 pipeline failed. Please, review the errors and note this is required for pull requests to be merged. In case this recipe is still not ported to Conan 2.x, please, ping @conan-io/barbarians on the PR and we will help you.

Failure in build 2 (8a03b4b815f1a2542bd2a700f39c930878f9f669):

faiss/1.8.0:
Error running command

conan graph info --requires faiss/1.8.0@#7d1189f594ffd764b3a019ad57a090d1 --build faiss/1.8.0@#7d1189f594ffd764b3a019ad57a090d1 -f json  -pr:h /home/conan/workspace/prod-v2/bsr/87548/cefde/profile_linux_11_libstdcpp11_17_gcc_release_64.-shared-False.txt -pr:b /home/conan/workspace/prod-v2/bsr/87548/cefde/profile_linux_11_libstdcpp11_17_gcc_release_64..txt

:

======== Computing dependency graph ========
openblas/0.3.27: Not found in local cache, looking in remotes...
openblas/0.3.27: Checking remote: conan-center
openblas/0.3.27: Downloaded recipe revision c16c3dbdf45e4a4ff56f8ada8631a2f3
openmp/system: Not found in local cache, looking in remotes...
openmp/system: Checking remote: conan-center
openmp/system: Checking remote: c3i_PR-v2-25272
Graph root
    cli
Requirements
    faiss/1.8.0#7d1189f594ffd764b3a019ad57a090d1 - Cache
    openblas/0.3.27#c16c3dbdf45e4a4ff56f8ada8631a2f3 - Downloaded (conan-center)
ERROR: Package 'openmp/system' not resolved: Unable to find 'openmp/system' in remotes.

^{Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.}

valgur · 2024-09-20T19:15:28Z

Opened a PR for OpenMP support in OpenBLAS: #25344. It's already included in my personal fork as well.

add package/faiss

c33c584

simshi mentioned this pull request Sep 15, 2024

[!!] Access request #4

Open

conan-center-bot added the User-approval pending label Sep 15, 2024

This comment has been minimized.

Sign in to view

uilianries mentioned this pull request Sep 16, 2024

[bot] Update authorized users list (2024-09-16) #25279

Merged

conan-center-bot added the Failed label Sep 19, 2024

This comment has been minimized.

Sign in to view

valgur reviewed Sep 19, 2024

View reviewed changes

valgur suggested changes Sep 19, 2024

View reviewed changes

simplify

8a03b4b

conan-center-bot removed the User-approval pending label Sep 20, 2024

valgur mentioned this pull request Sep 20, 2024

openblas: add v0.3.28, drop old versions and patches, add OpenMP and ILP64 support #25344

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add package/faiss #25272

add package/faiss #25272

simshi commented Sep 15, 2024 •

edited

Loading

CLAassistant commented Sep 15, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

valgur Sep 19, 2024

valgur Sep 19, 2024

valgur Sep 19, 2024

valgur Sep 19, 2024

valgur Sep 19, 2024

valgur commented Sep 19, 2024

valgur left a comment

valgur commented Sep 19, 2024

simshi commented Sep 20, 2024

conan-center-bot commented Sep 20, 2024

valgur commented Sep 20, 2024

	self.output.warning("Prefers openbals with use_openmp")
	self.output.warning("-o openblas/*:use_openmp is recommended but is not available.")

add package/faiss #25272

Are you sure you want to change the base?

add package/faiss #25272

Conversation

simshi commented Sep 15, 2024 • edited Loading

Summary

Motivation

Details

Limitations

Requires

CLAassistant commented Sep 15, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

valgur Sep 19, 2024

Choose a reason for hiding this comment

valgur Sep 19, 2024

Choose a reason for hiding this comment

valgur Sep 19, 2024

Choose a reason for hiding this comment

valgur Sep 19, 2024

Choose a reason for hiding this comment

valgur Sep 19, 2024

Choose a reason for hiding this comment

valgur commented Sep 19, 2024

valgur left a comment

Choose a reason for hiding this comment

valgur commented Sep 19, 2024

simshi commented Sep 20, 2024

conan-center-bot commented Sep 20, 2024

Conan v1 pipeline ❌

Conan v2 pipeline ❌

valgur commented Sep 20, 2024

simshi commented Sep 15, 2024 •

edited

Loading

CLAassistant commented Sep 15, 2024 •

edited

Loading