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

MB-60029 - Add index types for tuning for recall/latency. #46

Merged
merged 6 commits into from
Jan 16, 2024

Conversation

metonymic-smokey
Copy link
Member

@metonymic-smokey metonymic-smokey commented Jan 9, 2024

Extends VectorField interface for optimizing for either recall/latency.

vector.go Outdated Show resolved Hide resolved
vector.go Outdated Show resolved Hide resolved
vector.go Outdated
@@ -52,3 +52,8 @@ const IndexTypeRecall = "recall"
const IndexTypeLatency = "latency"

const DefaultIndexType = IndexTypeRecall

var SupportedIndexTunables = map[string]struct{}{
Copy link
Member

Choose a reason for hiding this comment

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

Poor naming again - very vague. Change to SupportedIndexConfigOptimizations or some.

@abhinavdangeti
Copy link
Member

Thinking a little more here - how about offering a numeric setting as opposed to these 2 strings, it'd be more flexible and configurable.

So one config - OptimizeForRecall - can take a numeric value ranging from 0 through 10.
Default is at 5, which means no changes to default settings.
Any number from 5 through 10 will optimize more for recall and any from 4 through 0 more for latency.

The growth could be a factor - so you'll need to apply settings based on your experiments.
Thoughts?

@metonymic-smokey metonymic-smokey marked this pull request as ready for review January 10, 2024 08:37
@metonymic-smokey metonymic-smokey requested a review from a team January 10, 2024 08:38
vector.go Outdated Show resolved Hide resolved
vector.go Outdated
@@ -45,3 +47,13 @@ var SupportedSimilarityMetrics = map[string]struct{}{
EuclideanDistance: {},
CosineSimilarity: {},
}

const IndexOptimizationForRecall = "recall"
const IndexOptimizationForLatency = "latency"
Copy link
Member

Choose a reason for hiding this comment

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

Do you not foresee us having to accommodate anything else here in the future?

vector.go Outdated Show resolved Hide resolved
vector.go Outdated Show resolved Hide resolved
vector.go Outdated

const DefaultIndexOptimization = IndexOptimizationForRecall

var SupportedIndexConfigOptimizations = map[string]struct{}{
Copy link
Member

Choose a reason for hiding this comment

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

SupportedVectorIndexOptimizations*

vector.go Outdated
@@ -23,6 +23,8 @@ type VectorField interface {
Dims() int
// Similarity metric to be used for scoring the vectors
Similarity() string
// index type - tuned for either latency/recall
IndexOptimization() string
Copy link
Member

Choose a reason for hiding this comment

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

IndexOptimizedFor() string

vector.go Outdated Show resolved Hide resolved
Copy link
Member

@abhinavdangeti abhinavdangeti left a comment

Choose a reason for hiding this comment

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

Every other setting (non-string) I could think of came with some flaws - so let's go with this approach. Requesting some changes here though.

vector.go Show resolved Hide resolved
vector.go Outdated Show resolved Hide resolved
vector.go Outdated Show resolved Hide resolved
@abhinavdangeti abhinavdangeti merged commit 54ca226 into master Jan 16, 2024
9 checks passed
@abhinavdangeti abhinavdangeti deleted the ivf-tuning branch January 16, 2024 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants