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.

Sign up for GitHub

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

Jump to bottom

Make doc and query count configurable in benchmark #270

Merged
merged 3 commits into from
Jan 27, 2022
Merged

Make doc and query count configurable in benchmark #270

merged 3 commits into from
Jan 27, 2022

Conversation

jmazanec15
Copy link
Member

Signed-off-by: John Mazanec jmazane@amazon.com

Description

Makes the document and query count configurable in the benchmarking tool. With this functionality, users can now specify to only index or search a subset of the vectors in the data set. This is useful for indices that require training that may only need a subset of the data set for training.

A query or ingest step might look like this now:

steps:
...
  - name: ingest
    index_name: target_index
    field_name: target_field
    bulk_size: 500
    dataset_format: hdf5
    dataset_path: mydata.hdf5
    doc_count: 50000
...
  - name: query
    k: 100
    r: 1
    index_name: target_index
    field_name: target_field
    dataset_format: hdf5
    dataset_path: mydata.hdf5
    query_count: 5
    neighbors_format: hdf5
    neighbors_path: mydata.hdf5

Check List

  • Commits are signed as per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@jmazanec15
Make doc and query count configurable in benchmark
a864a4a 
Makes the document and query count configurable in the benchmarking
tool. With this functionality, users can now specify to only index or
search a subset of the vectors in the data set. This is useful for
indices that require training that may only need a subset of the data set
for training.

Signed-off-by: John Mazanec <jmazane@amazon.com>
@jmazanec15 jmazanec15 requested review from a team, martin-gaievski and VijayanB January 26, 2022 22:10
@codecov-commenter
Copy link

codecov-commenter commented Jan 26, 2022 •
edited
Loading

Codecov Report

Merging #270 (2dd6f05) into main (6e859f5) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #270   +/-   ##
=========================================
  Coverage     83.38%   83.38%           
  Complexity      884      884           
=========================================
  Files           127      127           
  Lines          3833     3833           
  Branches        361      361           
=========================================
  Hits           3196     3196           
  Misses          475      475           
  Partials        162      162

Continue to review full report at Codecov.

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

@martin-gaievski
Copy link
Member

What are default values for new params?

And another question - does it make sense to reflect values of new params in test result, maybe with an average?

@jmazanec15
Copy link
Member Author

@martin-gaievski The default is the entire dataset. So, if you dont specify anything it will index each vector in the dataset.

And another question - does it make sense to reflect values of new params in test result, maybe with an average?

All metrics take into account the dataset size. For instance, querying produces the p50, p90 and p99 metrics. For ingest, it is the total time. In the future we could add an ingest metric that is docs/sec, but I think thats outside scope of this change for now.

VijayanB
VijayanB reviewed Jan 26, 2022
View reviewed changes
benchmarks/perf-tool/okpt/test/steps/steps.py
Returns:
Recall at R
"""
correct = 0.0
query = 0
while True:
for query in range(query_count):
true_neighbors = neighbor_dataset.read(1)
if true_neighbors is None:
break
true_neighbors_set = set(true_neighbors[0][:k])
for j in range(r):
Copy link
Member

Choose a reason for hiding this comment

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

FAR: may be change r to limit or something meaningful.

Copy link
Member Author

Choose a reason for hiding this comment

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

r is a technical term in recall@r. I calculate it now as the fraction of (# of top r results returned by the query are in the ground truth k set) / r.

However, I think I may have this mixed up a little bit and I will need to refactor this to follow how faiss computes it: https://github.com/facebookresearch/faiss/blob/main/faiss/AutoTune.cpp#L60-L97. I will make a separate issue for this.

@jmazanec15
Do not override variables
bca569d 
Signed-off-by: John Mazanec <jmazane@amazon.com>
@jmazanec15
Update var name
2dd6f05 
Signed-off-by: John Mazanec <jmazane@amazon.com>
VijayanB
VijayanB approved these changes Jan 27, 2022
View reviewed changes
Copy link
Member

@VijayanB VijayanB left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@jmazanec15 jmazanec15 merged commit 76ec5cd into opensearch-project:main Jan 27, 2022
martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 7, 2022
@jmazanec15 @martin-gaievski
Make doc and query count configurable in benchmark (opensearch-projec…
85e37c6 
…t#270)

Makes the document and query count configurable in the benchmarking
tool. With this functionality, users can now specify to only index or
search a subset of the vectors in the data set. This is useful for
indices that require training that may only need a subset of the data set
for training.

Signed-off-by: John Mazanec <jmazane@amazon.com>
martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 7, 2022
@jmazanec15 @martin-gaievski
Make doc and query count configurable in benchmark (opensearch-projec…
2b85d0a 
…t#270)

Makes the document and query count configurable in the benchmarking
tool. With this functionality, users can now specify to only index or
search a subset of the vectors in the data set. This is useful for
indices that require training that may only need a subset of the data set
for training.

Signed-off-by: John Mazanec <jmazane@amazon.com>
Signed-off-by: Martin Gaievski <gaievski@amazon.com>
martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 30, 2022
@jmazanec15
Make doc and query count configurable in benchmark (opensearch-projec…
ad28f07 
…t#270)

Makes the document and query count configurable in the benchmarking
tool. With this functionality, users can now specify to only index or
search a subset of the vectors in the data set. This is useful for
indices that require training that may only need a subset of the data set
for training.

Signed-off-by: John Mazanec <jmazane@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martin-gaievski martin-gaievski martin-gaievski approved these changes

@VijayanB VijayanB VijayanB approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jmazanec15 @codecov-commenter @martin-gaievski @VijayanB