jpountz commented on a change in pull request #2205:
URL: https://github.com/apache/lucene-solr/pull/2205#discussion_r559242170
##########
File path: lucene/core/src/java/org/apache/lucene/search/WANDScorer.java
##########
@@ -149,23 +167,33 @@ private static long scaleMinScore(float minScore, int
scalingFactor) {
this.doc = -1;
this.upTo = -1; // will be computed on the first call to nextDoc/advance
+ this.scoreMode = scoreMode;
+
head = new DisiPriorityQueue(scorers.size());
// there can be at most num_scorers - 1 scorers beyond the current position
tail = new DisiWrapper[scorers.size()];
- OptionalInt scalingFactor = OptionalInt.empty();
- for (Scorer scorer : scorers) {
- scorer.advanceShallow(0);
- float maxScore = scorer.getMaxScore(DocIdSetIterator.NO_MORE_DOCS);
- if (maxScore != 0 && Float.isFinite(maxScore)) {
- // 0 and +Infty should not impact the scale
- scalingFactor =
- OptionalInt.of(
- Math.min(scalingFactor.orElse(Integer.MAX_VALUE),
scalingFactor(maxScore)));
+ if (this.scoreMode == ScoreMode.TOP_SCORES) {
+ OptionalInt scalingFactor = OptionalInt.empty();
+ for (Scorer scorer : scorers) {
+ scorer.advanceShallow(0);
+ float maxScore = scorer.getMaxScore(DocIdSetIterator.NO_MORE_DOCS);
+ if (maxScore != 0 && Float.isFinite(maxScore)) {
+ // 0 and +Infty should not impact the scale
+ scalingFactor =
+ OptionalInt.of(
+ Math.min(scalingFactor.orElse(Integer.MAX_VALUE),
scalingFactor(maxScore)));
+ }
}
+
+ this.scalingFactor = scalingFactor.orElse(0);
+ this.maxScorePropagator = new MaxScoreSumPropagator(scorers);
+ } else {
+ this.scalingFactor = 0;
+ this.maxScorePropagator = null;
}
+
// Use a scaling factor of 0 if all max scores are either 0 or +Infty
Review comment:
this comment looks misplaced now?
##########
File path: lucene/core/src/java/org/apache/lucene/search/WANDScorer.java
##########
@@ -257,13 +288,17 @@ public int advance(int target) throws IOException {
// Advance 'head' as well
advanceHead(target);
- // Pop the new 'lead' from 'head'
- moveToNextCandidate(target);
+ if (scoreMode == ScoreMode.TOP_SCORES) {
+ // Update score bounds if necessary so
+ updateMaxScoresIfNecessary(target);
- if (doc == DocIdSetIterator.NO_MORE_DOCS) {
- return DocIdSetIterator.NO_MORE_DOCS;
+ if (doc == DocIdSetIterator.NO_MORE_DOCS) {
Review comment:
we may have a problem, or maybe this line is not needed anymore
Before, `moveToNextCandidate` would advance `doc`. But now that you are
calling `updateMaxScoresIfNecessary ` instead, which doesn't advance doc, it
shouldn't be possible that doc == NO_MORE_DOCS on this line?
----------------------------------------------------------------
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.
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
