implement the interfaces for add, knnsearch, serialize in pyramid #231

inabao · 2024-12-18T09:41:58Z

#166

src/index/pyramid_zparameters.cpp

src/index/pyramid.cpp

src/index/pyramid.h

LHT129

LGTM

jiaweizone · 2025-01-01T16:47:49Z

src/index/pyramid.cpp

+// Function to convert BinarySet to a Binary
+Binary
+binaryset_to_binary(const BinarySet binarySet) {
+    size_t totalSize = 0;


totalSize -> total_size

jiaweizone · 2025-01-01T16:47:57Z

src/index/pyramid.cpp

 namespace vsag {

+// Function to convert BinarySet to a Binary
+Binary
+binaryset_to_binary(const BinarySet binarySet) {


binarySet -> binary_set

jiaweizone · 2025-01-01T16:52:29Z

src/index/pyramid.cpp

+    size_t offset = 0;
+
+    for (const auto& key : keys) {
+        size_t keySize = key.size();


keySize -> key_size

jiaweizone · 2025-01-01T16:53:27Z

src/index/pyramid.cpp

+
+BinarySet
+binary_to_binaryset(const Binary binary) {
+    BinarySet binarySet;


binarySet -> binary_set

jiaweizone · 2025-01-01T16:53:38Z

src/index/pyramid.cpp

+    size_t offset = 0;
+
+    while (offset < binary.size) {
+        size_t keySize;


keySize -> key_size

jiaweizone · 2025-01-01T17:04:46Z

src/index/pyramid.cpp

+        start = end + 1;
+        end = str.find(delimiter, start);
+    }
+    std::string lastToken = str.substr(start);


lastToken -> last_token

jiaweizone · 2025-01-01T17:06:57Z

src/index/pyramid.cpp

+template <typename T>
+using Deque = std::deque<T, vsag::AllocatorWrapper<T>>;
+
+constexpr static const char PART_SLASH = '/';


move to file head ?

jiaweizone · 2025-01-01T17:08:08Z

src/index/pyramid.cpp

+constexpr static const char PART_SLASH = '/';
+constexpr static const char PART_OCTOTHORPE = '#';
+std::vector<std::string>
+split(const std::string& str, char delimiter) {


can use std lib or boost lib split implement ?

It seems that there is no split function in the standard library. Currently, vsag has no dependencies on Boost, so it would be a bit excessive to introduce Boost just for the split function.

OK，can move to common utils file

Please reference high performance implement (eg. boost / absl)

jiaweizone · 2025-01-01T17:13:08Z

src/index/pyramid.cpp

+        if (indexes_.find(result[0]) == indexes_.end()) {
+            indexes_[result[0]] = std::make_shared<IndexNode>(commom_param_.allocator_.get());
+        }
+        std::shared_ptr<IndexNode> node = indexes_.at(result[0]);


minor: here can reduce one find cost

jiaweizone · 2025-01-01T17:14:08Z

src/index/pyramid.cpp

+                node->children[result[j]] =
+                    std::make_shared<IndexNode>(commom_param_.allocator_.get());
+            }
+            node = node->children.at(result[j]);


codecov · 2025-01-02T04:11:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

see 24 files with indirect coverage changes

jiaweizone · 2025-01-02T17:30:04Z

src/index/pyramid.cpp

+constexpr static const char PART_SLASH = '/';
+constexpr static const char PART_OCTOTHORPE = '#';
+std::vector<std::string>
+split(const std::string& str, char delimiter) {


OK，can move to common utils file

jiaweizone · 2025-01-02T17:35:55Z

src/index/pyramid.cpp

+    auto data_vectors = base->GetFloat32Vectors();
+    for (int i = 0; i < data_num; ++i) {
+        std::string current_path = path[i];
+        auto result = split(current_path, PART_SLASH);


result -> path_slices ?

jiaweizone · 2025-01-02T18:05:50Z

src/index/pyramid.cpp

+constexpr static const char PART_SLASH = '/';
+constexpr static const char PART_OCTOTHORPE = '#';
+std::vector<std::string>
+split(const std::string& str, char delimiter) {


Please reference high performance implement (eg. boost / absl)

jiaweizone · 2025-01-02T18:08:00Z

src/index/pyramid.cpp

+    auto path = query->GetPaths();
+
+    std::string current_path = path[0];
+    auto parsed_path = split(current_path, PART_SLASH);


parsed_path -> path_slices ?

jiaweizone · 2025-01-02T18:09:21Z

src/index/pyramid.cpp

+        ret->Dim(0)->NumElements(1);
+        return ret;
+    }
+    std::shared_ptr<IndexNode> root = indexes_.at(parsed_path[0]);


minor: reduce cost

jiaweizone · 2025-01-02T18:09:58Z

src/index/pyramid.cpp

+        ret->Dim(0)->NumElements(1);
+        return ret;
+    }
+    std::shared_ptr<IndexNode> root = indexes_.at(parsed_path[0]);


minor: reduce cost

jiaweizone · 2025-01-02T18:10:00Z

src/index/pyramid.cpp

+        ret->Dim(0)->NumElements(1);
+        return ret;
+    }
+    std::shared_ptr<IndexNode> root = indexes_.at(parsed_path[0]);


minor: reduce cost

jiaweizone · 2025-01-02T18:10:17Z

src/index/pyramid.cpp

+        ret->Dim(0)->NumElements(1);
+        return ret;
+    }
+    std::shared_ptr<IndexNode> root = indexes_.at(parsed_path[0]);


minor: reduce cost

jiaweizone · 2025-01-02T18:19:56Z

src/index/pyramid.cpp

+            auto result = node->index->KnnSearch(query, k, parameters, invalid);
+            if (result.has_value()) {
+                DatasetPtr r = result.value();
+                for (int i = 0; i < r->GetDim(); ++i) {


r->GetDim() ?
is r->GetElements() ?

r->GetDim() denotes the number of the results for single query. r->GetElements() denotes the number of queries

reminder: Update in next PR

jiaweizone · 2025-01-02T18:24:52Z

src/index/pyramid.cpp

+            }
+        } else {
+            for (const auto& item : node->children) {
+                candidate_indexes.emplace_back(item.second);


do search for all sub tree ?

If a node has an index, then its child nodes will not be searched.

It's necessary to introduce retrieval logic that recursively flows from higher layers to lower layers.

src/default_allocator.h

jiaweizone · 2025-01-06T11:34:22Z

src/index/pyramid.cpp

+            auto result = node->index->KnnSearch(query, k, parameters, invalid);
+            if (result.has_value()) {
+                DatasetPtr r = result.value();
+                for (int i = 0; i < r->GetDim(); ++i) {


reminder: Update in next PR

Signed-off-by: jinjiabao.jjb <[email protected]>

jiaweizone

some issues will fix in next PR

inabao added the kind/feature New feature or request label Dec 18, 2024

inabao self-assigned this Dec 18, 2024

inabao requested review from wxyucs, LHT129, ShawnShawnYou and jiaweizone as code owners December 18, 2024 09:41

pull-request-size bot added the size/XL label Dec 18, 2024

inabao force-pushed the new-pyramid-implementation branch 5 times, most recently from 22b8903 to 5d932f3 Compare December 19, 2024 08:17

LHT129 requested changes Dec 23, 2024

View reviewed changes

src/index/pyramid_zparameters.cpp Outdated Show resolved Hide resolved

src/index/pyramid.cpp Outdated Show resolved Hide resolved

src/index/pyramid.h Outdated Show resolved Hide resolved

src/index/pyramid.h Outdated Show resolved Hide resolved

wxyucs added the version/0.13 label Dec 24, 2024

LHT129 self-requested a review December 25, 2024 06:12

LHT129 approved these changes Dec 25, 2024

View reviewed changes

jiaweizone reviewed Jan 1, 2025

View reviewed changes

inabao force-pushed the new-pyramid-implementation branch from 2ee1a92 to 3b6956c Compare January 2, 2025 03:38

jiaweizone reviewed Jan 2, 2025

View reviewed changes

inabao force-pushed the new-pyramid-implementation branch 2 times, most recently from c263cdd to d02dcaa Compare January 6, 2025 06:43

jiaweizone reviewed Jan 6, 2025

View reviewed changes

inabao force-pushed the new-pyramid-implementation branch 3 times, most recently from 7b23037 to 5f571db Compare January 7, 2025 07:28

implement the interfaces for add, knnsearch, serialize in pyramid

3d47fe7

Signed-off-by: jinjiabao.jjb <[email protected]>

inabao force-pushed the new-pyramid-implementation branch from 0270eea to 3d47fe7 Compare January 8, 2025 13:10

jiaweizone approved these changes Jan 9, 2025

View reviewed changes

inabao merged commit 462ae8c into main Jan 9, 2025
11 checks passed

inabao deleted the new-pyramid-implementation branch January 9, 2025 13:59

inabao mentioned this pull request Jan 11, 2025

define the "pyramid" index to support vector search with paths #166

Open

implement the interfaces for add, knnsearch, serialize in pyramid #231

implement the interfaces for add, knnsearch, serialize in pyramid #231

Conversation

inabao commented Dec 18, 2024

LHT129 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 2, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiaweizone left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 2, 2025 •

edited

Loading