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

Add Recall Tests #251

Merged

Conversation

naveentatikonda
Copy link
Member

Signed-off-by: Naveen Tatikonda navtat@amazon.com

Description

Adding recall tests to integration tests and backward compatibility tests. This includes two separate tests where the vectors are generated using some standard function in one test and randomly generated vectors in other test. 'l2' is the spaceType used to measure distance between two points.

Issues Resolved

#250

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.

@naveentatikonda naveentatikonda requested a review from a team January 6, 2022 21:01
@codecov-commenter
Copy link

codecov-commenter commented Jan 6, 2022

Codecov Report

Merging #251 (0a2b5f8) into main (fee4026) will increase coverage by 0.05%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #251      +/-   ##
============================================
+ Coverage     83.22%   83.28%   +0.05%     
- Complexity      865      866       +1     
============================================
  Files           123      123              
  Lines          3780     3780              
  Branches        359      359              
============================================
+ Hits           3146     3148       +2     
+ Misses          473      471       -2     
  Partials        161      161              
Impacted Files Coverage Δ
...ain/java/org/opensearch/knn/index/KNNSettings.java 83.21% <0.00%> (+1.45%) ⬆️

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 fee4026...0a2b5f8. Read the comment docs.

@naveentatikonda naveentatikonda force-pushed the add_recall_tests branch 3 times, most recently from bfb19bd to 28258ff Compare January 11, 2022 22:03
Copy link
Member

@martin-gaievski martin-gaievski left a comment

Choose a reason for hiding this comment

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

Lgtm, thx!

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
Copy link
Member

@jmazanec15 jmazanec15 left a comment

Choose a reason for hiding this comment

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

There seems to be some duplicate code in RecallIT and KNNBackwardsCompatibilityIT. Why is this necessary?

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
@naveentatikonda naveentatikonda merged commit 8a10fea into opensearch-project:main Jan 15, 2022
martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 7, 2022
* Add Recall Tests

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

* Calculate Recall using document ids and other minor changes

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
martin-gaievski pushed a commit to martin-gaievski/k-NN that referenced this pull request Mar 7, 2022
* Add Recall Tests

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

* Calculate Recall using document ids and other minor changes

Signed-off-by: Naveen Tatikonda <navtat@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
* Add Recall Tests

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>

* Calculate Recall using document ids and other minor changes

Signed-off-by: Naveen Tatikonda <navtat@amazon.com>
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.

5 participants