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

Streaming PQ #689

Merged
merged 24 commits into from
Mar 17, 2023
Merged

Streaming PQ #689

merged 24 commits into from
Mar 17, 2023

Conversation

eddyxu
Copy link
Contributor

@eddyxu eddyxu commented Mar 16, 2023

Train IVF_PQ on sample dataset, and use streaming to compute PQ code without loading full dataset in memory.

@eddyxu eddyxu requested a review from changhiskhan March 16, 2023 18:38
@eddyxu eddyxu marked this pull request as ready for review March 16, 2023 18:40
@eddyxu eddyxu self-assigned this Mar 16, 2023
@eddyxu eddyxu added rust Rust related tasks vector Vector Search labels Mar 16, 2023
@eddyxu eddyxu requested a review from gsilvestrin March 16, 2023 18:50
Copy link
Contributor

@changhiskhan changhiskhan left a comment

Choose a reason for hiding this comment

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

Given how much has changed, could you re-run the sift benchmarks just to make sure the latency / recall profile looks roughly the same still?

@@ -130,6 +131,15 @@ impl MatrixView {
}
}

pub fn row(&self, i: usize) -> Option<Float32Array> {
if i >= self.num_rows() {
None
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be an error?

let ivf_params = IvfBuildParams {
num_partitions: vec_params.num_partitions as usize,
metric_type: vec_params.metric_type,
max_iters: 50,
Copy link
Contributor

Choose a reason for hiding this comment

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

are these values from FAISS?

};
let pq_params = PQBuildParams {
num_sub_vectors: vec_params.num_sub_vectors as usize,
num_bits: 8,
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have to change much to support configurable num_bits? is it just exposing a new API parameter? or is the underlying index creation hard coded to 8 bits?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

num_nbits is not configurable via user-facing API yet. 8 bits is just 1 byte. other than 8 bits, the other configuration needs some work .

mat: &MatrixView,
metric_type: MetricType,
) -> Result<FixedSizeListArray> {
self.train(mat, metric_type, 50).await?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we move hard coded constants into one place so we know what needs to change in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, yea, lemme just make this a parameter passed from user-facing api

let transforms = stream::iter(index_metadata.transforms)
.map(|tf| async move {
Ok::<Arc<dyn Transformer>, Error>(Arc::new(
OptimizedProductQuantizer::load(
Copy link
Contributor

Choose a reason for hiding this comment

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

i thought this doesn't use OPQ yet?

@eddyxu
Copy link
Contributor Author

eddyxu commented Mar 16, 2023

Running on /benchmark/sift/run_all.sh

Running with ivf512
nprobes: 1, refine=None, recall@10=0.546, mean(s)=0.00124342679977417
nprobes: 1, refine=1, recall@10=0.573, mean(s)=0.001298525333404541
nprobes: 1, refine=5, recall@10=0.780, mean(s)=0.0016234064102172853
nprobes: 1, refine=10, recall@10=0.810, mean(s)=0.0021506214141845705
nprobes: 10, refine=None, recall@10=0.643, mean(s)=0.001989481449127197
nprobes: 10, refine=1, recall@10=0.643, mean(s)=0.002009923458099365
nprobes: 10, refine=5, recall@10=0.968, mean(s)=0.002357909679412842
nprobes: 10, refine=10, recall@10=0.975, mean(s)=0.0028011083602905275
nprobes: 50, refine=None, recall@10=0.627, mean(s)=0.005469377040863037
nprobes: 50, refine=1, recall@10=0.647, mean(s)=0.00498615026473999
nprobes: 50, refine=5, recall@10=0.972, mean(s)=0.005275309085845947
nprobes: 50, refine=10, recall@10=0.995, mean(s)=0.006533384323120117
nprobes: 100, refine=None, recall@10=0.650, mean(s)=0.008666768074035644
nprobes: 100, refine=1, recall@10=0.683, mean(s)=0.009069747924804687
nprobes: 100, refine=5, recall@10=0.973, mean(s)=0.009081656932830811
nprobes: 100, refine=10, recall@10=0.992, mean(s)=0.009473004341125489

@eddyxu eddyxu merged commit 299af4a into main Mar 17, 2023
@eddyxu eddyxu deleted the lei/stream_pq branch March 17, 2023 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Rust related tasks vector Vector Search
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants