alessandrobenedetti commented on a change in pull request #1571:
URL: https://github.com/apache/lucene-solr/pull/1571#discussion_r520509933
##########
File path:
solr/contrib/ltr/src/java/org/apache/solr/ltr/response/transform/LTRFeatureLoggerTransformerFactory.java
##########
@@ -210,50 +216,59 @@ public void setContext(ResultContext context) {
}
// Setup LTRScoringQuery
- scoringQuery = SolrQueryRequestContextUtils.getScoringQuery(req);
- docsWereNotReranked = (scoringQuery == null);
- String featureStoreName =
SolrQueryRequestContextUtils.getFvStoreName(req);
- if (docsWereNotReranked || (featureStoreName != null &&
(!featureStoreName.equals(scoringQuery.getScoringModel().getFeatureStoreName()))))
{
- // if store is set in the transformer we should overwrite the logger
-
- final ManagedFeatureStore fr =
ManagedFeatureStore.getManagedFeatureStore(req.getCore());
-
- final FeatureStore store = fr.getFeatureStore(featureStoreName);
- featureStoreName = store.getName(); // if featureStoreName was null
before this gets actual name
-
- try {
- final LoggingModel lm = new LoggingModel(loggingModelName,
- featureStoreName, store.getFeatures());
-
- scoringQuery = new LTRScoringQuery(lm,
- LTRQParserPlugin.extractEFIParams(localparams),
- true,
- threadManager); // request feature weights to be created for all
features
-
- }catch (final Exception e) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
- "retrieving the feature store "+featureStoreName, e);
- }
- }
+ rerankingQueries = SolrQueryRequestContextUtils.getScoringQueries(req);
- if (scoringQuery.getOriginalQuery() == null) {
- scoringQuery.setOriginalQuery(context.getQuery());
+ docsWereNotReranked = (rerankingQueries == null ||
rerankingQueries.length == 0);
+ if (docsWereNotReranked) {
+ rerankingQueries = new LTRScoringQuery[]{null};
}
- if (scoringQuery.getFeatureLogger() == null){
- scoringQuery.setFeatureLogger(
SolrQueryRequestContextUtils.getFeatureLogger(req) );
- }
- scoringQuery.setRequest(req);
-
- featureLogger = scoringQuery.getFeatureLogger();
+ modelWeights = new LTRScoringQuery.ModelWeight[rerankingQueries.length];
+ String featureStoreName =
SolrQueryRequestContextUtils.getFvStoreName(req);
+ for (int i = 0; i < rerankingQueries.length; i++) {
+ LTRScoringQuery scoringQuery = rerankingQueries[i];
+ if ((scoringQuery == null || !(scoringQuery instanceof
OriginalRankingLTRScoringQuery)) && (docsWereNotReranked || (featureStoreName
!= null &&
!featureStoreName.equals(scoringQuery.getScoringModel().getFeatureStoreName()))))
{
Review comment:
Ok, I have done an extensive refactor of this bit, following your
scenarios guideline, I believe the code is much more readable now.
I added few tests as well.
Thank you very much for the insight, now that part is extremely clear.
Before resolving this discussion, working on this, another consideration
sparkled:
Currently when we use the feature logger transformer, all features are
extracted (even the ones not used by the reranking model, if any).
Are we sure we want this behavior ?
We could re-use the extracted feature vector cached also for logging if we
just log the features actually used by the model.
I am just wondering why I would be interested in logging for a document, all
the features in a featureStore, including potentially features just used by
other models.
This could actually lead to confusion.
If you agree I create a separate Jira for that and I'll implement a solution
soon, to avoid to forget and context switch.
----------------------------------------------------------------
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
