Skip to content

Conversation

carlosdelest
Copy link
Member

@carlosdelest carlosdelest commented Dec 18, 2024

Following up on #116663 and #118774

#118774 makes k to be always specified, defaulting to size in case it's null.

This allows to perform some simplifications on rescoring, as we'll always have k to limit the results returned per shard.

@carlosdelest carlosdelest added >non-issue auto-backport Automatically create backport pull requests when merged v8.9.0 v8.18.0 labels Dec 18, 2024
* original vector values stored in the index
*/
public class VectorSimilarityFloatValueSource extends DoubleValuesSource implements QueryProfilerProvider {
public class VectorSimilarityFloatValueSource extends DoubleValuesSource {
Copy link
Member Author

Choose a reason for hiding this comment

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

Profiling can be simplified, as we always know the number of results to return


// Retrieve top k documents from the rescored query
TopDocs topDocs = searcher.search(query, k);
vectorOperations = topDocs.totalHits.value();
Copy link
Member Author

Choose a reason for hiding this comment

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

We know in advance the number of comparisons done

}

@ParametersFactory
public static Iterable<Object[]> parameters() {
Copy link
Member Author

Choose a reason for hiding this comment

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

We always have a specific k, it makes no sense to use parameters.

@carlosdelest
Copy link
Member Author

@elasticmachine update branch

@carlosdelest carlosdelest marked this pull request as ready for review December 19, 2024 08:46
@elasticsearchmachine elasticsearchmachine added the needs:triage Requires assignment of a team area label label Dec 19, 2024
@carlosdelest
Copy link
Member Author

@elasticmachine update branch

@carlosdelest carlosdelest added :Search Relevance/Vectors Vector search Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch labels Jan 3, 2025
@elasticsearchmachine elasticsearchmachine removed the needs:triage Requires assignment of a team area label label Jan 3, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@carlosdelest carlosdelest changed the title Vector rescoring - Use request size when k is null Vector rescoring - Simplify code for k == null Jan 8, 2025
@benwtrent benwtrent removed the v8.9.0 label Jan 8, 2025
Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

Good cleanup. Also, I removed the v8.9 tag, I think you added that by accident.

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

Actually, one of the things I thought we should do is switch to use k as the oversampling and not num candidates. That was one of the original reasons for the overall change of eagerly setting k to request size.

@carlosdelest
Copy link
Member Author

carlosdelest commented Jan 9, 2025

Actually, one of the things I thought we should do is switch to use k as the oversampling and not num candidates. That was one of the original reasons for the overall change of eagerly setting k to request size.

Oh yeah, it all started with this conversation.

I can work on that as a follow up - and get back the original design:

{
    "query": {
        "knn": {
            "field": "emb",
            "query_vector": [...],
            "k": 10,
            "num_candidates": 100,
            "rescore_vector": {
                "oversample": 2.0
            }
        }
    }
}

This will mean rescoring k * oversample from the num_candidates retrieved on each shard, and returning the top k out of them.

I'll merge this and start working on it. Draft PR: #119835

@carlosdelest carlosdelest merged commit 0cf2ebb into elastic:main Jan 9, 2025
16 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 118997

@carlosdelest
Copy link
Member Author

💚 All backports created successfully

Status Branch Result
8.x

Questions ?

Please refer to the Backport tool documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged backport pending >non-issue :Search Relevance/Vectors Vector search Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v8.18.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants