You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "alessandrobenedetti (via GitHub)" <gi...@apache.org> on 2024/05/31 14:25:59 UTC
[PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
alessandrobenedetti opened a new pull request, #2489:
URL: https://github.com/apache/solr/pull/2489
https://issues.apache.org/jira/browse/SOLR-17319
# Description
Reciprocal Rank Fusion (RRF) is an algorithm that takes in input multiple ranked lists to produce a unified result set.
Examples of use cases where RRF can be used include hybrid search and multiple Knn vector queries executed concurrently.
RRF is based on the concept of reciprocal rank, which is the inverse of the rank of a document in a ranked list of search results.
The combination of search results happens taking into account the position of
the items in the original rankings, and giving higher score to items that are ranked higher in multiple lists. RRF was introduced the first time by Cormack et al. in [1].
The syntax proposed:
JSON Request
```
{
"queries": {
"lexical1": {
"lucene": {
"query": "id:(10^=2 OR 2^=1 OR 4^=0.5)"
}
},
"lexical2": {
"lucene": {
"query": "id:(2^=2 OR 4^=1 OR 3^=0.5)"
}
}
},
"limit": 10,
"fields": "[id,score]",
"params": {
"combiner": true,
"combiner.upTo": 5,
"facet": true,
"facet.field": "id",
"facet.mincount": 1
}
}
```
[1] Cormack, Gordon V. et al. “Reciprocal rank fusion outperforms condorcet and individual rank learning methods.” Proceedings of the 32nd international ACM SIGIR conference on Research and development in information retrieval (2009)
# Solution
The support has been introduced leveraging the "queries" support in the JSON request syntax.
The combination of results happen at QueryComponent, processing level.
The debug component has the responsibility of managing the explainability part.
Limitations:
- grouping/field collapsing won't be supported in a first version
- the distributed support is present but not super advanced: query combination happens at node level and then results are combined at shard level.
# Tests
Tests have been added to cover the main use cases
# Checklist
Please review the following and check all that apply:
- [ ] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [ ] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1632229327
##########
solr/core/src/java/org/apache/solr/search/combining/QueriesCombiner.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import static org.apache.solr.common.params.CombinerParams.RECIPROCAl_RANK_FUSION;
+
+import java.io.IOException;
+import java.util.List;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+
+/**
+ * Combining considers two or more query rankedLists: resultA, resultB ...<br>
+ * For a given query, each query result is a ranked list of documents La = (a1,a2,...), Lb = (b1,
+ * b2, ...)...<br>
+ * A combining algorithm creates a unique ranked list I = (i1, i2, ...).<br>
+ * This list is created by combining elements from the lists la and lb as described by the
+ * implementation algorithm.<br>
+ */
+public abstract class QueriesCombiner {
Review Comment:
done, coming in next commit!
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1632229302
##########
solr/core/src/java/org/apache/solr/search/combining/QueriesCombiner.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import static org.apache.solr.common.params.CombinerParams.RECIPROCAl_RANK_FUSION;
+
+import java.io.IOException;
+import java.util.List;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+
+/**
+ * Combining considers two or more query rankedLists: resultA, resultB ...<br>
+ * For a given query, each query result is a ranked list of documents La = (a1,a2,...), Lb = (b1,
+ * b2, ...)...<br>
+ * A combining algorithm creates a unique ranked list I = (i1, i2, ...).<br>
+ * This list is created by combining elements from the lists la and lb as described by the
+ * implementation algorithm.<br>
+ */
+public abstract class QueriesCombiner {
+
+ protected int upTo;
+
+ public QueriesCombiner(SolrParams requestParams) {
+ this.upTo =
+ requestParams.getInt(CombinerParams.COMBINER_UP_TO, CombinerParams.COMBINER_UP_TO_DEFAULT);
+ }
+
+ public abstract QueryResult combine(QueryResult[] rankedLists);
+
+ QueryResult initCombinedResult(QueryResult[] rankedLists) {
Review Comment:
done, coming in next commit!
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2171937408
I did the work of migrating to a QParser but don't have permissions to push to your PR branch.
Beware, I'm reverting most/all of the files that you modified of existing Solr source files. If you would prefer I submit a separate PR, I'll do that. Instead of `combiner=true`, you'd do `q={!combine}`.
Wouldn't the docSet (assuming it's needed, like for faceting) be computed on each subQuery, and if so how is that rolled up to the final QueryResult? I see you tested that faceting works but I suspect there may be a very sad performance bug here.
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2375471479
This PR has had no activity for 60 days and is now labeled as stale. Any new activity or converting it to draft will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the dev@solr.apache.org mailing list. Thank you for your contribution!
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2153161526
> I haven't looked at this in detail but I'm wondering could we instead have a single 'q' query parser, that is itself a "query combiner" of its children clauses? Ultimately this would mean not needing all this special code in QueryComponent. I'm trying to make recommendations that "fit in" well with Solr's architecture; less bolted / hacked on.
>
> CC @hossman I think you'd have good inputs on this one too
In principle I agree, correct me if I'm wrong, your proposed solution means:
1) a Lucene contribution to create a new query
2) a Solr query parser to combine N queries into the new Lucene one?
I would need to think and design this possible solution, but I have no idea when this will happen as I am afraid it would take a significant amount of time. (of course anyone is happy to help).
Do you think it would make sense to start refining this contribution, bake it in so that users have this new possibility decently quickly and then iteratively replace it?
It's not because I have a strong opinion on this, I am just realistically think what could bring value from my side, in accordance with my future availabilty.
Happy to discuss!
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "renatoh (via GitHub)" <gi...@apache.org>.
renatoh commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1624657341
##########
solr/core/src/java/org/apache/solr/search/combining/ReciprocalRankFusion.java:
##########
@@ -0,0 +1,208 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import static org.apache.lucene.search.TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocIterator;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SortedIntDocSet;
+
+/**
+ * Reciprocal Rank Fusion (RRF) is an algorithm that takes in input multiple ranked lists to produce
+ * a unified result set. Examples of use cases where RRF can be used include hybrid search and
+ * multiple Knn vector queries executed concurrently. RRF is based on the concept of reciprocal
+ * rank, which is the inverse of the rank of a document in a ranked list of search results. The
+ * combination of search results happens taking into account the position of the items in the
+ * original rankings, and giving higher score to items that are ranked higher in multiple lists. RRF
+ * was introduced the first time by Cormack et al. in [1].<br>
+ *
+ * <p>[1] Cormack, Gordon V. et al. “Reciprocal rank fusion outperforms condorcet and individual
+ * rank learning methods.” Proceedings of the 32nd international ACM SIGIR conference on Research
+ * and development in information retrieval (2009)<br>
+ */
+public class ReciprocalRankFusion extends QueriesCombiner {
+ int k;
+
+ public ReciprocalRankFusion(SolrParams requestParams) {
+ super(requestParams);
+ this.k =
+ requestParams.getInt(CombinerParams.COMBINER_RRF_K, CombinerParams.COMBINER_RRF_K_DEFAULT);
+ }
+
+ @Override
+ public QueryResult combine(QueryResult[] rankedLists) {
+ QueryResult combinedRankedList = initCombinedResult(rankedLists);
+ List<DocList> docLists = new ArrayList<>(rankedLists.length);
+ for (QueryResult rankedList : rankedLists) {
+ docLists.add(rankedList.getDocList());
+ }
+ combineResults(combinedRankedList, docLists, false);
+ return combinedRankedList;
+ }
+
+ private Map<Integer, Integer[]> combineResults(
+ QueryResult combinedRankedList,
+ List<DocList> rankedLists,
+ boolean saveRankPositionsForExplain) {
+ Map<Integer, Integer[]> docIdToRanks = null;
+ HashMap<Integer, Float> docIdToScore = new HashMap<>();
+ for (DocList rankedList : rankedLists) {
+ DocIterator docs = rankedList.iterator();
+ int ranking = 1;
+ while (docs.hasNext() && ranking <= upTo) {
+ int docId = docs.nextDoc();
+ float rrfScore = 1f / (k + ranking);
+ docIdToScore.compute(docId, (id, score) -> (score == null) ? rrfScore : score + rrfScore);
+ ranking++;
+ }
+ }
+ Stream<Map.Entry<Integer, Float>> sortedByScoreDescending =
+ docIdToScore.entrySet().stream()
+ .sorted(Collections.reverseOrder(Map.Entry.comparingByValue()));
+
+ int combinedResultsLength = docIdToScore.size();
+ int[] combinedResultsDocIds = new int[combinedResultsLength];
+ float[] combinedResultScores = new float[combinedResultsLength];
+
+ int i = 0;
+ for (Map.Entry<Integer, Float> scoredDoc :
+ sortedByScoreDescending.collect(Collectors.toList())) {
+ combinedResultsDocIds[i] = scoredDoc.getKey();
+ combinedResultScores[i] = scoredDoc.getValue();
+ i++;
+ }
+
+ if (saveRankPositionsForExplain) {
+ docIdToRanks = getRanks(rankedLists, combinedResultsDocIds);
+ }
+
+ DocSlice combinedResultSlice =
+ new DocSlice(
+ 0,
+ combinedResultsLength,
+ combinedResultsDocIds,
+ combinedResultScores,
+ combinedResultsLength,
+ combinedResultScores[0],
+ GREATER_THAN_OR_EQUAL_TO);
+ combinedRankedList.setDocList(combinedResultSlice);
+ SortedIntDocSet docSet = new SortedIntDocSet(combinedResultsDocIds, combinedResultsLength);
+ combinedRankedList.setDocSet(docSet);
+
+ return docIdToRanks;
+ }
+
+ private Map<Integer, Integer[]> getRanks(List<DocList> docLists, int[] combinedResultsDocIDs) {
+ Map<Integer, Integer[]> docIdToRanks;
+ docIdToRanks = new HashMap<>();
+ for (int docID : combinedResultsDocIDs) {
+ docIdToRanks.put(docID, new Integer[docLists.size()]);
Review Comment:
oh dear, I completely missed the[] and thought you are create a new Integer by doing: new Integer() and therefore preventing them from being cached. sorry, my bad!
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1629951486
##########
solr/core/src/java/org/apache/solr/request/json/RequestUtil.java:
##########
@@ -241,13 +243,16 @@ public static void processParams(
if (queriesJsonObj instanceof Map && queriesJsonObj != null) {
@SuppressWarnings("unchecked")
final Map<String, Object> queriesAsMap = (Map<String, Object>) queriesJsonObj;
+ ArrayList<String> queryKeys = new ArrayList<>();
for (Map.Entry<String, Object> queryJsonProperty : queriesAsMap.entrySet()) {
out = queryJsonProperty.getKey();
+ queryKeys.add(out);
arr = true;
isQuery = true;
convertJsonPropertyToLocalParams(
newMap, jsonQueryConverter, queryJsonProperty, out, isQuery, arr);
}
+ newMap.put(COMBINER_KEYS, queryKeys.toArray(new String[queryKeys.size()]));
Review Comment:
```suggestion
newMap.put(COMBINER_KEYS, queryKeys.toArray(new String[0]));
```
We standardized on this recently; surprisingly a 0 helps perf
##########
solr/core/src/test/org/apache/solr/handler/component/DebugComponentTest.java:
##########
@@ -52,7 +64,7 @@ public void testBasicInterface() {
"//str[@name='querystring']='*:*'",
"//str[@name='parsedquery']='MatchAllDocsQuery(*:*)'",
"//str[@name='parsedquery_toString']='*:*'",
- "count(//lst[@name='explain']/*)=3",
+ "count(//lst[@name='explain']/*)=6",
Review Comment:
How does this PR affect this assertion of an existing test?
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #2489: Introduce support for Reciprocal Rank Fusion (combining queries)
URL: https://github.com/apache/solr/pull/2489
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "renatoh (via GitHub)" <gi...@apache.org>.
renatoh commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1623870400
##########
solr/core/src/java/org/apache/solr/search/combining/ReciprocalRankFusion.java:
##########
@@ -0,0 +1,208 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import static org.apache.lucene.search.TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocIterator;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SortedIntDocSet;
+
+/**
+ * Reciprocal Rank Fusion (RRF) is an algorithm that takes in input multiple ranked lists to produce
+ * a unified result set. Examples of use cases where RRF can be used include hybrid search and
+ * multiple Knn vector queries executed concurrently. RRF is based on the concept of reciprocal
+ * rank, which is the inverse of the rank of a document in a ranked list of search results. The
+ * combination of search results happens taking into account the position of the items in the
+ * original rankings, and giving higher score to items that are ranked higher in multiple lists. RRF
+ * was introduced the first time by Cormack et al. in [1].<br>
+ *
+ * <p>[1] Cormack, Gordon V. et al. “Reciprocal rank fusion outperforms condorcet and individual
+ * rank learning methods.” Proceedings of the 32nd international ACM SIGIR conference on Research
+ * and development in information retrieval (2009)<br>
+ */
+public class ReciprocalRankFusion extends QueriesCombiner {
+ int k;
+
+ public ReciprocalRankFusion(SolrParams requestParams) {
+ super(requestParams);
+ this.k =
+ requestParams.getInt(CombinerParams.COMBINER_RRF_K, CombinerParams.COMBINER_RRF_K_DEFAULT);
+ }
+
+ @Override
+ public QueryResult combine(QueryResult[] rankedLists) {
+ QueryResult combinedRankedList = initCombinedResult(rankedLists);
+ List<DocList> docLists = new ArrayList<>(rankedLists.length);
+ for (QueryResult rankedList : rankedLists) {
+ docLists.add(rankedList.getDocList());
+ }
+ combineResults(combinedRankedList, docLists, false);
+ return combinedRankedList;
+ }
+
+ private Map<Integer, Integer[]> combineResults(
+ QueryResult combinedRankedList,
+ List<DocList> rankedLists,
+ boolean saveRankPositionsForExplain) {
+ Map<Integer, Integer[]> docIdToRanks = null;
+ HashMap<Integer, Float> docIdToScore = new HashMap<>();
+ for (DocList rankedList : rankedLists) {
+ DocIterator docs = rankedList.iterator();
+ int ranking = 1;
+ while (docs.hasNext() && ranking <= upTo) {
+ int docId = docs.nextDoc();
+ float rrfScore = 1f / (k + ranking);
+ docIdToScore.compute(docId, (id, score) -> (score == null) ? rrfScore : score + rrfScore);
+ ranking++;
+ }
+ }
+ Stream<Map.Entry<Integer, Float>> sortedByScoreDescending =
+ docIdToScore.entrySet().stream()
+ .sorted(Collections.reverseOrder(Map.Entry.comparingByValue()));
+
+ int combinedResultsLength = docIdToScore.size();
+ int[] combinedResultsDocIds = new int[combinedResultsLength];
+ float[] combinedResultScores = new float[combinedResultsLength];
+
+ int i = 0;
+ for (Map.Entry<Integer, Float> scoredDoc :
+ sortedByScoreDescending.collect(Collectors.toList())) {
+ combinedResultsDocIds[i] = scoredDoc.getKey();
+ combinedResultScores[i] = scoredDoc.getValue();
+ i++;
+ }
+
+ if (saveRankPositionsForExplain) {
+ docIdToRanks = getRanks(rankedLists, combinedResultsDocIds);
+ }
+
+ DocSlice combinedResultSlice =
+ new DocSlice(
+ 0,
+ combinedResultsLength,
+ combinedResultsDocIds,
+ combinedResultScores,
+ combinedResultsLength,
+ combinedResultScores[0],
+ GREATER_THAN_OR_EQUAL_TO);
+ combinedRankedList.setDocList(combinedResultSlice);
+ SortedIntDocSet docSet = new SortedIntDocSet(combinedResultsDocIds, combinedResultsLength);
+ combinedRankedList.setDocSet(docSet);
+
+ return docIdToRanks;
+ }
+
+ private Map<Integer, Integer[]> getRanks(List<DocList> docLists, int[] combinedResultsDocIDs) {
+ Map<Integer, Integer[]> docIdToRanks;
+ docIdToRanks = new HashMap<>();
+ for (int docID : combinedResultsDocIDs) {
+ docIdToRanks.put(docID, new Integer[docLists.size()]);
Review Comment:
Any particular reason for not using Integer.valueOf()?
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1630219623
##########
solr/core/src/java/org/apache/solr/search/combining/QueriesCombiner.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import static org.apache.solr.common.params.CombinerParams.RECIPROCAl_RANK_FUSION;
+
+import java.io.IOException;
+import java.util.List;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+
+/**
+ * Combining considers two or more query rankedLists: resultA, resultB ...<br>
+ * For a given query, each query result is a ranked list of documents La = (a1,a2,...), Lb = (b1,
+ * b2, ...)...<br>
+ * A combining algorithm creates a unique ranked list I = (i1, i2, ...).<br>
+ * This list is created by combining elements from the lists la and lb as described by the
+ * implementation algorithm.<br>
+ */
+public abstract class QueriesCombiner {
Review Comment:
I like the API. Maybe add @version 9.7. Also mention in javadoc Used by {@link QueryComponent}.
##########
solr/core/src/java/org/apache/solr/search/combining/QueriesCombiner.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import static org.apache.solr.common.params.CombinerParams.RECIPROCAl_RANK_FUSION;
+
+import java.io.IOException;
+import java.util.List;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+
+/**
+ * Combining considers two or more query rankedLists: resultA, resultB ...<br>
+ * For a given query, each query result is a ranked list of documents La = (a1,a2,...), Lb = (b1,
+ * b2, ...)...<br>
+ * A combining algorithm creates a unique ranked list I = (i1, i2, ...).<br>
+ * This list is created by combining elements from the lists la and lb as described by the
+ * implementation algorithm.<br>
+ */
+public abstract class QueriesCombiner {
+
+ protected int upTo;
+
+ public QueriesCombiner(SolrParams requestParams) {
+ this.upTo =
+ requestParams.getInt(CombinerParams.COMBINER_UP_TO, CombinerParams.COMBINER_UP_TO_DEFAULT);
+ }
+
+ public abstract QueryResult combine(QueryResult[] rankedLists);
+
+ QueryResult initCombinedResult(QueryResult[] rankedLists) {
Review Comment:
mark protected
##########
solr/solrj/src/java/org/apache/solr/common/params/CombinerParams.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.params;
+
+/** Defines the request parameters used by all combiners. */
+public interface CombinerParams {
+ String COMBINER = "combiner";
+
+ String COMBINER_ALGORITHM = COMBINER + ".algorithm";
+
+ String COMBINER_KEYS = COMBINER + ".keys";
Review Comment:
WDYT of instead do `combine.q` multi-valued, so just supply each query as another value? One less indirection and thing to name.
##########
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##########
@@ -173,22 +175,51 @@ public void prepare(ResponseBuilder rb) throws IOException {
// get it from the response builder to give a different component a chance
// to set it.
- String queryString = rb.getQueryString();
- if (queryString == null) {
- // this is the normal way it's set.
- queryString = params.get(CommonParams.Q);
- rb.setQueryString(queryString);
+ List<String> unparsedQueries;
+ rb.isCombined = params.getBool(CombinerParams.COMBINER, false);
+ if (rb.isCombinedSearch()) {
+ String[] queriesToCombineKeys = params.getParams(CombinerParams.COMBINER_KEYS);
+ unparsedQueries = new ArrayList<>(queriesToCombineKeys.length);
+ rb.queriesCombiningStrategy = QueriesCombiner.getImplementation(params);
+ for (String queryKey : queriesToCombineKeys) {
+ unparsedQueries.add(params.get(queryKey));
+ }
+ } else {
+ unparsedQueries = new ArrayList<>(1);
Review Comment:
It'd be nicer to use List.of(oneThing) or List.of() instead but whatever.
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1643463278
##########
solr/core/src/java/org/apache/solr/request/json/RequestUtil.java:
##########
@@ -241,13 +243,16 @@ public static void processParams(
if (queriesJsonObj instanceof Map && queriesJsonObj != null) {
@SuppressWarnings("unchecked")
final Map<String, Object> queriesAsMap = (Map<String, Object>) queriesJsonObj;
+ ArrayList<String> queryKeys = new ArrayList<>();
for (Map.Entry<String, Object> queryJsonProperty : queriesAsMap.entrySet()) {
out = queryJsonProperty.getKey();
+ queryKeys.add(out);
arr = true;
isQuery = true;
convertJsonPropertyToLocalParams(
newMap, jsonQueryConverter, queryJsonProperty, out, isQuery, arr);
}
+ newMap.put(COMBINER_KEYS, queryKeys.toArray(new String[0])); // nocommit a hack
Review Comment:
Making RequestUtils add a param specific to a niche feature is a hack. If we really need this here (?), I think we should use a generic name like "json.queries".
##########
solr/core/src/java/org/apache/solr/search/combining/CombineQParserPlugin.java:
##########
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import org.apache.lucene.search.BooleanClause.Occur;
+import org.apache.lucene.search.MatchNoDocsQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.component.QueryComponent;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.search.ExtendedQueryBase;
+import org.apache.solr.search.QParser;
+import org.apache.solr.search.QParserPlugin;
+import org.apache.solr.search.QueryCommand;
+import org.apache.solr.search.QueryParsing;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SyntaxError;
+
+public class CombineQParserPlugin extends QParserPlugin {
Review Comment:
This source file captures the essential aspect of my proposal. Needs another iteration of refinement at least (e.g. javadocs, and param naming / defaulting). Maybe more.
##########
solr/core/src/test/org/apache/solr/handler/component/DebugComponentTest.java:
##########
@@ -52,7 +64,7 @@ public void testBasicInterface() {
"//str[@name='querystring']='*:*'",
"//str[@name='parsedquery']='MatchAllDocsQuery(*:*)'",
"//str[@name='parsedquery_toString']='*:*'",
- "count(//lst[@name='explain']/*)=3",
+ "count(//lst[@name='explain']/*)=6",
Review Comment:
I suppose I'd need to see a before & after to truly "see" the change.
##########
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##########
@@ -1689,10 +1689,11 @@ private void doProcessUngroupedSearch(ResponseBuilder rb, QueryCommand cmd, Quer
SolrIndexSearcher searcher = req.getSearcher();
- try {
+ if (cmd.getQuery() instanceof SelfExecutingQuery) {
+ // TODO QueryResult ought not to be created and passed in methods of QueryComponent
+ result = ((SelfExecutingQuery) cmd.getQuery()).search(searcher, cmd);
+ } else {
Review Comment:
Here's where the minimal essential bit to alter QueryComponent by adding a new abstraction (SelfExecutingQuery) that might be useful for other features; not just "combining".
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -706,9 +707,12 @@ public <K, V> boolean regenerateItem(
}
}
- public QueryResult search(QueryResult qr, QueryCommand cmd) throws IOException {
- getDocListC(qr, cmd);
- return qr;
+ public void search(QueryResult qr, QueryCommand cmd) throws IOException {
+ try {
+ getDocListC(qr, cmd);
+ } catch (FuzzyTermsEnum.FuzzyTermsException e) { // unsure where best to catch this; shrug
Review Comment:
I moved the FuzzyTermsException catch block from QueryComponent to here. I wanted it in a deeper place that would be caught by multiple inward code paths instead of just QC. It still feels it ought to be deeper but I didn't look for a better spot. CombineQuery calls into this.
##########
solr/core/src/java/org/apache/solr/search/combining/CombineQParserPlugin.java:
##########
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import org.apache.lucene.search.BooleanClause.Occur;
+import org.apache.lucene.search.MatchNoDocsQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.component.QueryComponent;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.search.ExtendedQueryBase;
+import org.apache.solr.search.QParser;
+import org.apache.solr.search.QParserPlugin;
+import org.apache.solr.search.QueryCommand;
+import org.apache.solr.search.QueryParsing;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SyntaxError;
+
+public class CombineQParserPlugin extends QParserPlugin {
+ public static final String NAME = "combine";
+
+ @Override
+ public QParser createParser(
+ String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) {
+ return new CombineQParser(qstr, localParams, params, req);
+ }
+
+ static class CombineQParser extends QParser {
+
+ private List<String> unparsedQueries;
+ private QueriesCombiner queriesCombiningStrategy;
+ private List<QParser> queryParsers;
+ private List<Query> queries;
+
+ public CombineQParser(
+ String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) {
+ super(qstr, localParams, params, req);
+ }
+
+ @Override
+ public Query parse() throws SyntaxError {
+
+ var queriesToCombineKeys = localParams.getParams("keys");
+ if (queriesToCombineKeys == null) {
+ queriesToCombineKeys = params.getParams(CombinerParams.COMBINER_KEYS);
+ }
+
+ unparsedQueries = new ArrayList<>(queriesToCombineKeys.length);
+ queryParsers = new ArrayList<>(queriesToCombineKeys.length);
+ queries = new ArrayList<>(queriesToCombineKeys.length);
+
+ // nocommit blend localParams and params without needless suffix in local
+ var blendParams = SolrParams.wrapDefaults(localParams, params);
+ queriesCombiningStrategy = QueriesCombiner.getImplementation(blendParams);
+
+ String defType = blendParams.get(QueryParsing.DEFTYPE, DEFAULT_QTYPE);
+
+ for (String queryKey : queriesToCombineKeys) {
+ final var unparsedQuery = blendParams.get(queryKey);
+ final var parser = QParser.getParser(unparsedQuery, defType, req);
+ var query = parser.getQuery();
+ if (query == null) { // sad this can happen
+ query = new MatchNoDocsQuery();
+ }
+ unparsedQueries.add(unparsedQuery);
+ queryParsers.add(parser);
+ queries.add(query);
+ }
+
+ return new CombineQuery(queriesCombiningStrategy, queries);
+ }
+
+ @Override
+ public void addDebugInfo(NamedList<Object> dbg) {
+ super.addDebugInfo(dbg);
+
+ NamedList<NamedList<Object>> queriesDebug = new SimpleOrderedMap<>();
+ String[] queryKeys = req.getParams().getParams(CombinerParams.COMBINER_KEYS);
+ for (int i = 0; i < queries.size(); i++) {
+ NamedList<Object> singleQueryDebug = new SimpleOrderedMap<>();
+ singleQueryDebug.add("querystring", unparsedQueries.get(i));
+ singleQueryDebug.add("queryparser", queryParsers.get(i).getClass().getSimpleName());
+ singleQueryDebug.add("parsedquery", QueryParsing.toString(queries.get(i), req.getSchema()));
+ singleQueryDebug.add("parsedquery_toString", queries.get(i).toString());
+ queriesDebug.add(queryKeys[i], singleQueryDebug);
+ }
+ dbg.add("queriesToCombine", queriesDebug);
+ }
+ }
+
+ static final class CombineQuery extends ExtendedQueryBase
+ implements QueryComponent.SelfExecutingQuery {
+
+ private final QueriesCombiner combiner;
+ private final List<Query> queries;
+
+ public CombineQuery(QueriesCombiner combiner, List<Query> queries) {
+ this.combiner = combiner;
+ this.queries = queries;
+ }
+
+ @Override
+ public String toString(String field) {
+ return super.toString(field) + "{!" + NAME + "}"; // TODO others
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (!(o instanceof CombineQuery)) return false;
+ CombineQuery that = (CombineQuery) o;
+ return Objects.equals(combiner, that.combiner) && Objects.equals(queries, that.queries);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(combiner, queries);
+ }
+
+ @Override
+ public void visit(QueryVisitor visitor) {
+ for (Query query : queries) {
+ query.visit(visitor.getSubVisitor(Occur.MUST, this));
+ }
+ }
+
+ @Override
+ public QueryResult search(SolrIndexSearcher searcher, QueryCommand cmd) throws IOException {
+ QueryResult[] results = new QueryResult[queries.size()];
+ // TODO do in multiple threads?
+ for (int i = 0; i < queries.size(); i++) {
+ cmd.setQuery(queries.get(i));
+ QueryResult qr = new QueryResult();
+ searcher.search(qr, cmd);
+ results[i] = qr;
+ }
+
+ // nocommit but how is the docSet (e.g. for faceting) or maybe other things supported?
Review Comment:
This seems like a fundamental limitation in the whole feature honestly. What Hossman said about re-rank query is interesting... not sure if it addresses this.
##########
solr/core/src/java/org/apache/solr/search/combining/CombineQParserPlugin.java:
##########
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import org.apache.lucene.search.BooleanClause.Occur;
+import org.apache.lucene.search.MatchNoDocsQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.component.QueryComponent;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.search.ExtendedQueryBase;
+import org.apache.solr.search.QParser;
+import org.apache.solr.search.QParserPlugin;
+import org.apache.solr.search.QueryCommand;
+import org.apache.solr.search.QueryParsing;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SyntaxError;
+
+public class CombineQParserPlugin extends QParserPlugin {
+ public static final String NAME = "combine";
+
+ @Override
+ public QParser createParser(
+ String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) {
+ return new CombineQParser(qstr, localParams, params, req);
+ }
+
+ static class CombineQParser extends QParser {
+
+ private List<String> unparsedQueries;
+ private QueriesCombiner queriesCombiningStrategy;
+ private List<QParser> queryParsers;
+ private List<Query> queries;
+
+ public CombineQParser(
+ String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) {
+ super(qstr, localParams, params, req);
+ }
+
+ @Override
+ public Query parse() throws SyntaxError {
+
+ var queriesToCombineKeys = localParams.getParams("keys");
+ if (queriesToCombineKeys == null) {
+ queriesToCombineKeys = params.getParams(CombinerParams.COMBINER_KEYS);
+ }
+
+ unparsedQueries = new ArrayList<>(queriesToCombineKeys.length);
+ queryParsers = new ArrayList<>(queriesToCombineKeys.length);
+ queries = new ArrayList<>(queriesToCombineKeys.length);
+
+ // nocommit blend localParams and params without needless suffix in local
+ var blendParams = SolrParams.wrapDefaults(localParams, params);
+ queriesCombiningStrategy = QueriesCombiner.getImplementation(blendParams);
+
+ String defType = blendParams.get(QueryParsing.DEFTYPE, DEFAULT_QTYPE);
+
+ for (String queryKey : queriesToCombineKeys) {
+ final var unparsedQuery = blendParams.get(queryKey);
+ final var parser = QParser.getParser(unparsedQuery, defType, req);
+ var query = parser.getQuery();
+ if (query == null) { // sad this can happen
+ query = new MatchNoDocsQuery();
+ }
+ unparsedQueries.add(unparsedQuery);
+ queryParsers.add(parser);
+ queries.add(query);
+ }
+
+ return new CombineQuery(queriesCombiningStrategy, queries);
+ }
+
+ @Override
+ public void addDebugInfo(NamedList<Object> dbg) {
+ super.addDebugInfo(dbg);
+
+ NamedList<NamedList<Object>> queriesDebug = new SimpleOrderedMap<>();
+ String[] queryKeys = req.getParams().getParams(CombinerParams.COMBINER_KEYS);
+ for (int i = 0; i < queries.size(); i++) {
+ NamedList<Object> singleQueryDebug = new SimpleOrderedMap<>();
+ singleQueryDebug.add("querystring", unparsedQueries.get(i));
+ singleQueryDebug.add("queryparser", queryParsers.get(i).getClass().getSimpleName());
+ singleQueryDebug.add("parsedquery", QueryParsing.toString(queries.get(i), req.getSchema()));
+ singleQueryDebug.add("parsedquery_toString", queries.get(i).toString());
+ queriesDebug.add(queryKeys[i], singleQueryDebug);
+ }
+ dbg.add("queriesToCombine", queriesDebug);
+ }
+ }
+
+ static final class CombineQuery extends ExtendedQueryBase
Review Comment:
A "SelfExecutingQuery" isn't normal; Lucene isn't going to work with this thing since it doesn't implement getWeight nor can it. But users don't need to know/care.
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2246125113
@cpoerschke see the parent JIRA issue for the real state of this controvertial PR
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1688438487
##########
solr/solrj/src/java/org/apache/solr/common/params/CombinerParams.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.params;
+
+/** Defines the request parameters used by all combiners. */
+public interface CombinerParams {
+ String COMBINER = "combiner";
+
+ String COMBINER_ALGORITHM = COMBINER + ".algorithm";
+
+ String COMBINER_KEYS = COMBINER + ".keys";
+
+ String RECIPROCAl_RANK_FUSION = "rrf";
Review Comment:
```suggestion
String RECIPROCAL_RANK_FUSION = "rrf";
```
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1688607183
##########
solr/solrj/src/java/org/apache/solr/common/params/CombinerParams.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.params;
+
+/** Defines the request parameters used by all combiners. */
+public interface CombinerParams {
+ String COMBINER = "combiner";
+
+ String COMBINER_ALGORITHM = COMBINER + ".algorithm";
+
+ String COMBINER_KEYS = COMBINER + ".keys";
Review Comment:
AFAIK it's guaranteed; we use a List not a Set intentionally
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2253034480
> @cpoerschke see the parent JIRA issue for the real state of this controvertial PR
Thanks for the pointer! It inspired me to do some #2597 scribbling.
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1632229605
##########
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##########
@@ -173,22 +175,51 @@ public void prepare(ResponseBuilder rb) throws IOException {
// get it from the response builder to give a different component a chance
// to set it.
- String queryString = rb.getQueryString();
- if (queryString == null) {
- // this is the normal way it's set.
- queryString = params.get(CommonParams.Q);
- rb.setQueryString(queryString);
+ List<String> unparsedQueries;
+ rb.isCombined = params.getBool(CombinerParams.COMBINER, false);
+ if (rb.isCombinedSearch()) {
+ String[] queriesToCombineKeys = params.getParams(CombinerParams.COMBINER_KEYS);
+ unparsedQueries = new ArrayList<>(queriesToCombineKeys.length);
+ rb.queriesCombiningStrategy = QueriesCombiner.getImplementation(params);
+ for (String queryKey : queriesToCombineKeys) {
+ unparsedQueries.add(params.get(queryKey));
+ }
+ } else {
+ unparsedQueries = new ArrayList<>(1);
Review Comment:
Done, coming in next commit
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2153266206
Want to quickly respond to this:
> a Lucene contribution to create a new query
No! The Lucene project is not the only place where subclasses of Query classes exist ;-)
I'm leary of blessing committing something architecturally unclean (in my mind; no offense) that will supposedly be fixed later. I confess I'm not going to veto this as it's not all that bad really; it reads clearly, I understand this part of it so I shouldn't complain too loudly. But personally I'm not going to approve this PR and this wont' block your merge.
There are trade-offs admittedly... a special/hacky Query as I proposed (assuming it needs to be hacky?) is its own abuse of sorts; we have another -- PostFilter that isn't *really* a Query. But at least those are contained to a whole abstraction implementation. Meaning, ignore it if you don't care about it. But QueryComponent is doing a lot; it's already complex... more conditions and special cases here is something we should watch out for. QueryComponent is on the critical path; a place to fight against complexity. Long ago QC was relatively clean and then query grouping came along and well, what a mess that made!
Overall I wish we made proposals on the dev list about our ideas before showing up with an implementation. I want my colleagues to do the same so peer review can influence the approach early. I love reviewing other people's ideas before time is invested in an implementation! I had an ad-hoc call with another committer this week to do this sort of thing.
I'll see if I can look closer at your PR to think more about a specific recommendation. Sigh; I'm busy though
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1688453058
##########
solr/core/src/java/org/apache/solr/search/combining/CombineQParserPlugin.java:
##########
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+import org.apache.lucene.search.BooleanClause.Occur;
+import org.apache.lucene.search.MatchNoDocsQuery;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryVisitor;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.component.QueryComponent;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.search.ExtendedQueryBase;
+import org.apache.solr.search.QParser;
+import org.apache.solr.search.QParserPlugin;
+import org.apache.solr.search.QueryCommand;
+import org.apache.solr.search.QueryParsing;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SyntaxError;
+
+public class CombineQParserPlugin extends QParserPlugin {
+ public static final String NAME = "combine";
+
+ @Override
+ public QParser createParser(
+ String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) {
+ return new CombineQParser(qstr, localParams, params, req);
+ }
+
+ static class CombineQParser extends QParser {
+
+ private List<String> unparsedQueries;
+ private QueriesCombiner queriesCombiningStrategy;
+ private List<QParser> queryParsers;
+ private List<Query> queries;
+
+ public CombineQParser(
+ String qstr, SolrParams localParams, SolrParams params, SolrQueryRequest req) {
+ super(qstr, localParams, params, req);
+ }
+
+ @Override
+ public Query parse() throws SyntaxError {
+
+ var queriesToCombineKeys = localParams.getParams("keys");
+ if (queriesToCombineKeys == null) {
+ queriesToCombineKeys = params.getParams(CombinerParams.COMBINER_KEYS);
+ }
+
+ unparsedQueries = new ArrayList<>(queriesToCombineKeys.length);
+ queryParsers = new ArrayList<>(queriesToCombineKeys.length);
+ queries = new ArrayList<>(queriesToCombineKeys.length);
+
+ // nocommit blend localParams and params without needless suffix in local
+ var blendParams = SolrParams.wrapDefaults(localParams, params);
+ queriesCombiningStrategy = QueriesCombiner.getImplementation(blendParams);
+
+ String defType = blendParams.get(QueryParsing.DEFTYPE, DEFAULT_QTYPE);
Review Comment:
Maybe could support different `defType` for different keys, though bit of a niche thing that perhaps.
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1688454713
##########
solr/core/src/java/org/apache/solr/search/combining/QueriesCombiner.java:
##########
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import static org.apache.solr.common.params.CombinerParams.RECIPROCAl_RANK_FUSION;
+
+import java.io.IOException;
+import java.util.List;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+
+/**
+ * Combining considers two or more query rankedLists: resultA, resultB ...<br>
+ * For a given query, each query result is a ranked list of documents La = (a1,a2,...), Lb = (b1,
+ * b2, ...)...<br>
+ * A combining algorithm creates a unique ranked list I = (i1, i2, ...).<br>
+ * This list is created by combining elements from the lists la and lb as described by the
+ * implementation algorithm.<br>
+ * since @Version 9.7 Used by {@link org.apache.solr.handler.component.QueryComponent}
+ */
+public abstract class QueriesCombiner {
+
+ protected int upTo;
+
+ public QueriesCombiner(SolrParams requestParams) {
+ this.upTo =
+ requestParams.getInt(CombinerParams.COMBINER_UP_TO, CombinerParams.COMBINER_UP_TO_DEFAULT);
+ }
+
+ public abstract QueryResult combine(QueryResult[] rankedLists);
+
+ protected QueryResult initCombinedResult(QueryResult[] rankedLists) {
+ QueryResult combinedRankedList = new QueryResult();
+ boolean partialResults = false;
+ for (QueryResult result : rankedLists) {
+ partialResults |= result.isPartialResults();
+ }
+ combinedRankedList.setPartialResults(partialResults);
+
+ boolean segmentTerminatedEarly = false;
+ for (QueryResult result : rankedLists) {
+ if (result.getSegmentTerminatedEarly() != null) {
+ segmentTerminatedEarly |= result.getSegmentTerminatedEarly();
+ }
+ }
+ combinedRankedList.setSegmentTerminatedEarly(segmentTerminatedEarly);
+
+ combinedRankedList.setNextCursorMark(rankedLists[0].getNextCursorMark());
Review Comment:
Not sure about picking the first list here and/or what does use of a cursor mark mean when combining queries. Alternative might be to just disallow cursor marks when combining queries.
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1632230094
##########
solr/solrj/src/java/org/apache/solr/common/params/CombinerParams.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.params;
+
+/** Defines the request parameters used by all combiners. */
+public interface CombinerParams {
+ String COMBINER = "combiner";
+
+ String COMBINER_ALGORITHM = COMBINER + ".algorithm";
+
+ String COMBINER_KEYS = COMBINER + ".keys";
Review Comment:
mmmmm, I do believe it can be useful to have a "query key", especially because the debug component explains both the query parsing and combination:
0.032522473 =
1/(60+1) + 1/(60+2) because its ranks were: 1 for query(lexical), 2 for query(lexical2)
So the keys help in my opinion.
If you were referring to this specific parameter, this is not intended for the user, it gets used automatically by Solr.
Happy to elaborate!
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1688437190
##########
solr/solrj/src/java/org/apache/solr/common/params/CombinerParams.java:
##########
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.common.params;
+
+/** Defines the request parameters used by all combiners. */
+public interface CombinerParams {
+ String COMBINER = "combiner";
+
+ String COMBINER_ALGORITHM = COMBINER + ".algorithm";
+
+ String COMBINER_KEYS = COMBINER + ".keys";
Review Comment:
> ... `combine.q` multi-valued ...
In the case of multi-valued parameters, is ordering 'guaranteed' or could `&combine.q=Foo&combine.q=Bar` turn into `&combine.q=Bar&combine.q=Foo` and if it did would it matter?
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2605986155
This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the dev@solr.apache.org mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution!
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2153348715
> I should have spent just another 10 minutes reading this to understand what's going on; I understand it better now. I had overlooked the query loop. I don't think a new Query would be that helpul for this use-case after-all. Even if one were to exist, _somewhere_ there would be code to detect it and loop it so _where_ would that go... either QueryComponent or SolrIndexSearcher which is another complexity minefield of greater magnitude. It's tempting to think of a QueryComponent subclass for this use-case (and for that matter grouping).
>
> BTW I like the query syntax, especially the JSON variant.
>
> Sorry for any distress; +1 overall.
No distress David! I'm not the kind of developer that get attached to its own solutions.
If there are better ways I'm always open to suggestions.
I also agree in principle with proposing ideas first, before spending days inplementing them, the only reason that I don't do that often is because I am not full time on Solr contributions right now, actually I end up at the moment with limited time to do them, so I prefer to produce something that could be valuable and iterate.
Having to wait for other volunteers time to discuss the idea could end up in exhausting the time I had allocated for the contrib and have to wait for the next availability window between clients.
Worst case scenario we end up with a discussion and we may ignore the Pull Request.
I remember at the beginning I tried query parsing and other approaches but the cleanest I got was the loop in the query component.
And I absolutely agree that component is messy, so after Berlin I'll spend a bit more time polishing the changes, and if in the meantime we come up with better ideas, the better!
No rush to merge, but at the same time if we get a decent consensus I would love to have this in as this feature will open many doors!
Thanks for the time you already spent on this, it's always very much appreciated!
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1630193292
##########
solr/core/src/test/org/apache/solr/handler/component/DebugComponentTest.java:
##########
@@ -52,7 +64,7 @@ public void testBasicInterface() {
"//str[@name='querystring']='*:*'",
"//str[@name='parsedquery']='MatchAllDocsQuery(*:*)'",
"//str[@name='parsedquery_toString']='*:*'",
- "count(//lst[@name='explain']/*)=3",
+ "count(//lst[@name='explain']/*)=6",
Review Comment:
I added more docs in the setup of the test class, for the new debug tests I added.
I thought that changing the few lines of debug that were counting the total docs was not much of a big deal.
I can obviously change that if you think it is worth.
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "renatoh (via GitHub)" <gi...@apache.org>.
renatoh commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1624657341
##########
solr/core/src/java/org/apache/solr/search/combining/ReciprocalRankFusion.java:
##########
@@ -0,0 +1,208 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import static org.apache.lucene.search.TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocIterator;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SortedIntDocSet;
+
+/**
+ * Reciprocal Rank Fusion (RRF) is an algorithm that takes in input multiple ranked lists to produce
+ * a unified result set. Examples of use cases where RRF can be used include hybrid search and
+ * multiple Knn vector queries executed concurrently. RRF is based on the concept of reciprocal
+ * rank, which is the inverse of the rank of a document in a ranked list of search results. The
+ * combination of search results happens taking into account the position of the items in the
+ * original rankings, and giving higher score to items that are ranked higher in multiple lists. RRF
+ * was introduced the first time by Cormack et al. in [1].<br>
+ *
+ * <p>[1] Cormack, Gordon V. et al. “Reciprocal rank fusion outperforms condorcet and individual
+ * rank learning methods.” Proceedings of the 32nd international ACM SIGIR conference on Research
+ * and development in information retrieval (2009)<br>
+ */
+public class ReciprocalRankFusion extends QueriesCombiner {
+ int k;
+
+ public ReciprocalRankFusion(SolrParams requestParams) {
+ super(requestParams);
+ this.k =
+ requestParams.getInt(CombinerParams.COMBINER_RRF_K, CombinerParams.COMBINER_RRF_K_DEFAULT);
+ }
+
+ @Override
+ public QueryResult combine(QueryResult[] rankedLists) {
+ QueryResult combinedRankedList = initCombinedResult(rankedLists);
+ List<DocList> docLists = new ArrayList<>(rankedLists.length);
+ for (QueryResult rankedList : rankedLists) {
+ docLists.add(rankedList.getDocList());
+ }
+ combineResults(combinedRankedList, docLists, false);
+ return combinedRankedList;
+ }
+
+ private Map<Integer, Integer[]> combineResults(
+ QueryResult combinedRankedList,
+ List<DocList> rankedLists,
+ boolean saveRankPositionsForExplain) {
+ Map<Integer, Integer[]> docIdToRanks = null;
+ HashMap<Integer, Float> docIdToScore = new HashMap<>();
+ for (DocList rankedList : rankedLists) {
+ DocIterator docs = rankedList.iterator();
+ int ranking = 1;
+ while (docs.hasNext() && ranking <= upTo) {
+ int docId = docs.nextDoc();
+ float rrfScore = 1f / (k + ranking);
+ docIdToScore.compute(docId, (id, score) -> (score == null) ? rrfScore : score + rrfScore);
+ ranking++;
+ }
+ }
+ Stream<Map.Entry<Integer, Float>> sortedByScoreDescending =
+ docIdToScore.entrySet().stream()
+ .sorted(Collections.reverseOrder(Map.Entry.comparingByValue()));
+
+ int combinedResultsLength = docIdToScore.size();
+ int[] combinedResultsDocIds = new int[combinedResultsLength];
+ float[] combinedResultScores = new float[combinedResultsLength];
+
+ int i = 0;
+ for (Map.Entry<Integer, Float> scoredDoc :
+ sortedByScoreDescending.collect(Collectors.toList())) {
+ combinedResultsDocIds[i] = scoredDoc.getKey();
+ combinedResultScores[i] = scoredDoc.getValue();
+ i++;
+ }
+
+ if (saveRankPositionsForExplain) {
+ docIdToRanks = getRanks(rankedLists, combinedResultsDocIds);
+ }
+
+ DocSlice combinedResultSlice =
+ new DocSlice(
+ 0,
+ combinedResultsLength,
+ combinedResultsDocIds,
+ combinedResultScores,
+ combinedResultsLength,
+ combinedResultScores[0],
+ GREATER_THAN_OR_EQUAL_TO);
+ combinedRankedList.setDocList(combinedResultSlice);
+ SortedIntDocSet docSet = new SortedIntDocSet(combinedResultsDocIds, combinedResultsLength);
+ combinedRankedList.setDocSet(docSet);
+
+ return docIdToRanks;
+ }
+
+ private Map<Integer, Integer[]> getRanks(List<DocList> docLists, int[] combinedResultsDocIDs) {
+ Map<Integer, Integer[]> docIdToRanks;
+ docIdToRanks = new HashMap<>();
+ for (int docID : combinedResultsDocIDs) {
+ docIdToRanks.put(docID, new Integer[docLists.size()]);
Review Comment:
oh dear, I completely missed the[] and thought you are create a new Integer by doing: new Integer() and therefore preventing them from being cached. sorry, my bad!
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "renatoh (via GitHub)" <gi...@apache.org>.
renatoh commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1623870400
##########
solr/core/src/java/org/apache/solr/search/combining/ReciprocalRankFusion.java:
##########
@@ -0,0 +1,208 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import static org.apache.lucene.search.TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocIterator;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SortedIntDocSet;
+
+/**
+ * Reciprocal Rank Fusion (RRF) is an algorithm that takes in input multiple ranked lists to produce
+ * a unified result set. Examples of use cases where RRF can be used include hybrid search and
+ * multiple Knn vector queries executed concurrently. RRF is based on the concept of reciprocal
+ * rank, which is the inverse of the rank of a document in a ranked list of search results. The
+ * combination of search results happens taking into account the position of the items in the
+ * original rankings, and giving higher score to items that are ranked higher in multiple lists. RRF
+ * was introduced the first time by Cormack et al. in [1].<br>
+ *
+ * <p>[1] Cormack, Gordon V. et al. “Reciprocal rank fusion outperforms condorcet and individual
+ * rank learning methods.” Proceedings of the 32nd international ACM SIGIR conference on Research
+ * and development in information retrieval (2009)<br>
+ */
+public class ReciprocalRankFusion extends QueriesCombiner {
+ int k;
+
+ public ReciprocalRankFusion(SolrParams requestParams) {
+ super(requestParams);
+ this.k =
+ requestParams.getInt(CombinerParams.COMBINER_RRF_K, CombinerParams.COMBINER_RRF_K_DEFAULT);
+ }
+
+ @Override
+ public QueryResult combine(QueryResult[] rankedLists) {
+ QueryResult combinedRankedList = initCombinedResult(rankedLists);
+ List<DocList> docLists = new ArrayList<>(rankedLists.length);
+ for (QueryResult rankedList : rankedLists) {
+ docLists.add(rankedList.getDocList());
+ }
+ combineResults(combinedRankedList, docLists, false);
+ return combinedRankedList;
+ }
+
+ private Map<Integer, Integer[]> combineResults(
+ QueryResult combinedRankedList,
+ List<DocList> rankedLists,
+ boolean saveRankPositionsForExplain) {
+ Map<Integer, Integer[]> docIdToRanks = null;
+ HashMap<Integer, Float> docIdToScore = new HashMap<>();
+ for (DocList rankedList : rankedLists) {
+ DocIterator docs = rankedList.iterator();
+ int ranking = 1;
+ while (docs.hasNext() && ranking <= upTo) {
+ int docId = docs.nextDoc();
+ float rrfScore = 1f / (k + ranking);
+ docIdToScore.compute(docId, (id, score) -> (score == null) ? rrfScore : score + rrfScore);
+ ranking++;
+ }
+ }
+ Stream<Map.Entry<Integer, Float>> sortedByScoreDescending =
+ docIdToScore.entrySet().stream()
+ .sorted(Collections.reverseOrder(Map.Entry.comparingByValue()));
+
+ int combinedResultsLength = docIdToScore.size();
+ int[] combinedResultsDocIds = new int[combinedResultsLength];
+ float[] combinedResultScores = new float[combinedResultsLength];
+
+ int i = 0;
+ for (Map.Entry<Integer, Float> scoredDoc :
+ sortedByScoreDescending.collect(Collectors.toList())) {
+ combinedResultsDocIds[i] = scoredDoc.getKey();
+ combinedResultScores[i] = scoredDoc.getValue();
+ i++;
+ }
+
+ if (saveRankPositionsForExplain) {
+ docIdToRanks = getRanks(rankedLists, combinedResultsDocIds);
+ }
+
+ DocSlice combinedResultSlice =
+ new DocSlice(
+ 0,
+ combinedResultsLength,
+ combinedResultsDocIds,
+ combinedResultScores,
+ combinedResultsLength,
+ combinedResultScores[0],
+ GREATER_THAN_OR_EQUAL_TO);
+ combinedRankedList.setDocList(combinedResultSlice);
+ SortedIntDocSet docSet = new SortedIntDocSet(combinedResultsDocIds, combinedResultsLength);
+ combinedRankedList.setDocSet(docSet);
+
+ return docIdToRanks;
+ }
+
+ private Map<Integer, Integer[]> getRanks(List<DocList> docLists, int[] combinedResultsDocIDs) {
+ Map<Integer, Integer[]> docIdToRanks;
+ docIdToRanks = new HashMap<>();
+ for (int docID : combinedResultsDocIDs) {
+ docIdToRanks.put(docID, new Integer[docLists.size()]);
Review Comment:
Any particular reason for not using Integer.valueOf()?
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on code in PR #2489:
URL: https://github.com/apache/solr/pull/2489#discussion_r1624531316
##########
solr/core/src/java/org/apache/solr/search/combining/ReciprocalRankFusion.java:
##########
@@ -0,0 +1,208 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.search.combining;
+
+import static org.apache.lucene.search.TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.StringJoiner;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocIterator;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SortedIntDocSet;
+
+/**
+ * Reciprocal Rank Fusion (RRF) is an algorithm that takes in input multiple ranked lists to produce
+ * a unified result set. Examples of use cases where RRF can be used include hybrid search and
+ * multiple Knn vector queries executed concurrently. RRF is based on the concept of reciprocal
+ * rank, which is the inverse of the rank of a document in a ranked list of search results. The
+ * combination of search results happens taking into account the position of the items in the
+ * original rankings, and giving higher score to items that are ranked higher in multiple lists. RRF
+ * was introduced the first time by Cormack et al. in [1].<br>
+ *
+ * <p>[1] Cormack, Gordon V. et al. “Reciprocal rank fusion outperforms condorcet and individual
+ * rank learning methods.” Proceedings of the 32nd international ACM SIGIR conference on Research
+ * and development in information retrieval (2009)<br>
+ */
+public class ReciprocalRankFusion extends QueriesCombiner {
+ int k;
+
+ public ReciprocalRankFusion(SolrParams requestParams) {
+ super(requestParams);
+ this.k =
+ requestParams.getInt(CombinerParams.COMBINER_RRF_K, CombinerParams.COMBINER_RRF_K_DEFAULT);
+ }
+
+ @Override
+ public QueryResult combine(QueryResult[] rankedLists) {
+ QueryResult combinedRankedList = initCombinedResult(rankedLists);
+ List<DocList> docLists = new ArrayList<>(rankedLists.length);
+ for (QueryResult rankedList : rankedLists) {
+ docLists.add(rankedList.getDocList());
+ }
+ combineResults(combinedRankedList, docLists, false);
+ return combinedRankedList;
+ }
+
+ private Map<Integer, Integer[]> combineResults(
+ QueryResult combinedRankedList,
+ List<DocList> rankedLists,
+ boolean saveRankPositionsForExplain) {
+ Map<Integer, Integer[]> docIdToRanks = null;
+ HashMap<Integer, Float> docIdToScore = new HashMap<>();
+ for (DocList rankedList : rankedLists) {
+ DocIterator docs = rankedList.iterator();
+ int ranking = 1;
+ while (docs.hasNext() && ranking <= upTo) {
+ int docId = docs.nextDoc();
+ float rrfScore = 1f / (k + ranking);
+ docIdToScore.compute(docId, (id, score) -> (score == null) ? rrfScore : score + rrfScore);
+ ranking++;
+ }
+ }
+ Stream<Map.Entry<Integer, Float>> sortedByScoreDescending =
+ docIdToScore.entrySet().stream()
+ .sorted(Collections.reverseOrder(Map.Entry.comparingByValue()));
+
+ int combinedResultsLength = docIdToScore.size();
+ int[] combinedResultsDocIds = new int[combinedResultsLength];
+ float[] combinedResultScores = new float[combinedResultsLength];
+
+ int i = 0;
+ for (Map.Entry<Integer, Float> scoredDoc :
+ sortedByScoreDescending.collect(Collectors.toList())) {
+ combinedResultsDocIds[i] = scoredDoc.getKey();
+ combinedResultScores[i] = scoredDoc.getValue();
+ i++;
+ }
+
+ if (saveRankPositionsForExplain) {
+ docIdToRanks = getRanks(rankedLists, combinedResultsDocIds);
+ }
+
+ DocSlice combinedResultSlice =
+ new DocSlice(
+ 0,
+ combinedResultsLength,
+ combinedResultsDocIds,
+ combinedResultScores,
+ combinedResultsLength,
+ combinedResultScores[0],
+ GREATER_THAN_OR_EQUAL_TO);
+ combinedRankedList.setDocList(combinedResultSlice);
+ SortedIntDocSet docSet = new SortedIntDocSet(combinedResultsDocIds, combinedResultsLength);
+ combinedRankedList.setDocSet(docSet);
+
+ return docIdToRanks;
+ }
+
+ private Map<Integer, Integer[]> getRanks(List<DocList> docLists, int[] combinedResultsDocIDs) {
+ Map<Integer, Integer[]> docIdToRanks;
+ docIdToRanks = new HashMap<>();
+ for (int docID : combinedResultsDocIDs) {
+ docIdToRanks.put(docID, new Integer[docLists.size()]);
Review Comment:
To do what? Isn't Integer.valueOf(int a) used to wrap an integer?
Can you elaborate?
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2142359815
As of today the solution is clean enough to be reviewed and tests are green.
The usage is intuitive and it should cover many use cases.
Pending activities:
- add a new documentation page
- add tests for the distributed scenario
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "alessandrobenedetti (via GitHub)" <gi...@apache.org>.
alessandrobenedetti commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2174445957
> I did the work of migrating to a QParser but don't have permissions to push to your PR branch.
>
> Beware, I'm reverting most/all of the files that you modified of existing Solr source files. If you would prefer I submit a separate PR, I'll do that. Instead of `combiner=true`, you'd do `q={!combine}`.
I don't remember if this is the best way to invite you as a collaborator but I just added you on our repo, hopefully that works!
I wasn't entirely sure a Query Parser is the right place for this, given we are not really building a "new query" but then I thought : "hey we have the boolean query parser, that more or less does the same", so I have nothing against it!
You can go ahead and add your commit, I don't care much if you overwrote many of my changes, I just want a nice and clean solution, however we come up to it! Worst case scenario we go back in commits and re-use what's necessary!
>
> Wouldn't the docSet (assuming it's needed, like for faceting) be computed on each subQuery, and if so how is that rolled up to the final QueryResult? I see you tested that faceting works but I suspect there may be a very sad performance bug here.
In regards to the docSet It was rolled up in:
`SortedIntDocSet docSet = new SortedIntDocSet(combinedResultsDocIds, combinedResultsLength);
combinedRankedList.setDocSet(docSet);`
combinedResultsDocIds is the merged array of doc Ids.
I admit I haven't spent the required time on this part and it was quickly done as a draft, so feel free to recommend a different way/correct me!
--
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
Re: [PR] Introduce support for Reciprocal Rank Fusion (combining queries) [solr]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2489:
URL: https://github.com/apache/solr/pull/2489#issuecomment-2746563527
This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again.
--
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