Add a new architecture mode: 'avx512-sr'. #4025
Conversation
… to AVX512. Signed-off-by: Mulugeta Mammo <[email protected]>
|
Hmm weird the CIs are not running on the PR. Do you mind pushing a new commit and see if the CIs start? |
Signed-off-by: Mulugeta Mammo <[email protected]>
|
@mengdilin Pushed a new commit but CI not starting. Is this possibly because I updated .github/workflows/build.yml? |
|
Yea there is a syntax error in the build file: see https://github.com/facebookresearch/faiss/actions/runs/11804384709 |
.github/workflows/build.yml
Outdated
| @@ -67,6 +67,17 @@ jobs: | |||
| uses: ./.github/actions/build_cmake | |||
| with: | |||
| opt_level: avx512 | |||
| linux-x86_64-AVX512-cmake: | |||
There was a problem hiding this comment.
linux-x86_64-AVX512-advanced-cmake
Signed-off-by: Mulugeta Mammo <[email protected]>
|
@mengdilin CI is picking g++ version 11. But AVX512-FP16 (-mavx512fp16) requires version 12+. |
|
Ah yea the conda publication CI is using the older compiler version. We should investigate on our side; however, I don't think we want to publish this architecture mode to conda right now, can you omit it? Looks like CI failure is coming from a unit test failure from Line 552 in 0fb56d9 Try commenting out that test and see if anything else fails? |
conda/faiss/build-lib.sh
Outdated
| -DFAISS_ENABLE_GPU=OFF \ | ||
| -DFAISS_ENABLE_PYTHON=OFF \ | ||
| -DBLA_VENDOR=Intel10_64lp \ | ||
| -DCMAKE_INSTALL_LIBDIR=lib \ | ||
| -DCMAKE_BUILD_TYPE=Release . | ||
|
|
||
| make -C _build -j$(nproc) faiss faiss_avx2 faiss_avx512 | ||
| make -C _build -j$(nproc) faiss faiss_avx2 faiss_avx512 faiss_avx512_sr |
There was a problem hiding this comment.
This is used for faiss's conda packaging upload. I don't think we want to expose this build mode yet in conda officially. Can you omit this for now?
Signed-off-by: Mulugeta Mammo <[email protected]>
Signed-off-by: Mulugeta Mammo <[email protected]>
|
Thanks @mengdilin. @kuarora Could you please review? |
|
@mulugetam I would use |
Signed-off-by: Mulugeta Mammo <[email protected]>
Signed-off-by: Mulugeta Mammo <[email protected]>
There was a problem hiding this comment.
Back from PTO. LGTM, can you resolve the conflict so I can import the PR. For the unit test, can you relax the threshold instead of commenting it out? If somehow you cannot relax this threshold, you can skip this unittest when in SR mode similar to
faiss/faiss/gpu/test/test_cagra.py
Line 13 in 697b6dd
| endif() | ||
| if(NOT WIN32) | ||
| # Architecture mode to support AVX512 extensions available since Intel(R) Sapphire Rapids. | ||
| # Ref: https://networkbuilders.intel.com/solutionslibrary/intel-avx-512-fp16-instruction-set-for-intel-xeon-processor-based-products-technology-guide |
| @@ -568,7 +569,7 @@ def test_ivf_train_2level(self): | |||
| # normally 47 / 200 differences | |||
| ndiff = (Iref != Inew).sum() | |||
| self.assertLess(ndiff, 51) | |||
There was a problem hiding this comment.
is there away to relax the threshold such that this test passes in SR mode as well?
This PR adds a new architecture mode to support the new extensions to AVX512, namely AVX512-FP16, which have been available since Intel® Sapphire Rapids.
This PR is a prerequisite for PR#4020 that speeds up hamming distance evaluations.