jpountz commented on code in PR #14827:
URL: https://github.com/apache/lucene/pull/14827#discussion_r2178335968
##########
lucene/CHANGES.txt:
##########
@@ -194,6 +194,8 @@ Optimizations
* GITHUB#14854: Using BatchScoreBulkScorer on CombinedFieldQuery. (Ge Song)
+* GITHUB#14827: Specialize filterCompetitiveHits when have exact 2 clauses.
(Ge Sone)
Review Comment:
Let's update changes since it now applies to any number of clauses?
##########
lucene/core/src/java/org/apache/lucene/search/ScorerUtil.java:
##########
@@ -128,15 +128,22 @@ static void filterCompetitiveHits(
double maxRemainingScore,
float minCompetitiveScore,
int numScorers) {
- if ((float) MathUtil.sumUpperBound(maxRemainingScore, numScorers) >=
minCompetitiveScore) {
+ // Compute minRequiredScore as the greatest float value so that (float)
+ // MathUtil.sumUpperBound(minRequiredScore + maxRemainingScore,
numScorers) <=
+ // minCompetitiveScore.
+ double minRequiredScore = minCompetitiveScore - maxRemainingScore;
+ while ((float) MathUtil.sumUpperBound(minRequiredScore +
maxRemainingScore, numScorers)
+ > minCompetitiveScore) {
+ minRequiredScore = Math.nextDown(minRequiredScore);
Review Comment:
I suspect that this would always converge quickly in practice, but the
paranoid me would prefer subtracting an ulp of `minCompetitiveScore` to make
sure it always converges quickly.
```suggestion
minRequiredScore -= Math.ulp((double) minCompetitiveScore);
```
(Many adjacent values may round to the same value when summed up with
`maxRemainingScore` because of how floating point sums round, subtracting an
ulp of minCompetitiveScore forces going to a value whose sum with
`maxRemainingScore` is lower.)
##########
lucene/core/src/java/org/apache/lucene/search/ScorerUtil.java:
##########
@@ -128,15 +128,22 @@ static void filterCompetitiveHits(
double maxRemainingScore,
float minCompetitiveScore,
int numScorers) {
- if ((float) MathUtil.sumUpperBound(maxRemainingScore, numScorers) >=
minCompetitiveScore) {
+ // Compute minRequiredScore as the greatest float value so that (float)
+ // MathUtil.sumUpperBound(minRequiredScore + maxRemainingScore,
numScorers) <=
+ // minCompetitiveScore.
Review Comment:
The comment needs updating (especially as the previous version that I
suggested did not actually compute the greatest float that meets this
condition), something like:
```
Compute a minimum required score, so that
(float) MathUtil.sumUpperBound(minRequiredScore + maxRemainingScore,
numScorers) <= minCompetitiveScore.
The computed value may not be the greatest value that meets this condition,
which means that we may fail to filter
out some docs. However, this doesn't hurt correctness, it just means that
these docs will be filtered out later, and
the extra work required to compute an optimal value would unlikely result in
a speedup.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
