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

Initial commit for the global-phase rank-score-drop-limit #33298

Merged
merged 2 commits into from
Feb 13, 2025

Conversation

dainiusjocas
Copy link
Contributor

I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.

Implements #31619

Continuation of #29955

The idea is to iterate through all hits in the global phase after re-ranking and drop those with a lower score than the set threshold.

Or should drop in the global phase work like in the second phase: drop within the reranked hits?

@geirst geirst requested a review from arnej27959 February 11, 2025 15:38
@arnej27959
Copy link
Member

This looks good, but I'm not sure about the total-hit-count adjustment; if this feature is active you won't be able to get any further hits so maybe it should be clamped to the hits you actually keep.
Please run
mvn install -Dabicheck.writeSpec
in the container-search module to update the ABI spec (as you added some new public methods).

Copy link
Member

@arnej27959 arnej27959 left a comment

Choose a reason for hiding this comment

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

this looks good, we'll pull it in and I will make / extend a system test.

@arnej27959 arnej27959 merged commit 9306f72 into vespa-engine:master Feb 13, 2025
2 checks passed
@dainiusjocas
Copy link
Contributor Author

I didn't fully understand your point about the total-hit-count adjustment. Is it related to paging?

@arnej27959
Copy link
Member

paging: yes - if you get a high total-hit-count it's expected that you can repeat the query asking for more hits (or with some offset) to get more results, but when this limit triggers all the hits below will be dropped as well. I'll test it and see how it looks, it's probably OK as-is.

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