Fix TestBooleanMinShouldMatch#testRandomQueries failure. (#14715)

This test generates random boolean queries and ensures that setting a minimum
number of matching SHOULD clauses returns a subset of the hits with the same
scores.

It already tries to work around accuracy loss due to arithmetic operations by
allowing a delta of up to one ulp between these two queries. However, sometimes
the delta can be higher.

For instance consider the following query that triggered the most recent test
failure: `(data:5 data:5 data:5 data:6 +data:6 data:Z data:X -data:1)~2`.
Without a minimum number of matching SHOULD clauses, it gets rewritten to
`(data:5^3 +data:6^2 data:Z data:X -data:1)`. So the score contribution of
`data:5` is computed as `(double) score(data:5) + (double) score(data:5) +
(double) score(data:5)` in one case, and `(double) (score(data:5: * 3f)`
(multiply first, then cast to a double) in the other case. The use of
`ReqOptSumScorer` also contributes accuracy losses as per existing comment, for
instance `data:6` is part of both the required and the optional clauses in the
first case, while it's only a required clauses (with a 2x boost) in the other
case. So accuracy loss accrues differently.

I don't think we should try too hard to avoid these accuracy losses, so I'm
instead increasing the leniency of the test.

Author: jpountz

lucene/core/src/test/org/apache/lucene/search/TestBooleanMinShouldMatch.java

@@ -25,6 +25,7 @@
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.StoredFields;
 import org.apache.lucene.index.Term;
+import org.apache.lucene.search.BooleanClause.Occur;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.tests.index.RandomIndexWriter;
 import org.apache.lucene.tests.search.CheckHits;
@@ -359,7 +360,7 @@ public void postCreate(BooleanQuery.Builder q) {
     // System.out.println("Total hits:"+tot);
   }
 
-  private void assertSubsetOfSameScores(Query q, TopDocs top1, TopDocs top2) {
+  private void assertSubsetOfSameScores(BooleanQuery q, TopDocs top1, TopDocs top2) {
     // The constrained query
     // should be a subset to the unconstrained query.
     if (top2.totalHits.value() > top1.totalHits.value()) {
@@ -371,6 +372,8 @@ private void assertSubsetOfSameScores(Query q, TopDocs top1, TopDocs top2) {
               + q.toString());
     }
 
+    int numScoringClauses = q.getClauses(Occur.SHOULD).size() + q.getClauses(Occur.MUST).size();
+
     for (int hit = 0; hit < top2.totalHits.value(); hit++) {
       int id = top2.scoreDocs[hit].doc;
       float score = top2.scoreDocs[hit].score;
@@ -391,14 +394,14 @@ private void assertSubsetOfSameScores(Query q, TopDocs top1, TopDocs top2) {
                   + q.toString(),
               score,
               otherScore,
-              // If there is at least one MUST/FILTER clause and if
-              // minShouldMatch is equal to the number of SHOULD clauses,
-              // then a query that was previously executed with
-              // ReqOptSumScorer is now executed with ConjunctionScorer.
-              // We need to introduce some leniency because ReqOptSumScorer
-              // casts intermediate values to floats before summing up again
-              // which hurts accuracy.
-              Math.ulp(score));
+              // While BooleanQuery tries to contain the accuracy loss of scores by summing up into
+              // doubles, there are a few places that do not do this, e.g. when (field:v field:v
+              // field:v) gets rewritten into field:v^3, the multiplication by 3 is performed on the
+              // float score, not on a double. Likewise, ReqOptSumScorer gets used in some cases,
+              // which casts the score of optional and required clauses into floats before summing
+              // them up together. So to avoid false positives, we allow losing one ulp of accuracy
+              // per scoring clause.
+              Math.ulp(score) * numScoringClauses);
         }
       }