Skip to content
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

Added Faiss #8572

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Added Faiss #8572

wants to merge 4 commits into from

Conversation

stemann
Copy link
Contributor

@stemann stemann commented Apr 30, 2024

No description provided.

@stemann
Copy link
Contributor Author

stemann commented Apr 30, 2024

Replaces #8483

@stemann stemann force-pushed the feature/faiss branch 2 times, most recently from 5b63415 to e8ac9ed Compare December 4, 2024 09:19
F/Faiss/common.jl Outdated Show resolved Hide resolved
* Added mingw32 patch: To make posix_memalign etc. available on mingw32.
* Dropped CUDA archs 60, 61 for CUDA 11.8+.
* Update CMake wrt. CUDA arch detection: https://github.com/Kitware/CMake/blob/v3.23.3/Modules/CMakeDetermineCUDACompiler.cmake#L654
* Fixed CUDA cmake configure: Added symlink from $prefix/cuda/lib64 to $prefix/cuda/lib
* Updated to v1.9.0
* Added header file products
* Added additional patches for mingw32
include(joinpath(YGGDRASIL_DIR, "fancy_toys.jl"))
include(joinpath(YGGDRASIL_DIR, "platforms", "cuda.jl"))

name = "Faiss_GPU"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maleadt Does it make more sense to name packages _CUDA, rather than _GPU?

There is also some level of AMD rocm support in faiss.

Copy link
Member

Choose a reason for hiding this comment

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

How is multiple GPU backends implemented in this package? One advantage to splitting AMD and CUDA into separate packages would be to avoid downloading both sets of dependencies when only one would be needed.

Copy link
Contributor Author

@stemann stemann Jan 7, 2025

Choose a reason for hiding this comment

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

It's implemented via build flags (cf. below), so in addition to keeping dependencies separate, it makes a lot of sense with separate CUDA and AMD GPU/ROCM builds.

  • GPU-related options:
    • -DCUDAToolkit_ROOT=/path/to/cuda-10.1 in order to hint to the path of
      the CUDA toolkit (for more information, see
      CMake docs),
    • -DCMAKE_CUDA_ARCHITECTURES="75;72" for specifying which GPU architectures
      to build against (see CUDA docs to
      determine which architecture(s) you should pick),
    • -DFAISS_ENABLE_ROCM=ON in order to enable building GPU indices for AMD GPUs.
      -DFAISS_ENABLE_GPU must be ON when using this option. (possible values are ON and OFF),

Source: https://github.com/facebookresearch/faiss/blob/main/INSTALL.md#step-1-invoking-cmake

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have renamed Faiss_GPU to Faiss_CUDA

@stemann stemann marked this pull request as ready for review December 4, 2024 09:33
@stemann stemann marked this pull request as draft December 28, 2024 01:30
@stemann stemann marked this pull request as ready for review January 7, 2025 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants