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

Small changes to metrics computation + additional metrics #107

Merged
merged 7 commits into from
Mar 7, 2019

Conversation

maumueller
Copy link
Collaborator

This PR makes the following changes:

  • Added mechanism to allow an implementation to report on additional characteristics. This is showcast by having IVF and HNSW (from faiss) report the number of distance computations done to answer the set of queries (which is a general benchmark of the quality more independent than running times).
  • Small refactorings in metric computation, report not only on avg but also standard deviation. (To be later added as error bars in plots.)
  • Store computed metrics inside the result file to avoid recomputation
  • Improved plotting time by a factor ~4 by better caching of hdf5 access and small changes to the metric computation

The general goal of these changes is to make it easier to work with our result files, e.g. for visualization of results beyond those provided at the moment.

We use it to create plots like this, which provide more inside into what the distribution of individual recall values looks compared to the average (the single dot):
image

This gives more details into the inner workings of different algorithmic ideas. (To be discussed: should such plottings scripts be added to ann-benchmarks as well?)

for i in range(len(run_distances)):
t = threshold(dataset_distances[i], count, epsilon)
actual = 0
for j in range(min(count, len(run_distances[i]))):
Copy link
Owner

@erikbern erikbern Mar 6, 2019

Choose a reason for hiding this comment

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

not blocking but could have been a bit more elegant :)

for d in run_distances[i][:count]:
    if d <= t:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! Much better. Changed it in cdda15b. (Also removed the break statement because it assumes that algorithms sort their answers to queries.)

@erikbern
Copy link
Owner

erikbern commented Mar 6, 2019

i think this change looks good – i don't find the plots super easy to understand, but the code seems fine!

@erikbern
Copy link
Owner

erikbern commented Mar 6, 2019

looks like build is passing except for flann (will solve separately)

I think we don't enforce that answers to queries
should be sorted in some way. The break statement
would be the only place where this would be visible.
@maumueller
Copy link
Collaborator Author

looks like build is passing except for flann (will solve separately)

That seems to be related to flann-lib/flann#399. I've added the libraries added their as explicit requirements in ce74df9, but their seems to be another issue with the Python wrapper.

@maumueller
Copy link
Collaborator Author

i think this change looks good – i don't find the plots super easy to understand, but the code seems fine!

A caption is definitely necessary. ;-) We made quite a lot of new plots for a recent paper. I don't think that all of such scripts should go into the base repo. What do you think about a repo just for additional plotting functionality? (ann-benchmarks-plotting? Would be easiest if everything would go under an ann-benchmarks organization.)

@maumueller
Copy link
Collaborator Author

Flann is still not building properly, opened up flann-lib/flann#406.

@erikbern erikbern merged commit 37a70ca into erikbern:master Mar 7, 2019
@erikbern
Copy link
Owner

erikbern commented Mar 7, 2019

thanks! i might disable flann for now

erikbern added a commit that referenced this pull request Apr 14, 2023
Small changes to metrics computation + additional metrics
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