Fix faiss swig build with version > 4.2.x #3315
Conversation
|
@junjieqi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@junjieqi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
I think the main uncertainty is on OSX and Windows since long is 32 bits on that platform. |
|
This test seems to fail https://github.com/facebookresearch/faiss/blob/main/tests/test_partitioning.cpp It would be useful to enable --output-on-failure for C++ tests. |
Summary: Currently, faiss can't build with swig version > 4.2.x. As the #3239 mentioned. Swig removed the support for 32bit swig/swig@9fb3a49. So SWIGTYPE_p_unsigned_long_long isn't supported any more. In this diff, we are going to remove the unsupported type from Faiss swig. Test Plan: STEP 1: create a new conda env ``` conda create --name faiss_swig conda activate faiss_swig ``` STEP 2: install dependecies from conda-forge ``` conda install -y -q python=3.11 cmake make swig mkl=2023 mkl-devel=2023 numpy scipy pytest gxx_linux-64 sysroot_linux-64=2.28 -c conda-forge ``` STEP 3: CMAKE ``` cmake -B build \ [system] -DBUILD_TESTING=ON \ -DBUILD_SHARED_LIBS=ON \ -DFAISS_ENABLE_GPU=OFF \ -DFAISS_ENABLE_RAFT=OFF \ -DFAISS_OPT_LEVEL=avx512 \ -DFAISS_ENABLE_C_API=ON \ -DPYTHON_EXECUTABLE=$(which python) \ -DCMAKE_BUILD_TYPE=Release \ -DBLA_VENDOR=Intel10_64_dyn \ -DCMAKE_CUDA_FLAGS="-gencode arch=compute_75,code=sm_75" \ . ``` STEP 4: build ``` make -C build -j faiss && make -C build -j swigfaiss ``` /var/folders/n5/8sm28y7j7hl8w4xdtl7r_33w0000gn/T/TemporaryItems/NSIRD_screencaptureui_AjUh4J/Screenshot 2024-03-25 at 12.21.25 AM.png
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
Tags:
|
@junjieqi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@junjieqi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@junjieqi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| @@ -1022,18 +1022,10 @@ PyObject *swig_ptr (PyObject *a) | |||
| return SWIG_NewPointerObj(data, SWIGTYPE_p_bool, 0); | |||
| } | |||
| if(PyArray_TYPE(ao) == NPY_UINT64) { | |||
| #ifdef SWIGWORDSIZE64 | |||
| return SWIG_NewPointerObj(data, SWIGTYPE_p_unsigned_long, 0); | |||
There was a problem hiding this comment.
This will probably fail on windows and / or mac because sizeof(long) = 4 on these platforms. So there should be a way to differentiate between them.
faiss/python/swigfaiss.swig
Outdated
| #ifdef SWIGWORDSIZE64 | ||
| return SWIG_NewPointerObj(data, SWIGTYPE_p_long, 0); | ||
| #if (__WORDSIZE == 32) | ||
| return SWIG_NewPointerObj(data, SWIGTYPE_p_unsigned_long_long, 0); |
There was a problem hiding this comment.
please check on __SIZEOF_LONG__ and generate SWIGTYPE_p_long_long or SWIGTYPE_p_long, not unsigned types
|
@junjieqi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
2 similar comments
|
@junjieqi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@junjieqi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: Currently, faiss can't build with swig version > 4.2.x. As the facebookresearch#3239 mentioned. Swig removed the support for 32bit swig/swig@9fb3a49. So SWIGTYPE_p_unsigned_long_long isn't supported any more. In this diff, we are going to remove the unsupported type from Faiss swig. Pull Request resolved: facebookresearch#3315 Test Plan: STEP 1: create a new conda env ``` conda create --name faiss_swig conda activate faiss_swig ``` STEP 2: install dependecies from conda-forge ``` conda install -y -q python=3.11 cmake make swig mkl=2023 mkl-devel=2023 numpy scipy pytest gxx_linux-64 sysroot_linux-64=2.28 -c conda-forge ``` STEP 3: CMAKE ``` cmake -B build \ -DBUILD_TESTING=ON \ -DBUILD_SHARED_LIBS=ON \ -DFAISS_ENABLE_GPU=OFF \ -DFAISS_ENABLE_RAFT=OFF \ -DFAISS_OPT_LEVEL=avx512 \ -DFAISS_ENABLE_C_API=ON \ -DPYTHON_EXECUTABLE=$(which python) \ -DCMAKE_BUILD_TYPE=Release \ -DBLA_VENDOR=Intel10_64_dyn \ -DCMAKE_CUDA_FLAGS="-gencode arch=compute_75,code=sm_75" \ . ``` STEP 4: build ``` make -C build -j faiss && make -C build -j swigfaiss ``` <img width="876" alt="Screenshot 2024-03-25 at 12 24 16 AM" src="https://github.com/facebookresearch/faiss/assets/8333898/918f0caf-398a-4361-989f-93ff547cf2b2"> Reviewed By: algoriddle Differential Revision: D55304004 Pulled By: junjieqi fbshipit-source-id: e958009dc637aa33b0e1a574a16a846a4abb1525
Summary:
Currently, faiss can't build with swig version > 4.2.x. As the #3239 mentioned. Swig removed the support for 32bit swig/swig@9fb3a49. So SWIGTYPE_p_unsigned_long_long isn't supported any more. In this diff, we are going to remove the unsupported type from Faiss swig.
Test Plan:
STEP 1: create a new conda env
STEP 2: install dependecies from conda-forge
STEP 3: CMAKE
STEP 4: build