alessandrobenedetti commented on a change in pull request #1571:
URL: https://github.com/apache/lucene-solr/pull/1571#discussion_r500956870
##########
File path:
solr/contrib/ltr/src/java/org/apache/solr/ltr/search/LTRQParserPlugin.java
##########
@@ -146,93 +149,114 @@ public LTRQParser(String qstr, SolrParams localParams,
SolrParams params,
@Override
public Query parse() throws SyntaxError {
// ReRanking Model
- final String modelName = localParams.get(LTRQParserPlugin.MODEL);
- if ((modelName == null) || modelName.isEmpty()) {
+ final String[] modelNames =
localParams.getParams(LTRQParserPlugin.MODEL);
+ if ((modelNames == null) || modelNames.length==0 ||
modelNames[0].isEmpty()) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"Must provide model in the request");
}
-
- final LTRScoringModel ltrScoringModel = mr.getModel(modelName);
- if (ltrScoringModel == null) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
- "cannot find " + LTRQParserPlugin.MODEL + " " + modelName);
- }
-
- final String modelFeatureStoreName =
ltrScoringModel.getFeatureStoreName();
- final boolean extractFeatures =
SolrQueryRequestContextUtils.isExtractingFeatures(req);
- final String fvStoreName =
SolrQueryRequestContextUtils.getFvStoreName(req);
- // Check if features are requested and if the model feature store and
feature-transform feature store are the same
- final boolean featuresRequestedFromSameStore =
(modelFeatureStoreName.equals(fvStoreName) || fvStoreName == null) ?
extractFeatures:false;
- if (threadManager != null) {
-
threadManager.setExecutor(req.getCore().getCoreContainer().getUpdateShardHandler().getUpdateExecutor());
- }
- final LTRScoringQuery scoringQuery = new LTRScoringQuery(ltrScoringModel,
- extractEFIParams(localParams),
- featuresRequestedFromSameStore, threadManager);
-
- // Enable the feature vector caching if we are extracting features, and
the features
- // we requested are the same ones we are reranking with
- if (featuresRequestedFromSameStore) {
- scoringQuery.setFeatureLogger(
SolrQueryRequestContextUtils.getFeatureLogger(req) );
+
+ LTRScoringQuery[] rerankingQueries = new
LTRScoringQuery[modelNames.length];
+ for (int i = 0; i < modelNames.length; i++) {
+ final LTRScoringQuery rerankingQuery;
+ if (!ORIGINAL_RANKING.equals(modelNames[i])) {
Review comment:
So @cpoerschke , I made a step back with the latest commit, let me
explain,
First of all, the approach you detailed can definitely work,
I don't want to undervalue your effort and contribution, I am actually quite
grateful for the time you dedicated in reviewing my proposal, so I really
appreciate that.
But let's aim to a Keep It Simple Stupid approach here (KISS) and avoid
over-complication in a first release, if possible:
1) Apache Solr makes use of "special names" in various places :
- "_DEFAULT_" to indicate the default feature/model store
- "_version" to indicate a meta-field used for versioning/optimistic
concurrency
ect
I notice the prefix and suffix '_' is used for them.
So let's use it : "_OriginalRanking_" could be a fair name to identify the
Apache Solr ranking pre-re-ranking.
I named it after the "OriginalScore" feature, already in place, happy to use
a different name, but wouldn't spend to much time on it.
My proposal would be to use "_OriginalRanking_" or potentially
"_OriginalScoreRanking_" if more readable
2) the user interact with the !ltr query parser as usual, and if he/she
wants to use interleaving, he/she just passes a second model (yes, we are
supporting only 2 interleaved model right now):
model=A model=B
If he/she wants to interleave with the original ranking:
model=A model=_OriginalRanking_
I think this is quite intuitive and easy to be used, quite explicit and
following current Apache Solr nomenclature for special named items
3) what happens if a user had/wants to use "_OriginalRanking_" for one of
his/her models?
In this unlikely event, a Jira issue can be raised and we design a possible
solution at that time.
A possible simple approach could be to make the original ranking name
parametric, so the user can specify the name in the solrconfig.xml
But I would delay this reasoning only if we get requests for that
functionality from users
What do you think?
If we find a good compromise here, we could move the review to the
re-scoring part, that is more delicate and we can make sure no mistake happened
there (I definitely don't want to slow down normal re-ranking and I definitely
want the interleaved re-ranking to be as fast as possible)
You find the latest changes in the latest commits, I will update the related
documentation as well today.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
