Skip to content
This repository has been archived by the owner on Aug 16, 2023. It is now read-only.

Sync perform of HNSW/DiskANN #924

Merged
merged 1 commit into from
Jun 10, 2023
Merged

Sync perform of HNSW/DiskANN #924

merged 1 commit into from
Jun 10, 2023

Conversation

liliu-z
Copy link
Member

@liliu-z liliu-z commented Jun 10, 2023

Issue: #910

We need to sync the perform of Indexes in Knowhere:

  1. When no specific parameter comes in, we use default but valid value. So no exception should be throw out in the case.
  2. When a specific parameter comes in, we need to check its validation and throw exception once it is bad.
  3. It is not acceptable to modify the parameter if user explicitly write it.

For HNSW:
We simple made the ef range to k - max, and default is min(k, 16).
For DiskANN:
We did the same thing.

@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liliu-z

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mergify
Copy link

mergify bot commented Jun 10, 2023

@liliu-z Please associate the related issue to the body of your Pull Request. (eg. “issue: #”)

@liliu-z
Copy link
Member Author

liliu-z commented Jun 10, 2023

/kind improvement

@liliu-z
Copy link
Member Author

liliu-z commented Jun 10, 2023

/lgtm

@sre-ci-robot
Copy link

@liliu-z: you cannot LGTM your own PR.

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@cqy123456
Copy link
Collaborator

/lgtm

@sre-ci-robot sre-ci-robot merged commit 998a8b8 into milvus-io:1.x Jun 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants