From 318dfba4644cf2b22abaa8b117e17fc956306b47 Mon Sep 17 00:00:00 2001 From: Britta Weber Date: Tue, 13 Oct 2015 13:05:14 +0200 Subject: [PATCH] use same score computation for actual scoring and explanation FiltersFunctionScoreQuery sums up scores and weights and scores as double but when we explain we cannot get the double scores from the explanation of score functions. as a result we cannot compute the exact score from the explanations of the functions alone. this commit makes the explanation more accurate but also causes the score to be computed one additional time. --- .../function/FiltersFunctionScoreQuery.java | 60 ++++--------------- .../functionscore/FunctionScoreTests.java | 21 +++++-- 2 files changed, 28 insertions(+), 53 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java b/core/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java index 5a2a9b96b53..314135db661 100644 --- a/core/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java +++ b/core/src/main/java/org/elasticsearch/common/lucene/search/function/FiltersFunctionScoreQuery.java @@ -207,19 +207,11 @@ public class FiltersFunctionScoreQuery extends Query { } // First: Gather explanations for all filters List filterExplanations = new ArrayList<>(); - float weightSum = 0; for (int i = 0; i < filterFunctions.length; ++i) { - FilterFunction filterFunction = filterFunctions[i]; - - if (filterFunction.function instanceof WeightFactorFunction) { - weightSum += ((WeightFactorFunction) filterFunction.function).getWeight(); - } else { - weightSum++; - } - Bits docSet = Lucene.asSequentialAccessBits(context.reader().maxDoc(), filterWeights[i].scorer(context)); if (docSet.get(doc)) { + FilterFunction filterFunction = filterFunctions[i]; Explanation functionExplanation = filterFunction.function.getLeafScoreFunction(context).explainScore(doc, subQueryExpl); double factor = functionExplanation.getValue(); float sc = CombineFunction.toFloat(factor); @@ -232,44 +224,11 @@ public class FiltersFunctionScoreQuery extends Query { return subQueryExpl; } - // Second: Compute the factor that would have been computed by the - // filters - double factor = 1.0; - switch (scoreMode) { - case FIRST: - factor = filterExplanations.get(0).getValue(); - break; - case MAX: - factor = Double.NEGATIVE_INFINITY; - for (Explanation filterExplanation : filterExplanations) { - factor = Math.max(filterExplanation.getValue(), factor); - } - break; - case MIN: - factor = Double.POSITIVE_INFINITY; - for (Explanation filterExplanation : filterExplanations) { - factor = Math.min(filterExplanation.getValue(), factor); - } - break; - case MULTIPLY: - for (Explanation filterExplanation : filterExplanations) { - factor *= filterExplanation.getValue(); - } - break; - default: - double totalFactor = 0.0f; - for (Explanation filterExplanation : filterExplanations) { - totalFactor += filterExplanation.getValue(); - } - if (weightSum != 0) { - factor = totalFactor; - if (scoreMode == ScoreMode.AVG) { - factor /= weightSum; - } - } - } + FiltersFunctionFactorScorer scorer = (FiltersFunctionFactorScorer)scorer(context); + scorer.advance(doc); + double score = scorer.computeScore(doc, subQueryExpl.getValue()); Explanation factorExplanation = Explanation.match( - CombineFunction.toFloat(factor), + CombineFunction.toFloat(score), "function score, score mode [" + scoreMode.toString().toLowerCase(Locale.ROOT) + "]", filterExplanations); return combineFunction.explain(subQueryExpl, factorExplanation, maxBoost); @@ -296,11 +255,16 @@ public class FiltersFunctionScoreQuery extends Query { @Override public float innerScore() throws IOException { int docId = scorer.docID(); - double factor = 1.0f; // Even if the weight is created with needsScores=false, it might // be costly to call score(), so we explicitly check if scores // are needed float subQueryScore = needsScores ? scorer.score() : 0f; + double factor = computeScore(docId, subQueryScore); + return scoreCombiner.combine(subQueryScore, factor, maxBoost); + } + + protected double computeScore(int docId, float subQueryScore) { + double factor = 1d; switch(scoreMode) { case FIRST: for (int i = 0; i < filterFunctions.length; i++) { @@ -360,7 +324,7 @@ public class FiltersFunctionScoreQuery extends Query { } break; } - return scoreCombiner.combine(subQueryScore, factor, maxBoost); + return factor; } } diff --git a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java index 398b818169a..16467613b1c 100644 --- a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreTests.java @@ -22,7 +22,6 @@ package org.elasticsearch.index.query.functionscore; import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; -import org.apache.lucene.document.FieldType; import org.apache.lucene.document.TextField; import org.apache.lucene.index.*; import org.apache.lucene.search.*; @@ -47,7 +46,6 @@ import java.io.IOException; import java.util.Collection; import java.util.concurrent.ExecutionException; -import static org.hamcrest.Matchers.closeTo; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsEqual.equalTo; @@ -374,9 +372,12 @@ public class FunctionScoreTests extends ESTestCase { public Explanation getFiltersFunctionScoreExplanation(IndexSearcher searcher, ScoreFunction... scoreFunctions) throws IOException { FiltersFunctionScoreQuery filtersFunctionScoreQuery = getFiltersFunctionScoreQuery(FiltersFunctionScoreQuery.ScoreMode.AVG, CombineFunction.AVG, scoreFunctions); + return getExplanation(searcher, filtersFunctionScoreQuery).getDetails()[1]; + } + + protected Explanation getExplanation(IndexSearcher searcher, FiltersFunctionScoreQuery filtersFunctionScoreQuery) throws IOException { Weight weight = searcher.createNormalizedWeight(filtersFunctionScoreQuery, true); - Explanation explanation = weight.explain(searcher.getIndexReader().leaves().get(0), 0); - return explanation.getDetails()[1]; + return weight.explain(searcher.getIndexReader().leaves().get(0), 0); } public FiltersFunctionScoreQuery getFiltersFunctionScoreQuery(FiltersFunctionScoreQuery.ScoreMode scoreMode, CombineFunction combineFunction, ScoreFunction... scoreFunctions) { @@ -430,7 +431,7 @@ public class FunctionScoreTests extends ESTestCase { @Override public Explanation explainScore(int docId, Explanation subQueryScore) throws IOException { - throw new UnsupportedOperationException(UNSUPPORTED); + return Explanation.match((float) score, "a random score for testing"); } }; } @@ -473,6 +474,8 @@ public class FunctionScoreTests extends ESTestCase { score *= weights[i] * scores[i]; } assertThat(scoreWithWeight / (float) score, is(1f)); + float explainedScore = getExplanation(searcher, filtersFunctionScoreQueryWithWeights).getValue(); + assertThat(explainedScore / scoreWithWeight, is(1f)); filtersFunctionScoreQueryWithWeights = getFiltersFunctionScoreQuery( FiltersFunctionScoreQuery.ScoreMode.SUM @@ -487,6 +490,8 @@ public class FunctionScoreTests extends ESTestCase { sum += weights[i] * scores[i]; } assertThat(scoreWithWeight / (float) sum, is(1f)); + explainedScore = getExplanation(searcher, filtersFunctionScoreQueryWithWeights).getValue(); + assertThat(explainedScore / scoreWithWeight, is(1f)); filtersFunctionScoreQueryWithWeights = getFiltersFunctionScoreQuery( FiltersFunctionScoreQuery.ScoreMode.AVG @@ -503,6 +508,8 @@ public class FunctionScoreTests extends ESTestCase { sum += weights[i] * scores[i]; } assertThat(scoreWithWeight / (float) (sum / norm), is(1f)); + explainedScore = getExplanation(searcher, filtersFunctionScoreQueryWithWeights).getValue(); + assertThat(explainedScore / scoreWithWeight, is(1f)); filtersFunctionScoreQueryWithWeights = getFiltersFunctionScoreQuery( FiltersFunctionScoreQuery.ScoreMode.MIN @@ -517,6 +524,8 @@ public class FunctionScoreTests extends ESTestCase { min = Math.min(min, weights[i] * scores[i]); } assertThat(scoreWithWeight / (float) min, is(1f)); + explainedScore = getExplanation(searcher, filtersFunctionScoreQueryWithWeights).getValue(); + assertThat(explainedScore / scoreWithWeight, is(1f)); filtersFunctionScoreQueryWithWeights = getFiltersFunctionScoreQuery( FiltersFunctionScoreQuery.ScoreMode.MAX @@ -531,6 +540,8 @@ public class FunctionScoreTests extends ESTestCase { max = Math.max(max, weights[i] * scores[i]); } assertThat(scoreWithWeight / (float) max, is(1f)); + explainedScore = getExplanation(searcher, filtersFunctionScoreQueryWithWeights).getValue(); + assertThat(explainedScore / scoreWithWeight, is(1f)); } @Test