Skip to content
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

support safe update #308

Merged
merged 1 commit into from
Jan 16, 2025
Merged

support safe update #308

merged 1 commit into from
Jan 16, 2025

Conversation

ShawnShawnYou
Copy link
Collaborator

@ShawnShawnYou ShawnShawnYou commented Jan 9, 2025

  • add vector check for update
  • if new vector is far from original vector, the update operation is failed

closes: #225

LOG_ERROR_AND_RETURNS(ErrorType::INVALID_ARGUMENT,
fmt::format("failed to pretrain(invalid argument): base tag id "
"({}) doesn't belong to index",
base_tag_id));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a minor fix: error message "bas tag id" to "base tag id"

* @return result indicates whether the update operation is successful.
*/
virtual tl::expected<bool, Error>
UpdateVector(int64_t id, const DatasetPtr& new_base, bool need_fine_tune = false) {
UpdateVector(int64_t id, const DatasetPtr& new_base, bool force_update = false) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p.s. We are not responsible for the consequences of this interface if the user specifies force_update = true

Copy link

codecov bot commented Jan 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

@@            Coverage Diff             @@
##             main     #308      +/-   ##
==========================================
+ Coverage   49.75%   49.81%   +0.06%     
==========================================
  Files         331      335       +4     
  Lines       32701    33124     +423     
  Branches     4129     4190      +61     
==========================================
+ Hits        16271    16502     +231     
- Misses      15568    15755     +187     
- Partials      862      867       +5     
Components Coverage Δ
common ∅ <ø> (∅)
datacell ∅ <ø> (∅)
index ∅ <ø> (∅)
simd ∅ <ø> (∅)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e87defc...9ef9613. Read the comment docs.

Copy link
Collaborator

@wxyucs wxyucs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

src/index/hnsw.cpp Show resolved Hide resolved
src/index/hnsw.cpp Show resolved Hide resolved
for (int i = 0; i < result.value()->GetDim(); i++) {
float neighbor_dist =
std::reinterpret_pointer_cast<hnswlib::HierarchicalNSW>(alg_hnsw_)
->getDistanceByLabel(result.value()->GetIds()[i], new_base_vec);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#309 provides a batch interface, which can be considered for use later on

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix this in other pr?

Signed-off-by: zhongxiaoyao.zxy <[email protected]>
@ShawnShawnYou ShawnShawnYou self-assigned this Jan 10, 2025
@ShawnShawnYou ShawnShawnYou added kind/improvement Code improvements (variable/function renaming, refactoring, etc. ) version/0.13 labels Jan 10, 2025
Copy link
Collaborator

@inabao inabao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@LHT129 LHT129 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@jiaweizone jiaweizone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ShawnShawnYou ShawnShawnYou merged commit cfd685a into main Jan 16, 2025
15 of 16 checks passed
@ShawnShawnYou ShawnShawnYou deleted the support-safe-update branch January 16, 2025 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/improvement Code improvements (variable/function renaming, refactoring, etc. ) size/M version/0.13
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support in-place update
5 participants