From 315319b76325e6385f4c689329f875bf0a43ca6e Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Tue, 25 Jul 2017 15:07:45 +0200 Subject: [PATCH] Remove assertion about deviation when casting to a float. (#25806) We cannot guarantee that the result of computations will be in the float range, since it depends on the data and how scores are computed. We already use doubles as intermediate representations and cast to a float as a final step, which is the right thing to do. Small doubles will just be rounded to zero, there is not much we can or should do about it. Closes #25330 --- .../search/function/CombineFunction.java | 24 ++++++------------- .../function/FieldValueFactorFunction.java | 2 +- .../function/FiltersFunctionScoreQuery.java | 4 ++-- .../search/function/RandomScoreFunction.java | 2 +- .../search/function/ScriptScoreFunction.java | 2 +- .../functionscore/DecayFunctionBuilder.java | 2 +- 6 files changed, 13 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/lucene/search/function/CombineFunction.java b/core/src/main/java/org/elasticsearch/common/lucene/search/function/CombineFunction.java index 90d110c3804..399f3d7a2e6 100644 --- a/core/src/main/java/org/elasticsearch/common/lucene/search/function/CombineFunction.java +++ b/core/src/main/java/org/elasticsearch/common/lucene/search/function/CombineFunction.java @@ -31,7 +31,7 @@ public enum CombineFunction implements Writeable { MULTIPLY { @Override public float combine(double queryScore, double funcScore, double maxBoost) { - return toFloat(queryScore * Math.min(funcScore, maxBoost)); + return (float) (queryScore * Math.min(funcScore, maxBoost)); } @Override @@ -48,7 +48,7 @@ public enum CombineFunction implements Writeable { REPLACE { @Override public float combine(double queryScore, double funcScore, double maxBoost) { - return toFloat(Math.min(funcScore, maxBoost)); + return (float) (Math.min(funcScore, maxBoost)); } @Override @@ -64,7 +64,7 @@ public enum CombineFunction implements Writeable { SUM { @Override public float combine(double queryScore, double funcScore, double maxBoost) { - return toFloat(queryScore + Math.min(funcScore, maxBoost)); + return (float) (queryScore + Math.min(funcScore, maxBoost)); } @Override @@ -79,7 +79,7 @@ public enum CombineFunction implements Writeable { AVG { @Override public float combine(double queryScore, double funcScore, double maxBoost) { - return toFloat((Math.min(funcScore, maxBoost) + queryScore) / 2.0); + return (float) ((Math.min(funcScore, maxBoost) + queryScore) / 2.0); } @Override @@ -87,7 +87,7 @@ public enum CombineFunction implements Writeable { Explanation minExpl = Explanation.match(Math.min(funcExpl.getValue(), maxBoost), "min of:", funcExpl, Explanation.match(maxBoost, "maxBoost")); return Explanation.match( - toFloat((Math.min(funcExpl.getValue(), maxBoost) + queryExpl.getValue()) / 2.0), "avg of", + (float) ((Math.min(funcExpl.getValue(), maxBoost) + queryExpl.getValue()) / 2.0), "avg of", queryExpl, minExpl); } @@ -95,7 +95,7 @@ public enum CombineFunction implements Writeable { MIN { @Override public float combine(double queryScore, double funcScore, double maxBoost) { - return toFloat(Math.min(queryScore, Math.min(funcScore, maxBoost))); + return (float) (Math.min(queryScore, Math.min(funcScore, maxBoost))); } @Override @@ -112,7 +112,7 @@ public enum CombineFunction implements Writeable { MAX { @Override public float combine(double queryScore, double funcScore, double maxBoost) { - return toFloat(Math.max(queryScore, Math.min(funcScore, maxBoost))); + return (float) (Math.max(queryScore, Math.min(funcScore, maxBoost))); } @Override @@ -129,16 +129,6 @@ public enum CombineFunction implements Writeable { public abstract float combine(double queryScore, double funcScore, double maxBoost); - public static float toFloat(double input) { - assert deviation(input) <= 0.001 : "input " + input + " out of float scope for function score deviation: " + deviation(input); - return (float) input; - } - - private static double deviation(double input) { // only with assert! - float floatVersion = (float) input; - return Double.compare(floatVersion, input) == 0 || input == 0.0d ? 0 : 1.d - (floatVersion) / input; - } - public abstract Explanation explain(Explanation queryExpl, Explanation funcExpl, float maxBoost); @Override diff --git a/core/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java b/core/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java index 1978c0acf0d..ec1ae6392df 100644 --- a/core/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java +++ b/core/src/main/java/org/elasticsearch/common/lucene/search/function/FieldValueFactorFunction.java @@ -96,7 +96,7 @@ public class FieldValueFactorFunction extends ScoreFunction { String defaultStr = missing != null ? "?:" + missing : ""; double score = score(docId, subQueryScore.getValue()); return Explanation.match( - CombineFunction.toFloat(score), + (float) score, String.format(Locale.ROOT, "field value function: %s(doc['%s'].value%s * factor=%s)", modifierStr, field, defaultStr, boostFactor)); } 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 40465dc6ece..5c137d3f97c 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 @@ -206,7 +206,7 @@ public class FiltersFunctionScoreQuery extends Query { FilterFunction filterFunction = filterFunctions[i]; Explanation functionExplanation = filterFunction.function.getLeafScoreFunction(context).explainScore(doc, expl); double factor = functionExplanation.getValue(); - float sc = CombineFunction.toFloat(factor); + float sc = (float) factor; Explanation filterExplanation = Explanation.match(sc, "function score, product of:", Explanation.match(1.0f, "match filter: " + filterFunction.filter.toString()), functionExplanation); filterExplanations.add(filterExplanation); @@ -219,7 +219,7 @@ public class FiltersFunctionScoreQuery extends Query { Explanation factorExplanation; if (filterExplanations.size() > 0) { factorExplanation = Explanation.match( - CombineFunction.toFloat(score), + (float) score, "function score, score mode [" + scoreMode.toString().toLowerCase(Locale.ROOT) + "]", filterExplanations); diff --git a/core/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java b/core/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java index 25aea6abfa7..601363a9384 100644 --- a/core/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java +++ b/core/src/main/java/org/elasticsearch/common/lucene/search/function/RandomScoreFunction.java @@ -84,7 +84,7 @@ public class RandomScoreFunction extends ScoreFunction { public Explanation explainScore(int docId, Explanation subQueryScore) throws IOException { String field = fieldData == null ? null : fieldData.getFieldName(); return Explanation.match( - CombineFunction.toFloat(score(docId, subQueryScore.getValue())), + (float) score(docId, subQueryScore.getValue()), "random score function (seed: " + originalSeed + ", field: " + field + ")"); } }; diff --git a/core/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java b/core/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java index 1113ab9cd93..6780b2c3b36 100644 --- a/core/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java +++ b/core/src/main/java/org/elasticsearch/common/lucene/search/function/ScriptScoreFunction.java @@ -109,7 +109,7 @@ public class ScriptScoreFunction extends ScoreFunction { subQueryScore.getValue(), "_score: ", subQueryScore); return Explanation.match( - CombineFunction.toFloat(score), explanation, + (float) score, explanation, scoreExp); } return exp; diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java index e296c102188..dcd1399bf51 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/DecayFunctionBuilder.java @@ -543,7 +543,7 @@ public abstract class DecayFunctionBuilder return Explanation.noMatch("No value for the distance"); } return Explanation.match( - CombineFunction.toFloat(score(docId, subQueryScore.getValue())), + (float) score(docId, subQueryScore.getValue()), "Function for field " + getFieldName() + ":", func.explainFunction(getDistanceString(ctx, docId), distance.doubleValue(), scale)); }