Add a common subclass for IndexNode::iterator #501
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #501 +/- ##
=========================================
+ Coverage 0 72.33% +72.33%
=========================================
Files 0 63 +63
Lines 0 4319 +4319
=========================================
+ Hits 0 3124 +3124
- Misses 0 1195 +1195 |
buqian-zilliz
force-pushed
the
iterator-new-interface
branch
from
de8b9f7
to
d1c6b95
Compare
include/knowhere/index_node.h
Outdated
| throw std::runtime_error("raw_distance not implemented"); | ||
| } | ||
|
|
||
| const bool refine_ratio_; |
include/knowhere/index_node.h
Outdated
| const bool refine_ratio_; | ||
| const bool refine_; | ||
|
|
||
| std::priority_queue<std::pair<float, int64_t>, std::vector<std::pair<float, int64_t>>, PairComparator> res_; |
There was a problem hiding this comment.
there are too many std::pair<float, int64_t> structs in the code. I'd create a simple 2-field structure with clear field names, because the current code uses .first or .second and who knows that they mean
| @@ -247,14 +247,24 @@ class IvfIndexNode : public IndexNode { | |||
| Status | |||
| TrainInternal(const DataSet& dataset, const Config& cfg); | |||
|
|
|||
| static bool | |||
| IsQuantized() { | |||
There was a problem hiding this comment.
you can make this function constexpr
Also, what about SCANN?
buqian-zilliz
force-pushed
the
iterator-new-interface
branch
from
2d38b6e
to
bdf16b5
Compare
|
|
||
| std::pair<int64_t, float> | ||
| Next() override { | ||
| if (!initialized_) { |
There was a problem hiding this comment.
Why can't this initialization action be placed inside the constructor? It seems this way, you won't need to write data in the HasNext interface, which looks more like a read-only interface.
There was a problem hiding this comment.
initialization must be done after constructor of subclass has finished, which we don't know when that'll happen in this class
buqian-zilliz
force-pushed
the
iterator-new-interface
branch
from
f48663f
to
dcf8b88
Compare
|
/unhold |
buqian-zilliz
force-pushed
the
iterator-new-interface
branch
from
dcf8b88
to
b0e1e51
Compare
…xes only need to implement next_batch, add impl for hnsw and ivf Signed-off-by: Buqian Zheng <zhengbuqian@gmail.com>
buqian-zilliz
force-pushed
the
iterator-new-interface
branch
from
b0e1e51
to
7e667ff
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: foxspy, zhengbuqian 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 |
so that different indexes only need to implement next_batch. Any index that gradually adds new points as the
iterator::Next()is called should implement their iterator by inheriting this new class.Updated the impl for supported ivf and hnsw index.
issue: #500
/kind: enhancement