cpoerschke commented on a change in pull request #1571:
URL: https://github.com/apache/lucene-solr/pull/1571#discussion_r498948018
##########
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:
Ah, good point about the special "OriginalRanking" also appearing in the
"[interleaving]" transformer!
When using interleaving there's always at least two models to be
interleaved, right?
The models could all be actual models or one of them could be the
"OriginalRanking" pseudo-model.
I wonder if class inheritance might help us e.g.
```
class InterleavingLTRQParserPlugin extends LTRQParserPlugin
```
and (say)
```
<queryParser name="ltr" class="org.apache.solr.ltr.search.LTRQParserPlugin"/>
<queryParser name="iltr"
class="org.apache.solr.ltr.search.InterleavingLTRQParserPlugin"/>
```
where
```
rq={!iltr model=myModelXYZ}
```
can be a convenience equivalent to
```
rq={!iltr model=OriginalRanking model=myModelXYZ}
```
i.e. if only one model is supplied then it is implied that the second model
is the original ranking.
And if the special "OriginalRanking" name doesn't suit someone (either
because they already have a real model that happens to be called
"OriginalRanking" or because they would prefer a different descriptor in the
"[interleaving]" transformer output) then something like
```
rq={!iltr originalRankingModel=noModel model=noModel model=someModel}
```
would allow them to call the "OriginalRanking" something else e.g. "noModel"
instead.
We could even reject any "OriginalRanking" models that are actual models via
something like
```
final String originalRankingModelName =
localParams.get("originalRankingModel", "OriginalRanking" /* default */);
if (null != mr.getModel(originalRankingModelName)) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"Found an actual '" + originalRankingModelName +
"' model, please ...
}
```
----
However, the "[interleaving]" transformer still needs to know about the
special name, hmm.
At the moment we have
```
public static boolean isOriginalRanking(LTRScoringQuery rerankingQuery){
return rerankingQuery.getScoringModel() == null;
}
```
in LTRQParserPlugin and in LTRInterleavingTransformerFactory
```
if (isOriginalRanking(rerankingQuery)) {
doc.addField(name, ORIGINAL_RANKING);
} else {
doc.addField(name, rerankingQuery.getScoringModel().getName());
}
```
and if we gave LTRScoringQuery a getScoringModelName method
```
public String getScoringModelName() {
return ltrScoringModel.getName();
}
```
then in LTRInterleavingTransformerFactory we could have
```
doc.addField(name, rerankingQuery.getScoringModelName());
```
if the
```
new LTRScoringQuery(null);
```
in LTRQParserPlugin becomes
```
new LTRScoringQuery(null) {
@Override
public String getScoringModelName() {
return originalRankingModelName;
}
};
```
but that wouldn't displace any other `isOriginalRanking(rerankingQuery)`
calls, though if we had something like
```
final class OriginalRankingLTRScoringQuery extends LTRScoringQuery {
private final String originalRankingModelName;
public OriginalRankingLTRScoringQuery(String originalRankingModelName) {
super(null /* LTRScoringModel */);
this.originalRankingModelName = originalRankingModelName;
}
@Override
public String getScoringModelName() {
return this.originalRankingModelName;
}
}
```
then `(rerankingQuery instanceof OriginalRankingLTRScoringQuery)` could
replace `isOriginalRanking(rerankingQuery)` conceptually.
----
Just thinking out aloud here, haven't yet tried to turn any of the above
into code.
Could you see inheritance (for LTRQParserPlugin and/or LTRScoringQuery) work
potentially?
----------------------------------------------------------------
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
