Add iterator for ivf_sq_8 #414
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cqy123456 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@cqy123456 🔍 Important: PR Classification Needed! For efficient project management and a seamless review process, it's essential to classify your PR correctly. Here's how:
For any PR outside the kind/improvement category, ensure you link to the associated issue using the format: “issue: #”. Thanks for your efforts and contribution to the community!. |
| : nullptr; | ||
| InvertedListScanner* scanner = get_InvertedListScanner(false, sel); | ||
| scanner->set_query(workspace->query_data); | ||
| scanner->set_list(list_no, coarse_list_centroid_dist); |
There was a problem hiding this comment.
add a line : add centroid id and distance for quantizer.
| continue; | ||
| } | ||
| float accu = accu0 + dc.query_to_code(codes); | ||
| counter_back ++; |
| heap_push<CMax<float, int64_t>>(counter_back, distances, labels, accu, ids[j]); | ||
| nup++; | ||
| } | ||
| } |
There was a problem hiding this comment.
missing return statement.
thirdparty/faiss/faiss/IndexIVF.cpp
Outdated
| InvertedLists::ScopedIds sids( | ||
| invlists, list_no, segment_offset); | ||
|
|
||
| size_t n_heap = scanner->scan_codes_and_push_back( |
There was a problem hiding this comment.
temp variable n_heap is not used. discussed with @alwayslove2013 and we can remove it.
There was a problem hiding this comment.
sorry for being not precise in the previous comment: I meant to simply remove this local variable while scan_codes_and_push_back can still keep the return value for future use(could be used for metrics) and to have a aligned behavior with scan_codes.
cqy123456
force-pushed
the
ivf_iterator
branch
from
1964e7b
to
d182a50
Compare
cqy123456
force-pushed
the
ivf_iterator
branch
from
d182a50
to
ca8f376
Compare
cqy123456
force-pushed
the
ivf_iterator
branch
from
ca8f376
to
3ed85f9
Compare
| } | ||
|
|
||
| // compute backup_count_threshold - (nprobe / nlist) * count | ||
| size_t nprobe = workspace->search_params->nprobe |
There was a problem hiding this comment.
what if workspace->search_params is nullptr?
|
|
||
| std::optional<std::pair<float, idx_t>> IndexIVF::getIteratorNext( | ||
| IVFIteratorWorkspace* workspace) const { | ||
| auto scan_one_list_then_add_to_backup = |
There was a problem hiding this comment.
please add a check if workspace is nullptr
| @@ -89,6 +90,30 @@ struct InvertedListScanner; | |||
| struct IndexIVFStats; | |||
| struct CodePacker; | |||
|
|
|||
| struct IVFIteratorWorkspace { | |||
There was a problem hiding this comment.
Please add very detailed comments on what this structure does exactly and how it is used
| @@ -313,6 +338,19 @@ struct IndexIVF : Index, IndexIVFInterface { | |||
| RangeSearchResult* result, | |||
| const SearchParameters* params = nullptr) const override; | |||
|
|
|||
| std::unique_ptr<IVFIteratorWorkspace> getIteratorWorkspace( | |||
There was a problem hiding this comment.
Please add comments for this method about what it does
Signed-off-by: cqy123456 <qianya.cheng@zilliz.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #414 +/- ##
=========================================
+ Coverage 0 72.32% +72.32%
=========================================
Files 0 63 +63
Lines 0 4293 +4293
=========================================
+ Hits 0 3105 +3105
- Misses 0 1188 +1188 |
|
please hold until #501 is merged. It slightly changed how we should implement iterator for indexes. It should make this PR much simpler. |
issue: #425