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

Improve explanation in rescore #30629

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
"Score should match explanation in rescore":
- skip:
version: " - 6.99.99"
reason: Explanation for rescoring was corrected after these versions
- do:
bulk:
refresh: true
body:
- '{"index": {"_index": "test_index", "_type": "_doc", "_id": "1"}}'
- '{"f1": "1"}'
- '{"index": {"_index": "test_index", "_type": "_doc", "_id": "2"}}'
- '{"f1": "2"}'
- '{"index": {"_index": "test_index", "_type": "_doc", "_id": "3"}}'
- '{"f1": "3"}'

- do:
search:
index: test_index
body:
explain: true
query:
match_all: {}
rescore:
window_size: 2
query:
rescore_query:
match_all: {}
query_weight: 5
rescore_query_weight: 10

- match: { hits.hits.0._score: 15 }
- match: { hits.hits.0._explanation.value: 15 }

- match: { hits.hits.1._score: 15 }
- match: { hits.hits.1._explanation.value: 15 }

- match: { hits.hits.2._score: 5 }
- match: { hits.hits.2._explanation.value: 5 }
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import java.util.Arrays;
import java.util.Comparator;
import java.util.Set;
import java.util.Collections;
import static java.util.stream.Collectors.toSet;

public final class QueryRescorer implements Rescorer {

Expand Down Expand Up @@ -61,6 +63,11 @@ protected float combine(float firstPassScore, boolean secondPassMatches, float s
// First take top slice of incoming docs, to be rescored:
TopDocs topNFirstPass = topN(topDocs, rescoreContext.getWindowSize());

// Save doc IDs for which rescoring was applied to be used in score explanation
Set<Integer> topNDocIDs = Collections.unmodifiableSet(
Arrays.stream(topNFirstPass.scoreDocs).map(scoreDoc -> scoreDoc.doc).collect(toSet()));
rescoreContext.setRescoredDocs(topNDocIDs);

// Rescore them:
TopDocs rescored = rescorer.rescore(searcher, topNFirstPass, rescoreContext.getWindowSize());

Expand All @@ -76,8 +83,6 @@ public Explanation explain(int topLevelDocId, IndexSearcher searcher, RescoreCon
// this should not happen but just in case
return Explanation.noMatch("nothing matched");
}
// TODO: this isn't right? I.e., we are incorrectly pretending all first pass hits were rescored? If the requested docID was
// beyond the top rescoreContext.window() in the first pass hits, we don't rescore it now?
Explanation rescoreExplain = searcher.explain(rescore.query(), topLevelDocId);
float primaryWeight = rescore.queryWeight();

Expand All @@ -92,7 +97,7 @@ public Explanation explain(int topLevelDocId, IndexSearcher searcher, RescoreCon

// NOTE: we don't use Lucene's Rescorer.explain because we want to insert our own description with which ScoreMode was used. Maybe
// we should add QueryRescorer.explainCombine to Lucene?
if (rescoreExplain != null && rescoreExplain.isMatch()) {
if (rescoreContext.isRescored(topLevelDocId) && rescoreExplain != null && rescoreExplain.isMatch()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We always build the rescore explanation even when rescoreContext.isRescored(topLevelDocId) is false. We could avoid this by building the primary explanation first and return it directly if rescoreContext.isRescored(topLevelDocId) is false ?

float secondaryWeight = rescore.rescoreQueryWeight();
Explanation sec = Explanation.match(
rescoreExplain.getValue() * secondaryWeight,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

package org.elasticsearch.search.rescore;

import java.util.Set;

/**
* Context available to the rescore while it is running. Rescore
* implementations should extend this with any additional resources that
Expand All @@ -27,6 +29,7 @@
public class RescoreContext {
private final int windowSize;
private final Rescorer rescorer;
private Set<Integer> recroredDocs; //doc Ids for which rescoring was applied
Copy link
Contributor

Choose a reason for hiding this comment

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

s/recrored/rescored/


/**
* Build the context.
Expand All @@ -50,4 +53,12 @@ public Rescorer rescorer() {
public int getWindowSize() {
return windowSize;
}

public void setRescoredDocs(Set<Integer> docIds) {
recroredDocs = docIds;
}

public boolean isRescored(int docId) {
return recroredDocs.contains(docId);
}
}