[GitHub] [solr] andywebb1975 commented on a diff in pull request #1431: SOLR-16643: Add reRankOperator=multiply/replace options to ReRank query parser

["andywebb1975 (via GitHub)" <gi...@apache.org>]

andywebb1975 commented on code in PR #1431:
URL: https://github.com/apache/solr/pull/1431#discussion_r1126991270


##########
solr/core/src/java/org/apache/solr/search/ReRankQParserPlugin.java:
##########
@@ -68,29 +71,51 @@ public Query parse() throws SyntaxError {
       Query reRankQuery = reRankParser.parse();
 
       int reRankDocs = localParams.getInt(RERANK_DOCS, RERANK_DOCS_DEFAULT);
-      reRankDocs = Math.max(1, reRankDocs); //
+      reRankDocs = Math.max(1, reRankDocs);
 
       double reRankWeight = localParams.getDouble(RERANK_WEIGHT, RERANK_WEIGHT_DEFAULT);
 
-      return new ReRankQuery(reRankQuery, reRankDocs, reRankWeight);
+      ReRankOperator reRankOperator =
+          ReRankOperator.get(localParams.get(RERANK_OPERATOR, RERANK_OPERATOR_DEFAULT));
+
+      return new ReRankQuery(reRankQuery, reRankDocs, reRankWeight, reRankOperator);
     }
   }
 
   private static final class ReRankQueryRescorer extends QueryRescorer {
 
-    final double reRankWeight;
+    final BiFloatFunction scoreCombiner;
+
+    @FunctionalInterface
+    interface BiFloatFunction {
+      float func(float a, float b);
+    }
 
-    public ReRankQueryRescorer(Query reRankQuery, double reRankWeight) {
+    public ReRankQueryRescorer(
+        Query reRankQuery, double reRankWeight, ReRankOperator reRankOperator) {
       super(reRankQuery);
-      this.reRankWeight = reRankWeight;
+      switch (reRankOperator) {
+        case ADD:
+          scoreCombiner = (score, second) -> (float) (score + reRankWeight * second);
+          break;
+        case MULTIPLY:
+          scoreCombiner = (score, second) -> (float) (score * reRankWeight * second);
+          break;
+        case REPLACE:
+          scoreCombiner = (score, second) -> (float) (reRankWeight * second);

Review Comment:
   Thanks Christine! Yes, as far as I can see using `reRankWeight` is redundant for both `multiply` and `replace` - it’ll just scale the scores of all the reranked docs without changing their positions relative to each other - but I figured it'd be less surprising to be consistent between the operators rather than ignoring `reRankWeight` for some of them. This has got me wondering about negative weights though - I'll check what happens with those...
   
   Using an `originalWeight` option would work too, yes - though I think it'll be clearer to have the explicit `reRankOperator=replace` in the query syntax so I'd prefer to stick with that. We'd still need the `reRankOperator=multiply` option anyway and it was trivial to add the extra `replace` option once `multiply` was there!



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org