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

Implement the merge function in HNSW #432

Merged
merged 4 commits into from
Feb 25, 2025
Merged

Implement the merge function in HNSW #432

merged 4 commits into from
Feb 25, 2025

Conversation

inabao
Copy link
Collaborator

@inabao inabao commented Feb 24, 2025

@inabao inabao self-assigned this Feb 24, 2025
@inabao inabao added the kind/feature New feature or request label Feb 24, 2025
Copy link

codecov bot commented Feb 24, 2025

Codecov Report

Attention: Patch coverage is 99.04762% with 1 line in your changes missing coverage. Please review.

@@            Coverage Diff             @@
##             main     #432      +/-   ##
==========================================
+ Coverage   91.24%   91.55%   +0.31%     
==========================================
  Files         151      160       +9     
  Lines        9859     9905      +46     
==========================================
+ Hits         8996     9069      +73     
+ Misses        863      836      -27     
Flag Coverage Δ
cpp 91.55% <99.04%> (+0.31%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
common 94.07% <ø> (-1.10%) ⬇️
datacell 92.38% <ø> (+0.25%) ⬆️
index 90.67% <100.00%> (-0.34%) ⬇️
simd 87.71% <ø> (+3.91%) ⬆️

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 1502384...2808ac5. Read the comment docs.

struct ODescentParameter : public Parameter {
public:
void
FromJson(const JsonType& json) override;
Copy link
Collaborator

Choose a reason for hiding this comment

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

construction function

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

jinjiabao.jjb added 2 commits February 24, 2025 19:12
Signed-off-by: jinjiabao.jjb <[email protected]>
Signed-off-by: jinjiabao.jjb <[email protected]>
Allocator* allocator_;

const ODescentParameterPtr odescent_parameter_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor: use short name odescent_param_

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

if (neighbors.size() > max_degree_) {
neighbors.resize(max_degree_);
if (neighbors.size() > odescent_parameter_->max_degree) {
neighbors.resize(odescent_parameter_->max_degree);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: Only do shrink operation ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes

float alpha,
int64_t turn,
float sample_rate,
struct IdsSequence {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can replace to std::vector ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Signed-off-by: jinjiabao.jjb <[email protected]>
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

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

@inabao inabao merged commit a85c0d5 into main Feb 25, 2025
18 checks passed
@inabao inabao deleted the merge-impl branch February 25, 2025 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants