-
Notifications
You must be signed in to change notification settings - Fork 197
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
IVF-PQ: tighten the test criteria #1135
IVF-PQ: tighten the test criteria #1135
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Not yet, sorry! I'm doing a bit more investigation on couple edge cases |
@achirkin got it! No rush for sure |
Test failures to be expected until #1141 is merged |
Codecov ReportBase: 87.99% // Head: 87.99% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## branch-23.02 #1135 +/- ##
=============================================
Coverage 87.99% 87.99%
=============================================
Files 21 21
Lines 483 483
=============================================
Hits 425 425
Misses 58 58 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
/merge |
In #1135, we adjusted the min_recall values to report if any regressions happen in ivf-pq. However, `eval_neighbours` function, which is used in several ANN test suites, doesn't fail unless the regression is really large (it prints a warning if the calculated recall is "slightly" smaller than the expected recall). In this PR, I make `eval_neighbours` always fail if the calculated recall is smaller than the expected recall. Slightly adjust the tests and do a small refactoring along the way. Authors: - Artem M. Chirkin (https://github.com/achirkin) - Corey J. Nolet (https://github.com/cjnolet) Approvers: - Tamas Bela Feher (https://github.com/tfeher) URL: #1156
Make the recall reporting a bit more verbose and try to tighten the
min_recall
for various test cases. This should help spot any regressions in future and improve our understanding of ivf-pq performance for various inputs.