Skip to content

Commit

Permalink
function_score: remove explanation of query score from functions
Browse files Browse the repository at this point in the history
The score is explained already, it should not be again explained per function.

Also, remove explanation from parameter list of ScoreFunction#explainScore()
and leave only the score.

This also removes ExplainableSearchScript which is not used anywhere and
was the only reason to have the Explanation in the parameter anyway.

closes #7245
  • Loading branch information
brwe authored and areek committed Sep 8, 2014
1 parent 41bc0cd commit 6e09bfe
Show file tree
Hide file tree
Showing 12 changed files with 106 additions and 62 deletions.
Expand Up @@ -49,7 +49,7 @@ public double score(int docId, float subQueryScore) {
}

@Override
public Explanation explainScore(int docId, Explanation subQueryExpl) {
public Explanation explainScore(int docId, float subQueryScore) {
Explanation exp = new Explanation(boost, "static boost factor");
exp.addDetail(new Explanation(boost, "boostFactor"));
return exp;
Expand Down
Expand Up @@ -70,14 +70,13 @@ public double score(int docId, float subQueryScore) {
}

@Override
public Explanation explainScore(int docId, Explanation subQueryExpl) {
public Explanation explainScore(int docId, float subQueryScore) {
Explanation exp = new Explanation();
String modifierStr = modifier != null ? modifier.toString() : "";
double score = score(docId, subQueryExpl.getValue());
double score = score(docId, subQueryScore);
exp.setValue(CombineFunction.toFloat(score));
exp.setDescription("field value function: " +
modifierStr + "(" + "doc['" + field + "'].value * factor=" + boostFactor + ")");
exp.addDetail(subQueryExpl);
return exp;
}

Expand Down
Expand Up @@ -180,7 +180,7 @@ public Explanation explain(AtomicReaderContext context, int doc) throws IOExcept
filterFunction.filter.getDocIdSet(context, context.reader().getLiveDocs()));
if (docSet.get(doc)) {
filterFunction.function.setNextReader(context);
Explanation functionExplanation = filterFunction.function.explainScore(doc, subQueryExpl);
Explanation functionExplanation = filterFunction.function.explainScore(doc, subQueryExpl.getValue());
double factor = functionExplanation.getValue();
float sc = CombineFunction.toFloat(factor);
ComplexExplanation filterExplanation = new ComplexExplanation(true, sc, "function score, product of:");
Expand Down
Expand Up @@ -131,7 +131,7 @@ public Explanation explain(AtomicReaderContext context, int doc) throws IOExcept
return subQueryExpl;
}
function.setNextReader(context);
Explanation functionExplanation = function.explainScore(doc, subQueryExpl);
Explanation functionExplanation = function.explainScore(doc, subQueryExpl.getValue());
return combineFunction.explain(getBoost(), subQueryExpl, functionExplanation, maxBoost);
}
}
Expand Down
Expand Up @@ -44,10 +44,9 @@ public double score(int docId, float subQueryScore) {
}

@Override
public Explanation explainScore(int docId, Explanation subQueryExpl) {
public Explanation explainScore(int docId, float subQueryScore) {
Explanation exp = new Explanation();
exp.setDescription("random score function (seed: " + prng.originalSeed + ")");
exp.addDetail(subQueryExpl);
return exp;
}

Expand Down
Expand Up @@ -33,7 +33,7 @@ public abstract class ScoreFunction {

public abstract double score(int docId, float subQueryScore);

public abstract Explanation explainScore(int docId, Explanation subQueryExpl);
public abstract Explanation explainScore(int docId, float subQueryScore);

public CombineFunction getDefaultScoreCombiner() {
return scoreCombiner;
Expand Down
Expand Up @@ -22,7 +22,6 @@
import org.apache.lucene.index.AtomicReaderContext;
import org.apache.lucene.search.Explanation;
import org.apache.lucene.search.Scorer;
import org.elasticsearch.script.ExplainableSearchScript;
import org.elasticsearch.script.SearchScript;

import java.io.IOException;
Expand Down Expand Up @@ -101,18 +100,14 @@ public double score(int docId, float subQueryScore) {
}

@Override
public Explanation explainScore(int docId, Explanation subQueryExpl) {
public Explanation explainScore(int docId, float subQueryScore) {
Explanation exp;
if (script instanceof ExplainableSearchScript) {
script.setNextDocId(docId);
scorer.docid = docId;
scorer.score = subQueryExpl.getValue();
exp = ((ExplainableSearchScript) script).explain(subQueryExpl);
} else {
double score = score(docId, subQueryExpl.getValue());
exp = new Explanation(CombineFunction.toFloat(score), "script score function: composed of:");
exp.addDetail(subQueryExpl);
double score = score(docId, subQueryScore);
String explanation = "script score function, computed with script:\"" + sScript;
if (params != null) {
explanation += "\" and parameters: \n" + params.toString();
}
exp = new Explanation(CombineFunction.toFloat(score), explanation);
return exp;
}

Expand Down
Expand Up @@ -455,9 +455,9 @@ public double score(int docId, float subQueryScore) {
protected abstract String getFieldName();

@Override
public Explanation explainScore(int docId, Explanation subQueryExpl) {
public Explanation explainScore(int docId, float subQueryScore) {
ComplexExplanation ce = new ComplexExplanation();
ce.setValue(CombineFunction.toFloat(score(docId, subQueryExpl.getValue())));
ce.setValue(CombineFunction.toFloat(score(docId, subQueryScore)));
ce.setMatch(true);
ce.setDescription("Function for field " + getFieldName() + ":");
ce.addDetail(func.explainFunction(getDistanceString(docId), distance(docId), scale));
Expand Down

This file was deleted.

2 changes: 0 additions & 2 deletions src/main/java/org/elasticsearch/script/SearchScript.java
Expand Up @@ -27,8 +27,6 @@

/**
* A search script.
*
* @see {@link ExplainableSearchScript} for script which can explain a score
*/
public interface SearchScript extends ExecutableScript, ReaderContextAware, ScorerAware {

Expand Down
Expand Up @@ -183,11 +183,10 @@ public void testExplainScoreReportsOriginalSeed() {
function.score(0, 1.0f);

// Generate the randomScore explanation
Explanation randomExplanation = function.explainScore(0, subExplanation);
Explanation randomExplanation = function.explainScore(0, subExplanation.getValue());

// Original seed should be there
assertThat(randomExplanation.getDescription(), containsString("" + seed));
assertThat(randomExplanation.getDetails(), arrayContaining(subExplanation));
}


Expand Down
@@ -0,0 +1,90 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.search.functionscore;

import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchType;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.junit.Test;

import java.io.IOException;
import java.util.concurrent.ExecutionException;

import static org.elasticsearch.client.Requests.searchRequest;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.index.query.QueryBuilders.functionScoreQuery;
import static org.elasticsearch.index.query.QueryBuilders.termQuery;
import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.*;
import static org.elasticsearch.search.builder.SearchSourceBuilder.searchSource;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;

public class FunctionScoreTests extends ElasticsearchIntegrationTest {

@Test
public void testExplainQueryOnlyOnce() throws IOException, ExecutionException, InterruptedException {
assertAcked(prepareCreate("test").addMapping(
"type1",
jsonBuilder().startObject().startObject("type1").startObject("properties").startObject("test").field("type", "string")
.endObject().startObject("num").field("type", "float").endObject().endObject().endObject().endObject()));
ensureYellow();

client().prepareIndex()
.setType("type1")
.setId("1")
.setIndex("test")
.setSource(
jsonBuilder().startObject().field("test", "value").field("num", 10).endObject()).get();
refresh();

SearchResponse response = client().search(
searchRequest().searchType(SearchType.QUERY_THEN_FETCH).source(
searchSource().explain(true).query(
functionScoreQuery(termQuery("test", "value")).add(gaussDecayFunction("num", 5, 5)).add(exponentialDecayFunction("num", 5, 5)).add(linearDecayFunction("num", 5, 5))))).get();
String explanation = response.getHits().getAt(0).explanation().toString();

checkQueryExplanationAppearsOnlyOnce(explanation);
response = client().search(
searchRequest().searchType(SearchType.QUERY_THEN_FETCH).source(
searchSource().explain(true).query(
functionScoreQuery(termQuery("test", "value")).add(fieldValueFactorFunction("num"))))).get();
explanation = response.getHits().getAt(0).explanation().toString();
checkQueryExplanationAppearsOnlyOnce(explanation);

response = client().search(
searchRequest().searchType(SearchType.QUERY_THEN_FETCH).source(
searchSource().explain(true).query(
functionScoreQuery(termQuery("test", "value")).add(randomFunction(10))))).get();
explanation = response.getHits().getAt(0).explanation().toString();

checkQueryExplanationAppearsOnlyOnce(explanation);
}

private void checkQueryExplanationAppearsOnlyOnce(String explanation) {
// use some substring of the query explanation and see if it appears twice
String queryExplanation = "idf(docFreq=1, maxDocs=1)";
int queryExplanationIndex = explanation.indexOf(queryExplanation, 0);
assertThat(queryExplanationIndex, greaterThan(-1));
queryExplanationIndex = explanation.indexOf(queryExplanation, queryExplanationIndex + 1);
assertThat(queryExplanationIndex, equalTo(-1));
}

}

0 comments on commit 6e09bfe

Please sign in to comment.