Skip to content

Commit

Permalink
Remove assertion about deviation when casting to a float. (#25806)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jpountz authored Jul 25, 2017
1 parent 3759aad commit 315319b
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -48,7 +48,7 @@ public Explanation explain(Explanation queryExpl, Explanation funcExpl, float ma
REPLACE {
@Override
public float combine(double queryScore, double funcScore, double maxBoost) {
return toFloat(Math.min(funcScore, maxBoost));
return (float) (Math.min(funcScore, maxBoost));
}

@Override
Expand All @@ -64,7 +64,7 @@ public Explanation explain(Explanation queryExpl, Explanation funcExpl, float ma
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
Expand All @@ -79,23 +79,23 @@ public Explanation explain(Explanation queryExpl, Explanation funcExpl, float ma
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
public Explanation explain(Explanation queryExpl, Explanation funcExpl, float maxBoost) {
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);
}

},
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
Expand All @@ -112,7 +112,7 @@ public Explanation explain(Explanation queryExpl, Explanation funcExpl, float ma
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
Expand All @@ -129,16 +129,6 @@ public Explanation explain(Explanation queryExpl, Explanation funcExpl, float ma

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public Explanation explainScore(int docId, Explanation subQueryScore) throws IOE
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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio
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);
Expand All @@ -219,7 +219,7 @@ public Explanation explain(LeafReaderContext context, int doc) throws IOExceptio
Explanation factorExplanation;
if (filterExplanations.size() > 0) {
factorExplanation = Explanation.match(
CombineFunction.toFloat(score),
(float) score,
"function score, score mode [" + scoreMode.toString().toLowerCase(Locale.ROOT) + "]",
filterExplanations);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public double score(int docId, float subQueryScore) throws IOException {
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 + ")");
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public Explanation explainScore(int docId, Explanation subQueryScore) throws IOE
subQueryScore.getValue(), "_score: ",
subQueryScore);
return Explanation.match(
CombineFunction.toFloat(score), explanation,
(float) score, explanation,
scoreExp);
}
return exp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ public Explanation explainScore(int docId, Explanation subQueryScore) throws IOE
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));
}
Expand Down

0 comments on commit 315319b

Please sign in to comment.