You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/06/23 18:38:48 UTC
[GitHub] [solr] madrob commented on a change in pull request #189: SOLR-14920: contrib/ltr/src/java: apply & enforce 'spotless' code formatting
madrob commented on a change in pull request #189:
URL: https://github.com/apache/solr/pull/189#discussion_r657360768
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRRescorer.java
##########
@@ -186,18 +191,40 @@ public void scoreFeatures(IndexSearcher indexSearcher,
docBase = readerContext.docBase;
scorer = modelWeight.scorer(readerContext);
}
- scoreSingleHit(indexSearcher, topN, modelWeight, docBase, hitUpto, hit, docID, scoringQuery, scorer, reranked);
+ scoreSingleHit(
Review comment:
ugh, these are both bad, not sure what I think we should do here.
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRRescorer.java
##########
@@ -34,16 +33,14 @@
import org.apache.solr.ltr.interleaving.OriginalRankingLTRScoringQuery;
import org.apache.solr.search.SolrIndexSearcher;
-
/**
- * Implements the rescoring logic. The top documents returned by solr with their
- * original scores, will be processed by a {@link LTRScoringQuery} that will assign a
- * new score to each document. The top documents will be resorted based on the
- * new score.
- * */
+ * Implements the rescoring logic. The top documents returned by solr with their original scores,
+ * will be processed by a {@link LTRScoringQuery} that will assign a new score to each document. The
+ * top documents will be resorted based on the new score.
+ */
public class LTRRescorer extends Rescorer {
- final private LTRScoringQuery scoringQuery;
+ private final LTRScoringQuery scoringQuery;
Review comment:
didn't realize this also did modifier ordering. happy to see it though.
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/FeatureLogger.java
##########
@@ -36,46 +39,42 @@ protected FeatureLogger(String fvCacheName, FeatureFormat f) {
}
/**
- * Log will be called every time that the model generates the feature values
- * for a document and a query.
+ * Log will be called every time that the model generates the feature values for a document and a
+ * query.
*
- * @param docid
- * Solr document id whose features we are saving
- * @param featuresInfo
- * List of all the {@link LTRScoringQuery.FeatureInfo} objects which contain name and value
- * for all the features triggered by the result set
- * @return true if the logger successfully logged the features, false
- * otherwise.
+ * @param docid Solr document id whose features we are saving
+ * @param featuresInfo List of all the {@link LTRScoringQuery.FeatureInfo} objects which contain
+ * name and value for all the features triggered by the result set
+ * @return true if the logger successfully logged the features, false otherwise.
*/
-
- public boolean log(int docid, LTRScoringQuery scoringQuery,
- SolrIndexSearcher searcher, LTRScoringQuery.FeatureInfo[] featuresInfo) {
+ public boolean log(
+ int docid,
+ LTRScoringQuery scoringQuery,
+ SolrIndexSearcher searcher,
+ LTRScoringQuery.FeatureInfo[] featuresInfo) {
final String featureVector = makeFeatureVector(featuresInfo);
if (featureVector == null) {
return false;
}
- return searcher.cacheInsert(fvCacheName,
- fvCacheKey(scoringQuery, docid), featureVector) != null;
+ return searcher.cacheInsert(fvCacheName, fvCacheKey(scoringQuery, docid), featureVector)
+ != null;
Review comment:
I don't like this
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LTRScoringModel.java
##########
@@ -113,74 +122,78 @@ public static LTRScoringModel getInstance(SolrResourceLoader solrResourceLoader,
return model;
}
- public LTRScoringModel(String name, List<Feature> features,
+ public LTRScoringModel(
+ String name,
+ List<Feature> features,
List<Normalizer> norms,
- String featureStoreName, List<Feature> allFeatures,
- Map<String,Object> params) {
+ String featureStoreName,
+ List<Feature> allFeatures,
+ Map<String, Object> params) {
this.name = name;
- this.features = features != null ? Collections.unmodifiableList(new ArrayList<>(features)) : null;
+ this.features =
+ features != null ? Collections.unmodifiableList(new ArrayList<>(features)) : null;
this.featureStoreName = featureStoreName;
- this.allFeatures = allFeatures != null ? Collections.unmodifiableList(new ArrayList<>(allFeatures)) : null;
+ this.allFeatures =
+ allFeatures != null ? Collections.unmodifiableList(new ArrayList<>(allFeatures)) : null;
this.params = params != null ? Collections.unmodifiableMap(new LinkedHashMap<>(params)) : null;
this.norms = norms != null ? Collections.unmodifiableList(new ArrayList<>(norms)) : null;
}
/**
- * Validate that settings make sense and throws
- * {@link ModelException} if they do not make sense.
+ * Validate that settings make sense and throws {@link ModelException} if they do not make sense.
Review comment:
surprised this didn't collapse into a single line
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRRescorer.java
##########
@@ -186,18 +191,40 @@ public void scoreFeatures(IndexSearcher indexSearcher,
docBase = readerContext.docBase;
scorer = modelWeight.scorer(readerContext);
}
- scoreSingleHit(indexSearcher, topN, modelWeight, docBase, hitUpto, hit, docID, scoringQuery, scorer, reranked);
+ scoreSingleHit(
+ indexSearcher,
+ topN,
+ modelWeight,
+ docBase,
+ hitUpto,
+ hit,
+ docID,
+ scoringQuery,
+ scorer,
+ reranked);
hitUpto++;
}
}
- protected static void scoreSingleHit(IndexSearcher indexSearcher, int topN, LTRScoringQuery.ModelWeight modelWeight, int docBase, int hitUpto, ScoreDoc hit, int docID, LTRScoringQuery rerankingQuery, LTRScoringQuery.ModelWeight.ModelScorer scorer, ScoreDoc[] reranked) throws IOException {
+ protected static void scoreSingleHit(
+ IndexSearcher indexSearcher,
+ int topN,
+ LTRScoringQuery.ModelWeight modelWeight,
+ int docBase,
+ int hitUpto,
+ ScoreDoc hit,
+ int docID,
+ LTRScoringQuery rerankingQuery,
+ LTRScoringQuery.ModelWeight.ModelScorer scorer,
+ ScoreDoc[] reranked)
+ throws IOException {
final FeatureLogger featureLogger = rerankingQuery.getFeatureLogger();
// Scorer for a LTRScoringQuery.ModelWeight should never be null since we always have to
Review comment:
Can we make a block comment here, I think that would collapse some of the line fragments better.
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/FeatureLogger.java
##########
@@ -19,15 +19,18 @@
import org.apache.solr.search.SolrIndexSearcher;
/**
- * FeatureLogger can be registered in a model and provide a strategy for logging
- * the feature values.
+ * FeatureLogger can be registered in a model and provide a strategy for logging the feature values.
*/
public abstract class FeatureLogger {
- /** the name of the cache using for storing the feature value **/
+ /** the name of the cache using for storing the feature value * */
Review comment:
I would manually change it to `*/`
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/interleaving/algorithms/TeamDraftInterleaving.java
##########
@@ -22,31 +22,31 @@
import java.util.LinkedList;
import java.util.Random;
import java.util.Set;
-
import org.apache.lucene.search.ScoreDoc;
import org.apache.solr.ltr.interleaving.Interleaving;
import org.apache.solr.ltr.interleaving.InterleavingResult;
/**
- * Interleaving was introduced the first time by Joachims in [1, 2].
- * Team Draft Interleaving is among the most successful and used interleaving approaches[3].
- * Team Draft Interleaving implements a method similar to the way in which captains select their players in team-matches.
- * Team Draft Interleaving produces a fair distribution of ranking models’ elements in the final interleaved list.
- * "Team draft interleaving" has also proved to overcome an issue of the "Balanced interleaving" approach, in determining the winning model[4].
- * <p>
- * [1] T. Joachims. Optimizing search engines using clickthrough data. KDD (2002)
- * [2] T.Joachims.Evaluatingretrievalperformanceusingclickthroughdata.InJ.Franke, G. Nakhaeizadeh, and I. Renz, editors,
- * Text Mining, pages 79–96. Physica/Springer (2003)
- * [3] F. Radlinski, M. Kurup, and T. Joachims. How does clickthrough data reflect re-
- * trieval quality? In CIKM, pages 43–52. ACM Press (2008)
- * [4] O. Chapelle, T. Joachims, F. Radlinski, and Y. Yue.
- * Large-scale validation and analysis of interleaved search evaluation. ACM TOIS, 30(1):1–41, Feb. (2012)
+ * Interleaving was introduced the first time by Joachims in [1, 2]. Team Draft Interleaving is
+ * among the most successful and used interleaving approaches[3]. Team Draft Interleaving implements
+ * a method similar to the way in which captains select their players in team-matches. Team Draft
+ * Interleaving produces a fair distribution of ranking models’ elements in the final interleaved
+ * list. "Team draft interleaving" has also proved to overcome an issue of the "Balanced
+ * interleaving" approach, in determining the winning model[4].
+ *
+ * <p>[1] T. Joachims. Optimizing search engines using clickthrough data. KDD (2002) [2]
+ * T.Joachims.Evaluatingretrievalperformanceusingclickthroughdata.InJ.Franke, G. Nakhaeizadeh, and
+ * I. Renz, editors, Text Mining, pages 79–96. Physica/Springer (2003) [3] F. Radlinski, M. Kurup,
+ * and T. Joachims. How does clickthrough data reflect re- trieval quality? In CIKM, pages 43–52.
+ * ACM Press (2008) [4] O. Chapelle, T. Joachims, F. Radlinski, and Y. Yue. Large-scale validation
+ * and analysis of interleaved search evaluation. ACM TOIS, 30(1):1–41, Feb. (2012)
*/
public class TeamDraftInterleaving implements Interleaving {
public static Random RANDOM;
static {
- // We try to make things reproducible in the context of our tests by initializing the random instance
+ // We try to make things reproducible in the context of our tests by initializing the random
+ // instance
Review comment:
combine with next line
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/OriginalScoreFeature.java
##########
@@ -28,48 +27,56 @@
import org.apache.lucene.search.Weight;
import org.apache.solr.ltr.DocInfo;
import org.apache.solr.request.SolrQueryRequest;
+
/**
- * This feature returns the original score that the document had before performing
- * the reranking.
+ * This feature returns the original score that the document had before performing the reranking.
* Example configuration:
+ *
* <pre>{
- "name": "originalScore",
- "class": "org.apache.solr.ltr.feature.OriginalScoreFeature",
- "params": { }
-}</pre>
- **/
+ * "name": "originalScore",
+ * "class": "org.apache.solr.ltr.feature.OriginalScoreFeature",
+ * "params": { }
+ * }</pre>
+ */
public class OriginalScoreFeature extends Feature {
- public OriginalScoreFeature(String name, Map<String,Object> params) {
+ public OriginalScoreFeature(String name, Map<String, Object> params) {
super(name, params);
}
@Override
- public LinkedHashMap<String,Object> paramsToMap() {
+ public LinkedHashMap<String, Object> paramsToMap() {
return defaultParamsToMap();
}
@Override
- protected void validate() throws FeatureException {
- }
+ protected void validate() throws FeatureException {}
@Override
- public OriginalScoreWeight createWeight(IndexSearcher searcher,
- boolean needsScores, SolrQueryRequest request, Query originalQuery, Map<String,String[]> efi) throws IOException {
+ public OriginalScoreWeight createWeight(
+ IndexSearcher searcher,
+ boolean needsScores,
+ SolrQueryRequest request,
+ Query originalQuery,
+ Map<String, String[]> efi)
+ throws IOException {
return new OriginalScoreWeight(searcher, request, originalQuery, efi);
-
}
public class OriginalScoreWeight extends FeatureWeight {
final Weight w;
- public OriginalScoreWeight(IndexSearcher searcher,
- SolrQueryRequest request, Query originalQuery, Map<String,String[]> efi) throws IOException {
+ public OriginalScoreWeight(
+ IndexSearcher searcher,
+ SolrQueryRequest request,
+ Query originalQuery,
+ Map<String, String[]> efi)
+ throws IOException {
super(OriginalScoreFeature.this, searcher, request, originalQuery, efi);
w = searcher.createWeight(searcher.rewrite(originalQuery), ScoreMode.COMPLETE, 1);
- };
-
+ }
+ ;
Review comment:
is this an extra semicolon?
##########
File path: solr/contrib/ltr/src/java/org/apache/solr/ltr/LTRRescorer.java
##########
@@ -118,52 +112,63 @@ public TopDocs rescore(IndexSearcher searcher, TopDocs firstPassTopDocs,
return new TopDocs(firstPassTopDocs.totalHits, reranked);
}
- private ScoreDoc[] rerank(IndexSearcher searcher, int topN, ScoreDoc[] firstPassResults) throws IOException {
+ private ScoreDoc[] rerank(IndexSearcher searcher, int topN, ScoreDoc[] firstPassResults)
+ throws IOException {
final ScoreDoc[] reranked = new ScoreDoc[topN];
final List<LeafReaderContext> leaves = searcher.getIndexReader().leaves();
- final LTRScoringQuery.ModelWeight modelWeight = (LTRScoringQuery.ModelWeight) searcher
- .createWeight(searcher.rewrite(scoringQuery), ScoreMode.COMPLETE, 1);
+ final LTRScoringQuery.ModelWeight modelWeight =
+ (LTRScoringQuery.ModelWeight)
+ searcher.createWeight(searcher.rewrite(scoringQuery), ScoreMode.COMPLETE, 1);
- scoreFeatures(searcher,topN, modelWeight, firstPassResults, leaves, reranked);
+ scoreFeatures(searcher, topN, modelWeight, firstPassResults, leaves, reranked);
// Must sort all documents that we reranked, and then select the top
sortByScore(reranked);
return reranked;
}
protected static void sortByScore(ScoreDoc[] reranked) {
- Arrays.sort(reranked, new Comparator<ScoreDoc>() {
- @Override
- public int compare(ScoreDoc a, ScoreDoc b) {
- // Sort by score descending, then docID ascending:
- if (a.score > b.score) {
- return -1;
- } else if (a.score < b.score) {
- return 1;
- } else {
- // This subtraction can't overflow int
- // because docIDs are >= 0:
- return a.doc - b.doc;
- }
- }
- });
+ Arrays.sort(
+ reranked,
+ new Comparator<ScoreDoc>() {
Review comment:
probably outside of scope here, but we could make a static instance of this comparator and reuse it.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org