forked from Enet4/faiss
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Sync #1
Merged
Merged
Sync #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: **Bugs:** When following rocksdb_ivf demo to build executable file, its output as: ```bash faiss/demos/rocksdb_ivf/RocksDBInvertedLists.h:52:35: error: 'faiss::InvertedListsIterator* faiss_rocksdb::RocksDBInvertedLists::get_iterator(size_t) const' marked 'override', but does not override 52 | faiss::InvertedListsIterator* get_iterator(size_t list_no) const override; | ^~~~~~~~~~~~ make[2]: *** [CMakeFiles/demo_rocksdb_ivf.dir/build.make:90: CMakeFiles/demo_rocksdb_ivf.dir/RocksDBInvertedLists.cpp.o] Error 1 ``` **Solution:** Add relevant variable `void* inverted_list_contex` corresponding `get_iterator`'s base virtual function. Pull Request resolved: #3326 Reviewed By: mlomeli1, mdouze Differential Revision: D55629580 Pulled By: algoriddle fbshipit-source-id: a12fcacb483e0dd576411ad91a3dd1e0de94abec
…Lists (#3327) Summary: Pull Request resolved: #3327 **Context** 1. [Issue 2621](#2621) discuss inconsistency between OnDiskInvertedList and InvertedList. OnDiskInvertedList is supposed to handle disk based multiple Index Shards. Thus, we should name it differently when merging invls from index shard. 2. [Issue 2876](#2876) provides usecase of shifting ids when merging invls from different shards. **In this diff**, 1. To address #1 above, I renamed the merge_from function to merge_from_multiple without touching merge_from base class. why so? To continue to allow merge invl from one index to ondiskinvl from other index. 2. To address #2 above, I have added support of shift_ids in merge_from_multiple to shift ids from different shards. This can be used when each shard has same set of ids but different data. This is not recommended if id is already unique across shards. Reviewed By: mdouze Differential Revision: D55482518 fbshipit-source-id: 95470c7449160488d2b45b024d134cbc037a2083
Summary: Pull Request resolved: #3338 add reconstruct_n for GPU IVFFlat Reviewed By: mdouze Differential Revision: D55577561 fbshipit-source-id: 47f8b939611e2df7dbcd087129538145f627293c
Summary: Pull Request resolved: #3349 **Context** [Issue 3128](#3128) is an enhancement request to support remove_ids for IVFPQFastScan. Existing mechanism use direct map and iterate over items in selector and use scopecodes and scopeIds to replace item to be removed. Given that codes are packed, it is hard to return single code how it is packed in CodePackerPQ4. Thus, we need a custom implementation to removed_ids. **In this diff**, 1. We have added custom implementation of remove_ids from BlockInvertedLists which unpack code as it iterate and repack in new position. DirectMap use this remove_id function in BlockInvertedLists for type NoMap in DirectMap. 2. Also, we are throwing exception for other map type in DirectMap i.e. HashTable Reviewed By: mdouze Differential Revision: D55723390 fbshipit-source-id: 0017b556bd790765251e778ac48ed37ff3a29a45
…3336) Summary: Pull Request resolved: #3336 Issues: #3269 #3024 List of implemented GPU indices: https://github.com/facebookresearch/faiss/wiki/Faiss-on-the-GPU#implemented-indexes Reviewed By: mdouze Differential Revision: D55577576 fbshipit-source-id: 49f490cfba6784661e378acf4de3cce4195bb43b
Differential Revision: D55723390 Original commit changeset: 0017b556bd79 Original Phabricator Diff: D55723390 fbshipit-source-id: 58d61467b30dd11d27398f9f825162f598896845
Summary: Pull Request resolved: #3354 **Change was previously reverted because of build failure as change D55577576 removed the definition of FAISS_THROW_IF_MSG** **Context** [Issue 3128](#3128) is an enhancement request to support remove_ids for IVFPQFastScan. Existing mechanism use direct map and iterate over items in selector and use scopecodes and scopeIds to replace item to be removed. Given that codes are packed, it is hard to return single code how it is packed in CodePackerPQ4. Thus, we need a custom implementation to removed_ids. **In this diff**, 1. We have added custom implementation of remove_ids from BlockInvertedLists which unpack code as it iterate and repack in new position. DirectMap use this remove_id function in BlockInvertedLists for type NoMap in DirectMap. 2. Also, we are throwing exception for other map type in DirectMap i.e. HashTable Reviewed By: ramilbakhshyiev Differential Revision: D55858959 fbshipit-source-id: c8a0631495380b7dead36720e4507f4d1900d39f
Summary: Pull Request resolved: #3304 Reviewed By: junjieqi Differential Revision: D55823369 Pulled By: mdouze fbshipit-source-id: c0e9f4b85d979758f02e9953f3706b63a846bf22
Summary: Pull Request resolved: #3362 Add test to Alex' PR Reviewed By: junjieqi Differential Revision: D56003946 fbshipit-source-id: 5a8a881d450bc97ae0777d73ce0ce8607ec6b686
Summary: Pull Request resolved: #3363 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead {F1484071654} Reviewed By: kuarora Differential Revision: D56009251 fbshipit-source-id: ec222cf589ff98b016979058d59fc20cccec8f43
Summary: LLVM-15 has a warning `-Wunused-but-set-variable` which we treat as an error because it's so often diagnostic of a code issue. Unused variables can compromise readability or, worse, performance. This diff either (a) removes an unused variable and, possibly, it's associated code, or (b) qualifies the variable with `[[maybe_unused]]`, mostly in cases where the variable _is_ used, but, eg, in an `assert` statement that isn't present in production code. - If you approve of this diff, please use the "Accept & Ship" button :-) Reviewed By: dmm-fb Differential Revision: D56065763 fbshipit-source-id: b0541b8a759c4b6ca0e8753fc24b8c227047bd3d
Summary: Previously this code conformed from clang-format 12. Reviewed By: igorsugak Differential Revision: D56065247 fbshipit-source-id: f5a985dd8f8b84f2f9e1818b3719b43c5a1b05b3
Summary: The CMakeLists.txt in faiss/gpu uses the $<LINK_LIBRARY:WHOLE_ARCHIVE expression which requires at least cmake 3.24. Pull Request resolved: #3305 Reviewed By: mlomeli1 Differential Revision: D56234500 Pulled By: algoriddle fbshipit-source-id: dfe7df3379c5250dedec7d1988cffa889fc1c393
Summary: In this commit ab2b7f5, they changed format based on clang-format-18. However, we still use clang-format-11 in our circle ci job which caused the failure. In this PR, we are going to switch to clang-format-18 Pull Request resolved: #3372 Reviewed By: kuarora Differential Revision: D56280363 Pulled By: junjieqi fbshipit-source-id: f832ab2112f762e6000b55a155e3e43fe99071d7
Summary: Pull Request resolved: #3371 This will never happen because N is fixed at compile time and the buffer is large enough. It is misleading to add error handling code for a case that will never happen. Reviewed By: kuarora Differential Revision: D56274458 fbshipit-source-id: ca706f1223dbc97e69d5ac9750b277afa4df80a7
Summary: The current loop goes from 0 to 31. It has an if statement to do an assignment for j < 16 and a different assignment for j >= 16. By unrolling the loop to do the j < 16 and the j >= 16 iterations in parallel the if j < 16 is eliminated and the number of loop iterations is reduced in half. Then unroll the loop for the j < 16 and the j >=16 to a depth of 2. This change results in approximately a 55% reduction in the execution time for the bench_ivf_fastscan.py workload on Power 10 when compiled with CMAKE_INSTALL_CONFIG_NAME=Release. The removal of the if (j < 16) statement and the unrolling of the loop removes branch cycle stall and register dependencies on instruction issue. The result is the unrolled code is able issue instructions earlier thus reducing the total number of cycles required to execute the function. Pull Request resolved: #3364 Reviewed By: kuarora Differential Revision: D56455690 Pulled By: mdouze fbshipit-source-id: 490a17a40d9d4439b1a8ea22e991e706d68fb2fa
Summary: This pull request is for issue #3330. This patch makes sure that packed code arrays are in big endian format. Kindly let us know if we need any changes or if we can have a better approach. Pull Request resolved: #3345 Reviewed By: junjieqi Differential Revision: D55957630 Pulled By: mdouze fbshipit-source-id: f728f9563f6b942af9d8899b54662d7ceb811206
Summary: Pull Request resolved: #3361 Fix a few issues in the PR. Normally all tests should pass on a litlle-endian machine Reviewed By: junjieqi Differential Revision: D56003181 fbshipit-source-id: 405dec8c71898494f5ddcd2718c35708a1abf9cb
Summary: Pull Request resolved: #3383 In this diff, I am fixing minor issues in bench_fw where either certain fields are not accessible when index is build from codec. It also requires index to be discovered using codec alias as index factory is not always available. In subsequent diff internal to meta will have testcase that execute this path. Reviewed By: algoriddle Differential Revision: D56444641 fbshipit-source-id: b7af7e7bb47b20bbb5515a66f41dd24f42459d52
Summary: Fixes #3343 Reviewed By: kuarora, junjieqi Differential Revision: D56526842 fbshipit-source-id: b7c4377495db4e68283cf4ce2b7c8fae008cd404
Summary: The osx failed https://app.circleci.com/pipelines/github/facebookresearch/faiss/5698/workflows/4e029c32-8d8b-4db7-99e2-8e802aad6653/jobs/32701 Pull Request resolved: #3357 Reviewed By: kuarora Differential Revision: D56039739 Pulled By: junjieqi fbshipit-source-id: dd434a8817148364797eae39c09e0e1e9edbe858
Summary: Remove debugging log lines Reviewed By: mlomeli1 Differential Revision: D56626636 fbshipit-source-id: 2721b84e4e1359d1372df2b2c95cc668c6a75c3f
Summary: This demonstrates how to query several independent IVF indexes with a trained index in common. This avoids to duplicate the coarse quantizer and metadata in memory. On the Faiss side, it also implements a InvertedListIterator on top of the flat inverted lists, which can prove useful. Reviewed By: junjieqi Differential Revision: D56575887 fbshipit-source-id: cc3b26e952ee21f24b10169b5b614066600cf4b8
Summary: `nullptr` is typesafe. `0` and `NULL` are not. In the future, only `nullptr` will be allowed. This diff helps us embrace the future _now_ in service of enabling `-Wzero-as-null-pointer-constant`. Reviewed By: palmje Differential Revision: D56650318 fbshipit-source-id: 803ae62114c39143b65946f6f0387715eaf7f534
Summary: This commit is the first in a series in an attempt to incrementally enable all jobs currenlty performed by CircleCI. It includes the main configuration files provided by GitHub team + 1 build. Original PR: #3325 Reviewed By: junjieqi Differential Revision: D56671582 fbshipit-source-id: c8a21cd69aabaf86134eb86753e90b1facf51bc3
Summary: GitHub checks Reviewed By: junjieqi Differential Revision: D56733297 fbshipit-source-id: fe5a2ca7c67f36a4fe986af78fb6dc8f4f843150
Summary: Migration to GitHub actions Reviewed By: junjieqi Differential Revision: D56745520 fbshipit-source-id: 5311a549842f19672ae574edaa8be3ea5a580dbc
Summary: Pull Request resolved: #3405 Migration to GitHub Actions Reviewed By: junjieqi Differential Revision: D56843276 fbshipit-source-id: 3d5c7ee9a36a783407dfdcc3574c377da5f9db78
Summary: Pull Request resolved: #3406 Migration to GitHub Actions Reviewed By: junjieqi Differential Revision: D56848895 fbshipit-source-id: 5a351534d9151369a9104314fee203657ac40043
Summary: Pull Request resolved: #3901 1) remove system time from benchmark as this metric has extremely high jitter (50-100%) and is not useful for us 2) clean up command-line arguments and define a main function the external world can call 3) tweak default so microbenchmark runs fast by default (this does not the parameters we pass to microbenchmarks for servicelab) Reviewed By: mnorris11 Differential Revision: D63650110 fbshipit-source-id: efc81563291f00701a0d1df1d27172adeb3ef231
Summary: Pull Request resolved: #3887 Reviewed By: kuarora Differential Revision: D63355030 Pulled By: asadoughi fbshipit-source-id: 38792e49fe678c2811896faca7a3ddcab19f8bd0
Summary: Pull Request resolved: #3907 same as title. Fix checking right desc Reviewed By: satymish Differential Revision: D63854967 fbshipit-source-id: b8bc48662bc38ac96cf9241bdbe2be2b23f1a37e
Summary: Pull Request resolved: #3921 Reviewed By: pankajsingh88 Differential Revision: D64005877 Pulled By: ramilbakhshyiev fbshipit-source-id: 663c7ab752db04751c7675095d2545adec4be173
Summary: Similar to .github/workflows/nightly.yml Pull Request resolved: #3910 Reviewed By: kuarora, pankajsingh88 Differential Revision: D63923478 Pulled By: asadoughi fbshipit-source-id: df92a86ba48aa0d19aae40d7ca11aeedf4dfac51
Summary: Pull Request resolved: #3919 These tests are passing successfully in `dev` mode during my local development when I added them but I recently noticed they are failing on contbuild which is running them in opt/mode: https://www.internalfb.com/intern/test/281475152762853/ Upon further inspection, 2 of these were from floating point comparisons which we can fix with `EXPECT_NEAR`. The another one stems from indeterminism of the results in opt mode, so we will relax the test until we figure out a way to deal with the indeterminism Reviewed By: junjieqi Differential Revision: D63942329 fbshipit-source-id: 60f1c0b8a0db93015cd32bf991ab983ff2d1af13
Summary: Pull Request resolved: #3916 Adding missing wrapper to the torch wrappers in Faiss + test it. Also factorized a bit of code between search functions. Reviewed By: algoriddle Differential Revision: D63974821 fbshipit-source-id: a0415a57a763e2d1896956c503e503615c167860
Summary: Sometimes between Sept 25 to Oct 2, downloading and linking against `openblas=*=*openmp*` package to run tests have caused a 4-7x slow down. Link it with the regular openblas package which is not compiled with `USE_OPENMP=1`. We will set the openblas omp threads via the environment variable `OPENBLAS_NUM_THREADS` according to https://github.com/OpenMathLib/OpenBLAS/wiki/Faq#multi-threaded Pull Request resolved: #3918 Test Plan: SVE CI should finish within 40 minutes Reviewed By: ramilbakhshyiev Differential Revision: D64059860 Pulled By: mengdilin fbshipit-source-id: 3ba2bda5fce5122f051421f459692f15ad5360a4
Summary: Pull Request resolved: #3928 Fix issue in T203425107 Reviewed By: asadoughi Differential Revision: D64068971 fbshipit-source-id: 56db439793539570a102773ff2c7158d48feb7a9
Summary: * Replaced 1.8.0 to 1.9.0. * Fixed x86-64 architecture reference: https://en.wikipedia.org/wiki/X86-64 Tested with: `conda install -c pytorch/label/staging faiss-cpu` Pull Request resolved: #3929 Reviewed By: ramilbakhshyiev Differential Revision: D64082430 Pulled By: asadoughi fbshipit-source-id: 8a1427a7c14b8c3de4a341533b138d9d8f8490f2
Summary: Pull Request resolved: #3934 Initial thought was to be able to call individual operations on execution operator but it make sense to keep single interface 'execute' and move all these implementations to respective operators. Reviewed By: satymish Differential Revision: D63290104 fbshipit-source-id: d1f0b1391c38552c5cdb0a8ea935e23d0d0cb75b
Summary: Pull Request resolved: #3935 same as title Reviewed By: satymish Differential Revision: D64144800 fbshipit-source-id: 4a298aa83315d82f44ee424bf0a30737d5bf48a4
Summary: Pull Request resolved: #3917 The norm computation ST_norm_from_LUT was not implemented in Faiss. See issue #3882 This diff adds an implementation for it. It is probably not very quick. A few precomputed tables for AdditiveQuantizer were moved form ResidualQuantizer. Reviewed By: asadoughi Differential Revision: D63975689 fbshipit-source-id: 1bbe497a66bb3891ae727a1cd2b719479f80a836
…3922) Summary: Pull Request resolved: #3922 We need to be able to build external modules into FAISS, but don't have an example yet. This diff shows what CMakeLists.txt changes need to happen to incorporate an external module. Reference: #3699 Reviewed By: mdouze Differential Revision: D63991471 fbshipit-source-id: 0c1cd25eabbffb01d2a7170d6725a0c4a13c5bf0
Summary: Pull Request resolved: #3941 Support PQFS with index trainer Reviewed By: kuarora Differential Revision: D64259953 fbshipit-source-id: fd7ed90aed2ff6b6351460dcf7b61058c59cd25b
Summary: Pull Request resolved: #3942 Reverted splitting of partition string to align with previous behavior of finding partition intersection rather than the union. Also changed util dependency from laser to vector_search, which makes more sense. Reviewed By: kuarora Differential Revision: D64274531 fbshipit-source-id: 4614ae42d0fb534c9c9ce3314fd3c26a0c74d049
Summary: #2943 had removed about SVE information (added on #2886 ) on the installation document. This PR fixes it. This PR changes only the document, so it doesn't affect software behavior. Pull Request resolved: #3915 Reviewed By: asadoughi Differential Revision: D63967842 Pulled By: ramilbakhshyiev fbshipit-source-id: ce0a0bfe591cb75b504cdf6362b5e8ed156928d5
Summary: Pull Request resolved: #3948 I noticed this file I added was violating the header check. https://www.internalfb.com/intern/opensource/github/repo/1812028399049977/checkup/ Reviewed By: asadoughi Differential Revision: D64341054 fbshipit-source-id: 4564191661acbff193c8ffe970582cef8fb3a490
Summary: related: #2884 I added some SVE implementations of: - `code_distance` - `distance_single_code` - `distance_four_codes` - `exhaustive_L2sqr_blas_cmax_sve` - `fvec_inner_products_ny` - `fvec_madd` ## Evaluation result I evaluated the search for SIFT1M dataset on AWS EC2 c7g.large and r8g.large instances. `main` is the current (2e6551f) implementation. ### c7g.large (Graviton 3) ![g3_sift1m](https://github.com/user-attachments/assets/9c03cffa-72d1-4c77-9ae8-0ec0a5f5a6a5) ![g3_ivfpq](https://github.com/user-attachments/assets/4a8dfcc8-823c-4c31-ae79-3f4af9be28c8) On Graviton 3, `IndexIVFPQ` has been improved particularly. In the best case (IndexIVFPQ + IndexFlatL2, M: 32), this PR is approx. 2.38-~~2.50~~**2.44**x faster than `main` . - nprobe: 1, 0.069ms/query → 0.029ms/query - nprobe: 4, 0.181ms/query → ~~0.074~~**0.075**ms/query - nprobe: 16, 0.613ms/query → ~~0.245~~**0.251**ms/query ### r8g.large (Graviton 4) ![g4_sift1m](https://github.com/user-attachments/assets/e8510163-49d2-4143-babe-d406e2e40398) ![g4_ivfpq](https://github.com/user-attachments/assets/dc9a3ae0-a6b5-4a07-9898-c6aff372025c) On Graviton 4, especially `IndexIVFPQ` for tiny `nprobe` has been improved. In the best case (IndexIVFPQ + IndexFlatL2, M: 8, nprobe: 1), this PR is approx. 1.33x faster than `main` (0.016ms/query → 0.012ms/query). Pull Request resolved: #3933 Reviewed By: mengdilin Differential Revision: D64249808 Pulled By: asadoughi fbshipit-source-id: 8a625f0ab37732d330192599c851f864350885c4
Summary: Pull Request resolved: #3959 Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so. This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug. **What's a shadowed variable?** Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs. This diff fixes such an issue by renaming the variable. - If you approve of this diff, please use the "Accept & Ship" button :-) Reviewed By: meyering Differential Revision: D64398686 fbshipit-source-id: 44c60ea6e99d9542acf5af15adba6cdccda95577
…viceProperties (#3950) Summary: This diff enables to cache the device major version value so getCudaDeviceProperties() doesn't need to be called multiple times. Currently, the profiler of the code looks as so: {F1933796291} Pull Request resolved: #3950 Test Plan: N5114369 -- provides a toy example (2) which exhibits the following timings: Average timings before change: 3.35s Average tmings after change: 1.99s Reviewed By: algoriddle Differential Revision: D64047778 Pulled By: mlomeli1 fbshipit-source-id: 2f09373944237e80b96d40f35c6714c06f5741a9
Summary: Pull Request resolved: #3958 Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so. This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug. **What's a shadowed variable?** Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs. This diff fixes such an issue by renaming the variable. - If you approve of this diff, please use the "Accept & Ship" button :-) Reviewed By: meyering Differential Revision: D64398701 fbshipit-source-id: 9f7b417bf6e8da6758f9cac4167a8b581bfed8b7
Summary: Pull Request resolved: #3961 Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so. This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug. **What's a shadowed variable?** Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs. This diff fixes such an issue by renaming the variable. - If you approve of this diff, please use the "Accept & Ship" button :-) Reviewed By: meyering Differential Revision: D64398743 fbshipit-source-id: 3ec24a1655133ee0d3b94a55e38857ffa8853268
Summary: Pull Request resolved: #3960 Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so. This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug. **What's a shadowed variable?** Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs. This diff fixes such an issue by renaming the variable. - If you approve of this diff, please use the "Accept & Ship" button :-) Reviewed By: meyering Differential Revision: D64398709 fbshipit-source-id: b10e44b40aa1d3e21aeb5112eb93fb63d64d4118
Summary: Pull Request resolved: #3952 Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so. This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug. **What's a shadowed variable?** Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs. This diff fixes such an issue by renaming the variable. - If you approve of this diff, please use the "Accept & Ship" button :-) Reviewed By: asadoughi Differential Revision: D64398749 fbshipit-source-id: 0e8fd4ab8f6dbf780d4412083fa88fc0df3b89c2
aalekhpatel07
pushed a commit
that referenced
this pull request
Oct 17, 2024
…Lists (facebookresearch#3327) Summary: Pull Request resolved: facebookresearch#3327 **Context** 1. [Issue 2621](facebookresearch#2621) discuss inconsistency between OnDiskInvertedList and InvertedList. OnDiskInvertedList is supposed to handle disk based multiple Index Shards. Thus, we should name it differently when merging invls from index shard. 2. [Issue 2876](facebookresearch#2876) provides usecase of shifting ids when merging invls from different shards. **In this diff**, 1. To address #1 above, I renamed the merge_from function to merge_from_multiple without touching merge_from base class. why so? To continue to allow merge invl from one index to ondiskinvl from other index. 2. To address Enet4#2 above, I have added support of shift_ids in merge_from_multiple to shift ids from different shards. This can be used when each shard has same set of ids but different data. This is not recommended if id is already unique across shards. Reviewed By: mdouze Differential Revision: D55482518 fbshipit-source-id: 95470c7449160488d2b45b024d134cbc037a2083
aalekhpatel07
pushed a commit
that referenced
this pull request
Oct 17, 2024
…ookresearch#3527) Summary: Pull Request resolved: facebookresearch#3527 **Context** Design Doc: [Faiss Benchmarking](https://docs.google.com/document/d/1c7zziITa4RD6jZsbG9_yOgyRjWdyueldSPH6QdZzL98/edit) **In this diff** 1. Be able to reference codec and index from blobstore (bucket & path) outside the experiment 2. To support #1, naming is moved to descriptors. 3. Build index can be written as well. 4. You can run benchmark with train and then refer it in index built and then refer index built in knn search. Index serialization is optional. Although not yet exposed through index descriptor. 5. Benchmark can support index with different datasets sizes 6. Working with varying dataset now support multiple ground truth. There may be small fixes before we could use this. 7. Added targets for bench_fw_range, ivf, codecs and optimize. **Analysis of ivf result**: D58823037 Reviewed By: algoriddle Differential Revision: D57236543 fbshipit-source-id: ad03b28bae937a35f8c20f12e0a5b0a27c34ff3b
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.