Iterator for diskann #874
Iterator for diskann #874
Conversation
|
@alwayslove2013 🔍 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!. |
| aligned_query_float[q_dim] = 0; | ||
| } | ||
| for (uint32_t i = 0; i < q_dim; i++) { | ||
| aligned_query_T[i] = (T) ((float) aligned_query_T[i] / query_norm); |
There was a problem hiding this comment.
no check for query_norm == 0
There was a problem hiding this comment.
aligned_query_T[q_dim]
aligned_query_float[q_dim]For the fixed version, this two elements do not get populated for query_norm == 0 use case.
There was a problem hiding this comment.
Basically,
if (query_norm != 0) {
query_norm = std::sqrt(query_norm);
if (metric == diskann::Metric::INNER_PRODUCT) {
aligned_query_T[q_dim] = 0;
aligned_query_float[q_dim] = 0;
}
for (uint32_t i = 0; i < q_dim; i++) {
aligned_query_T[i] = (T) ((float) aligned_query_T[i] / query_norm);
aligned_query_float[i] /= query_norm;
}
}
else {
.. do what.. ?
}There was a problem hiding this comment.
No need to do anything.
When the metric is ip or cosine and query_norm==0, workspace next batch will be skipped. iterator's results will not be updated and will always be empty, which means Iterator.HasNext() will be false.
void PQFlashIndex<T>::getIteratorNextBatch(IteratorWorkspace<T> *workspace) {
if (metric == diskann::Metric::INNER_PRODUCT ||
metric == diskann::Metric::COSINE) {
if (workspace->query_norm == 0) {
return;
}
}
...
}
I will modify the logic to make it clearer.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #874 +/- ##
=========================================
+ Coverage 0 77.67% +77.67%
=========================================
Files 0 84 +84
Lines 0 6364 +6364
=========================================
+ Hits 0 4943 +4943
- Misses 0 1421 +1421 |
alwayslove2013
force-pushed
the
iterator_for_diskann
branch
from
f85c0df
to
a6fdc55
Compare
|
/kind improvement |
diskann iterator for knowhere (opensource) Signed-off-by: min.tian <min.tian.cn@gmail.com> remove diskann-range_search, use index_node-range_search instead Signed-off-by: min.tian <min.tian.cn@gmail.com> pre-commit style Signed-off-by: min.tian <min.tian.cn@gmail.com> knowhere-diskann iterator fix: skip if query_norm == 0; remove range_search test - min_k < max_k Signed-off-by: min.tian <min.tian.cn@gmail.com>
alwayslove2013
force-pushed
the
iterator_for_diskann
branch
from
a6fdc55
to
73e7a44
Compare
|
/lgtm |
src/index/diskann/diskann.cc
Outdated
| auto filter_ratio = static_cast<float>(search_conf.filter_threshold.value()); | ||
|
|
||
| bool use_reorder_data = false; | ||
| bool for_tuning = false; |
There was a problem hiding this comment.
these two default to false? Possibly pass them in from config?
There was a problem hiding this comment.
both removed.
Signed-off-by: min.tian <min.tian.cn@gmail.com>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alwayslove2013, PwzXxm 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 |
|
/hold |
|
/unhold |
issue: milvus-io/milvus#34816
co-author: @xxxlzhxxx
report: https://zilliverse.feishu.cn/docx/SEnCdj247oMxnXxIrp2cNH5knqd?from=from_copylink