From fb24d451cc98120d5a70bd0e63a5b296ce69123c Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 10 May 2017 08:29:55 +0100 Subject: [PATCH] LUCENE-7741: Add explain() to DoubleValuesSource --- lucene/CHANGES.txt | 3 + .../lucene/search/DoubleValuesSource.java | 47 +++++++++++++- .../lucene/search/TestDoubleValuesSource.java | 63 +++++++++++++++++++ .../expressions/ExpressionRescorer.java | 13 +--- .../expressions/ExpressionValueSource.java | 15 +++++ .../expressions/TestDemoExpressions.java | 32 +--------- .../expressions/TestExpressionRescorer.java | 2 +- .../facet/range/TestRangeFacetCounts.java | 5 ++ .../queries/function/FunctionScoreQuery.java | 11 ++-- .../lucene/queries/function/ValueSource.java | 11 ++++ .../TestFunctionScoreExplanations.java | 2 +- .../function/TestFunctionScoreQuery.java | 4 +- 12 files changed, 154 insertions(+), 54 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 54cc54a2891..b92b3b9e90a 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -50,6 +50,9 @@ API Changes * LUCENE-7701: Grouping collectors have been refactored, such that groups are now defined by a GroupSelector implementation. (Alan Woodward) +* LUCENE-7741: DoubleValuesSource now has an explain() method (Alan Woodward, + Adrien Grand) + Bug Fixes * LUCENE-7626: IndexWriter will no longer accept broken token offsets diff --git a/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java b/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java index c22a3c37d23..929441d3c52 100644 --- a/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java +++ b/lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java @@ -58,6 +58,16 @@ public abstract class DoubleValuesSource { */ public abstract boolean needsScores(); + /** + * An explanation of the value for the named document. + * + * @param ctx the readers context to create the {@link Explanation} for. + * @param docId the document's id relative to the given context's reader + * @return an Explanation for the value + * @throws IOException if an {@link IOException} occurs + */ + public abstract Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException; + /** * Create a sort field based on the value of this producer * @param reverse true if the sort should be decreasing @@ -149,6 +159,11 @@ public abstract class DoubleValuesSource { public boolean needsScores() { return true; } + + @Override + public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) { + return scoreExplanation; + } }; /** @@ -176,6 +191,11 @@ public abstract class DoubleValuesSource { return false; } + @Override + public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) { + return Explanation.match((float) value, "constant(" + value + ")"); + } + @Override public String toString() { return "constant(" + value + ")"; @@ -186,7 +206,7 @@ public abstract class DoubleValuesSource { /** * Creates a DoubleValuesSource that is a function of another DoubleValuesSource */ - public static DoubleValuesSource function(DoubleValuesSource in, DoubleUnaryOperator function) { + public static DoubleValuesSource function(DoubleValuesSource in, String description, DoubleUnaryOperator function) { return new DoubleValuesSource() { @Override public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException { @@ -208,15 +228,22 @@ public abstract class DoubleValuesSource { public boolean needsScores() { return in.needsScores(); } + + @Override + public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException { + Explanation inner = in.explain(ctx, docId, scoreExplanation); + return Explanation.match((float) function.applyAsDouble(inner.getValue()), description + ", computed from:", inner, scoreExplanation); + } }; } /** * Creates a DoubleValuesSource that is a function of another DoubleValuesSource and a score * @param in the DoubleValuesSource to use as an input + * @param description a description of the function * @param function a function of the form (source, score) == result */ - public static DoubleValuesSource scoringFunction(DoubleValuesSource in, ToDoubleBiFunction function) { + public static DoubleValuesSource scoringFunction(DoubleValuesSource in, String description, ToDoubleBiFunction function) { return new DoubleValuesSource() { @Override public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException { @@ -238,6 +265,13 @@ public abstract class DoubleValuesSource { public boolean needsScores() { return true; } + + @Override + public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException { + Explanation inner = in.explain(ctx, docId, scoreExplanation); + return Explanation.match((float) function.applyAsDouble((double)inner.getValue(), (double)scoreExplanation.getValue()), + description + ", computed from:", inner, scoreExplanation); + } }; } @@ -303,6 +337,15 @@ public abstract class DoubleValuesSource { public boolean needsScores() { return false; } + + @Override + public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException { + DoubleValues values = getValues(ctx, null); + if (values.advanceExact(docId)) + return Explanation.match((float)values.doubleValue(), "double(" + field + ")"); + else + return Explanation.noMatch("double(" + field + ")"); + } } private static class DoubleValuesSortField extends SortField { diff --git a/lucene/core/src/test/org/apache/lucene/search/TestDoubleValuesSource.java b/lucene/core/src/test/org/apache/lucene/search/TestDoubleValuesSource.java index 13a5168c924..54501e699de 100644 --- a/lucene/core/src/test/org/apache/lucene/search/TestDoubleValuesSource.java +++ b/lucene/core/src/test/org/apache/lucene/search/TestDoubleValuesSource.java @@ -17,6 +17,7 @@ package org.apache.lucene.search; +import java.io.IOException; import java.util.Arrays; import java.util.Collections; @@ -26,6 +27,7 @@ import org.apache.lucene.document.Field; import org.apache.lucene.document.FloatDocValuesField; import org.apache.lucene.document.NumericDocValuesField; import org.apache.lucene.index.IndexReader; +import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.RandomIndexWriter; import org.apache.lucene.index.Term; import org.apache.lucene.store.Directory; @@ -164,4 +166,65 @@ public class TestDoubleValuesSource extends LuceneTestCase { CheckHits.checkEqual(query, expected.scoreDocs, actual.scoreDocs); } } + + static final Query[] testQueries = new Query[]{ + new MatchAllDocsQuery(), + new TermQuery(new Term("oddeven", "odd")), + new BooleanQuery.Builder() + .add(new TermQuery(new Term("english", "one")), BooleanClause.Occur.MUST) + .add(new TermQuery(new Term("english", "two")), BooleanClause.Occur.MUST) + .build() + }; + + public void testExplanations() throws Exception { + for (Query q : testQueries) { + testExplanations(q, DoubleValuesSource.fromIntField("int")); + testExplanations(q, DoubleValuesSource.fromLongField("long")); + testExplanations(q, DoubleValuesSource.fromFloatField("float")); + testExplanations(q, DoubleValuesSource.fromDoubleField("double")); + testExplanations(q, DoubleValuesSource.fromDoubleField("onefield")); + testExplanations(q, DoubleValuesSource.constant(5.45)); + testExplanations(q, DoubleValuesSource.function( + DoubleValuesSource.fromDoubleField("double"), "v * 4 + 73", + v -> v * 4 + 73 + )); + testExplanations(q, DoubleValuesSource.scoringFunction( + DoubleValuesSource.fromDoubleField("double"), "v * score", (v, s) -> v * s + )); + } + } + + private void testExplanations(Query q, DoubleValuesSource vs) throws IOException { + searcher.search(q, new SimpleCollector() { + + DoubleValues v; + LeafReaderContext ctx; + + @Override + protected void doSetNextReader(LeafReaderContext context) throws IOException { + this.ctx = context; + } + + @Override + public void setScorer(Scorer scorer) throws IOException { + this.v = vs.getValues(this.ctx, DoubleValuesSource.fromScorer(scorer)); + } + + @Override + public void collect(int doc) throws IOException { + Explanation scoreExpl = searcher.explain(q, ctx.docBase + doc); + if (this.v.advanceExact(doc)) { + CheckHits.verifyExplanation("", doc, (float) v.doubleValue(), true, vs.explain(ctx, doc, scoreExpl)); + } + else { + assertFalse(vs.explain(ctx, doc, scoreExpl).isMatch()); + } + } + + @Override + public boolean needsScores() { + return vs.needsScores(); + } + }); + } } diff --git a/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionRescorer.java b/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionRescorer.java index e3e7a4eb828..1b52e4aa52b 100644 --- a/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionRescorer.java +++ b/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionRescorer.java @@ -18,8 +18,6 @@ package org.apache.lucene.expressions; import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import org.apache.lucene.index.LeafReaderContext; @@ -79,15 +77,6 @@ class ExpressionRescorer extends SortRescorer { LeafReaderContext readerContext = leaves.get(subReader); int docIDInSegment = docID - readerContext.docBase; - DoubleValues scores = scores(docIDInSegment, firstPassExplanation.getValue()); - - List subs = new ArrayList<>(Arrays.asList(superExpl.getDetails())); - for(String variable : expression.variables) { - DoubleValues dv = bindings.getDoubleValuesSource(variable).getValues(readerContext, scores); - if (dv.advanceExact(docIDInSegment)) - subs.add(Explanation.match((float) dv.doubleValue(), "variable \"" + variable + "\"")); - } - - return Explanation.match(superExpl.getValue(), superExpl.getDescription(), subs); + return expression.getDoubleValuesSource(bindings).explain(readerContext, docIDInSegment, superExpl); } } diff --git a/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionValueSource.java b/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionValueSource.java index 7842de9e9d2..0fb8d05bf6e 100644 --- a/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionValueSource.java +++ b/lucene/expressions/src/java/org/apache/lucene/expressions/ExpressionValueSource.java @@ -25,6 +25,7 @@ import java.util.Objects; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.DoubleValues; import org.apache.lucene.search.DoubleValuesSource; +import org.apache.lucene.search.Explanation; /** * A {@link DoubleValuesSource} which evaluates a {@link Expression} given the context of an {@link Bindings}. @@ -137,4 +138,18 @@ final class ExpressionValueSource extends DoubleValuesSource { public boolean needsScores() { return needsScores; } + + @Override + public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException { + Explanation[] explanations = new Explanation[variables.length]; + DoubleValues dv = getValues(ctx, DoubleValuesSource.constant(scoreExplanation.getValue()).getValues(ctx, null)); + if (dv.advanceExact(docId) == false) { + return Explanation.noMatch(expression.sourceText); + } + int i = 0; + for (DoubleValuesSource var : variables) { + explanations[i++] = var.explain(ctx, docId, scoreExplanation); + } + return Explanation.match((float)dv.doubleValue(), expression.sourceText + ", computed from:", explanations); + } } diff --git a/lucene/expressions/src/test/org/apache/lucene/expressions/TestDemoExpressions.java b/lucene/expressions/src/test/org/apache/lucene/expressions/TestDemoExpressions.java index d76ef1c1bdd..f597626c407 100644 --- a/lucene/expressions/src/test/org/apache/lucene/expressions/TestDemoExpressions.java +++ b/lucene/expressions/src/test/org/apache/lucene/expressions/TestDemoExpressions.java @@ -16,19 +16,15 @@ */ package org.apache.lucene.expressions; -import java.io.IOException; - import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; import org.apache.lucene.document.NumericDocValuesField; import org.apache.lucene.expressions.js.JavascriptCompiler; import org.apache.lucene.expressions.js.VariableContext; import org.apache.lucene.index.DirectoryReader; -import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.RandomIndexWriter; import org.apache.lucene.index.Term; import org.apache.lucene.search.CheckHits; -import org.apache.lucene.search.DoubleValues; import org.apache.lucene.search.DoubleValuesSource; import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.IndexSearcher; @@ -252,30 +248,6 @@ public class TestDemoExpressions extends LuceneTestCase { assertEquals(2D, (Double)d.fields[0], 1E-4); } - private static DoubleValuesSource constant(double value) { - return new DoubleValuesSource() { - @Override - public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException { - return new DoubleValues() { - @Override - public double doubleValue() throws IOException { - return value; - } - - @Override - public boolean advanceExact(int doc) throws IOException { - return true; - } - }; - } - - @Override - public boolean needsScores() { - return false; - } - }; - } - public void testDynamicExtendedVariableExample() throws Exception { Expression popularity = JavascriptCompiler.compile("doc['popularity'].value + magicarray[0] + fourtytwo"); @@ -301,12 +273,12 @@ public class TestDemoExpressions extends LuceneTestCase { } } else if (base.equals("magicarray")) { if (var.length > 1 && var[1].type == INT_INDEX) { - return constant(2048); + return DoubleValuesSource.constant(2048); } else { fail();// error case, magic array isn't an array } } else if (base.equals("fourtytwo")) { - return constant(42); + return DoubleValuesSource.constant(42); } else { fail();// error case (variable doesn't exist) } diff --git a/lucene/expressions/src/test/org/apache/lucene/expressions/TestExpressionRescorer.java b/lucene/expressions/src/test/org/apache/lucene/expressions/TestExpressionRescorer.java index f60d45b7d94..98012a5e8b2 100644 --- a/lucene/expressions/src/test/org/apache/lucene/expressions/TestExpressionRescorer.java +++ b/lucene/expressions/src/test/org/apache/lucene/expressions/TestExpressionRescorer.java @@ -111,7 +111,7 @@ public class TestExpressionRescorer extends LuceneTestCase { // Confirm the explanation breaks out the individual // variables: - assertTrue(expl.contains("= variable \"popularity\"")); + assertTrue(expl.contains("= double(popularity)")); // Confirm the explanation includes first pass details: assertTrue(expl.contains("= first pass score")); diff --git a/lucene/facet/src/test/org/apache/lucene/facet/range/TestRangeFacetCounts.java b/lucene/facet/src/test/org/apache/lucene/facet/range/TestRangeFacetCounts.java index 31f9e59a648..06bd2e562e1 100644 --- a/lucene/facet/src/test/org/apache/lucene/facet/range/TestRangeFacetCounts.java +++ b/lucene/facet/src/test/org/apache/lucene/facet/range/TestRangeFacetCounts.java @@ -741,6 +741,11 @@ public class TestRangeFacetCounts extends FacetTestCase { public boolean needsScores() { return false; } + + @Override + public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException { + return Explanation.match(docId + 1, ""); + } }; FacetsConfig config = new FacetsConfig(); diff --git a/lucene/queries/src/java/org/apache/lucene/queries/function/FunctionScoreQuery.java b/lucene/queries/src/java/org/apache/lucene/queries/function/FunctionScoreQuery.java index 29ef41f60ba..63a98de8637 100644 --- a/lucene/queries/src/java/org/apache/lucene/queries/function/FunctionScoreQuery.java +++ b/lucene/queries/src/java/org/apache/lucene/queries/function/FunctionScoreQuery.java @@ -113,13 +113,12 @@ public final class FunctionScoreQuery extends Query { Scorer scorer = inner.scorer(context); if (scorer.iterator().advance(doc) != doc) return Explanation.noMatch("No match"); - DoubleValues scores = valueSource.getValues(context, DoubleValuesSource.fromScorer(scorer)); - scores.advanceExact(doc); - Explanation scoreExpl = scoreExplanation(context, doc, scores); + Explanation scoreExplanation = inner.explain(context, doc); + Explanation expl = valueSource.explain(context, doc, scoreExplanation); if (boost == 1f) - return scoreExpl; - return Explanation.match(scoreExpl.getValue() * boost, "product of:", - Explanation.match(boost, "boost"), scoreExpl); + return expl; + return Explanation.match(expl.getValue() * boost, "product of:", + Explanation.match(boost, "boost"), expl); } private Explanation scoreExplanation(LeafReaderContext context, int doc, DoubleValues scores) throws IOException { diff --git a/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java b/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java index 6bf29267110..e49c8233f3c 100644 --- a/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java +++ b/lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java @@ -25,6 +25,7 @@ import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.search.DocIdSetIterator; import org.apache.lucene.search.DoubleValues; import org.apache.lucene.search.DoubleValuesSource; +import org.apache.lucene.search.Explanation; import org.apache.lucene.search.FieldComparator; import org.apache.lucene.search.FieldComparatorSource; import org.apache.lucene.search.IndexSearcher; @@ -187,6 +188,16 @@ public abstract class ValueSource { public boolean needsScores() { return true; // be on the safe side } + + @Override + public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException { + Map context = new HashMap<>(); + FakeScorer scorer = new FakeScorer(); + scorer.score = scoreExplanation.getValue(); + context.put("scorer", scorer); + FunctionValues fv = ValueSource.this.getValues(context, ctx); + return fv.explain(docId); + } }; } diff --git a/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreExplanations.java b/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreExplanations.java index 5c643961060..bc8dbc113a7 100644 --- a/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreExplanations.java +++ b/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreExplanations.java @@ -59,7 +59,7 @@ public class TestFunctionScoreExplanations extends BaseExplanationTestCase { public void testExplanationsIncludingScore() throws Exception { - DoubleValuesSource scores = DoubleValuesSource.function(DoubleValuesSource.SCORES, v -> v * 2); + DoubleValuesSource scores = DoubleValuesSource.function(DoubleValuesSource.SCORES, "v * 2", v -> v * 2); Query q = new TermQuery(new Term(FIELD, "w1")); FunctionScoreQuery csq = new FunctionScoreQuery(q, scores); diff --git a/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreQuery.java b/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreQuery.java index 8f6ef8e9c9e..de5ad8812ea 100644 --- a/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreQuery.java +++ b/lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreQuery.java @@ -70,7 +70,7 @@ public class TestFunctionScoreQuery extends FunctionTestSetup { public void testScoreModifyingSource() throws Exception { DoubleValuesSource iii = DoubleValuesSource.fromIntField("iii"); - DoubleValuesSource score = DoubleValuesSource.scoringFunction(iii, (v, s) -> v * s); + DoubleValuesSource score = DoubleValuesSource.scoringFunction(iii, "v * s", (v, s) -> v * s); BooleanQuery bq = new BooleanQuery.Builder() .add(new TermQuery(new Term(TEXT_FIELD, "first")), BooleanClause.Occur.SHOULD) @@ -96,7 +96,7 @@ public class TestFunctionScoreQuery extends FunctionTestSetup { public void testBoostsAreAppliedLast() throws Exception { DoubleValuesSource scores - = DoubleValuesSource.function(DoubleValuesSource.SCORES, v -> Math.log(v + 4)); + = DoubleValuesSource.function(DoubleValuesSource.SCORES, "ln(v + 4)", v -> Math.log(v + 4)); Query q1 = new FunctionScoreQuery(new TermQuery(new Term(TEXT_FIELD, "text")), scores); TopDocs plain = searcher.search(q1, 5);