tomglk commented on a change in pull request #123:
URL: https://github.com/apache/solr/pull/123#discussion_r633071371



##########
File path: 
solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldValueFeature.java
##########
@@ -70,86 +78,92 @@ public void after() throws Exception {
 
   @Test
   public void testRanking() throws Exception {
-
-    final SolrQuery query = new SolrQuery();
-    query.setQuery("title:w1");
-    query.add("fl", "*, score");
-    query.add("rows", "4");
-
-    // Normal term match
-    assertJQ("/query" + query.toQueryString(), "/response/numFound/==4");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[0]/id=='1'");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[1]/id=='8'");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[2]/id=='6'");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[3]/id=='7'");
-
-    query.add("rq", "{!ltr model=popularity-model reRankDocs=4}");
-
-    assertJQ("/query" + query.toQueryString(), "/response/numFound/==4");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[0]/id=='8'");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[1]/id=='7'");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[2]/id=='6'");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[3]/id=='1'");
-
-    query.setQuery("*:*");
-    query.remove("rows");
-    query.add("rows", "8");
-    query.remove("rq");
-    query.add("rq", "{!ltr model=popularity-model reRankDocs=8}");
-
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[0]/id=='8'");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[1]/id=='7'");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[2]/id=='6'");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[3]/id=='5'");
+    for (String field : FIELD_NAMES) {
+
+      final SolrQuery query = new SolrQuery();
+      query.setQuery("title:w1");
+      query.add("fl", "*, score");
+      query.add("rows", "4");
+
+      // Normal term match
+      assertJQ("/query" + query.toQueryString(), "/response/numFound/==4");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[0]/id=='1'");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[1]/id=='8'");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[2]/id=='6'");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[3]/id=='7'");
+
+      query.add("rq", "{!ltr model="+field+"-model reRankDocs=4}");
+
+      assertJQ("/query" + query.toQueryString(), "/response/numFound/==4");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[0]/id=='8'");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[1]/id=='7'");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[2]/id=='6'");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[3]/id=='1'");
+
+      query.setQuery("*:*");
+      query.remove("rows");
+      query.add("rows", "8");
+      query.remove("rq");
+      query.add("rq", "{!ltr model="+field+"-model reRankDocs=8}");
+
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[0]/id=='8'");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[1]/id=='7'");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[2]/id=='6'");
+      assertJQ("/query" + query.toQueryString(), "/response/docs/[3]/id=='5'");
+    }
   }
 
 
   @Test
   public void testIfADocumentDoesntHaveAFieldDefaultValueIsReturned() throws 
Exception {
-    SolrQuery query = new SolrQuery();
-    query.setQuery("id:42");
-    query.add("fl", "*, score");
-    query.add("rows", "4");
-
-    assertJQ("/query" + query.toQueryString(), "/response/numFound/==1");
-    assertJQ("/query" + query.toQueryString(), "/response/docs/[0]/id=='42'");
-    query = new SolrQuery();
-    query.setQuery("id:42");
-    query.add("rq", "{!ltr model=popularity-model reRankDocs=4}");
-    query.add("fl", "[fv]");
-    assertJQ("/query" + query.toQueryString(), "/response/numFound/==1");
-    assertJQ("/query" + query.toQueryString(),
-            
"/response/docs/[0]/=={'[fv]':'"+FeatureLoggerTestUtils.toFeatureVector("popularity",Float.toString(FIELD_VALUE_FEATURE_DEFAULT_VAL))+"'}");
-
+    for (String field : FIELD_NAMES) {
+      SolrQuery query = new SolrQuery();
+      query.setQuery("id:42");
+      query.add("fl", "*, score");
+      query.add("rows", "4");
+
+      assertJQ("/query" + query.toQueryString(), "/response/numFound/==1");
+      assertJQ("/query" + query.toQueryString(), 
"/response/docs/[0]/id=='42'");
+      query = new SolrQuery();
+      query.setQuery("id:42");
+      query.add("rq", "{!ltr model="+field+"-model reRankDocs=4}");
+      query.add("fl", "[fv]");
+      assertJQ("/query" + query.toQueryString(), "/response/numFound/==1");
+      assertJQ("/query" + query.toQueryString(),
+              
"/response/docs/[0]/=={'[fv]':'"+FeatureLoggerTestUtils.toFeatureVector(field,Float.toString(FIELD_VALUE_FEATURE_DEFAULT_VAL))+"'}");

Review comment:
       I think the problem is `final private boolean extractAllFeatures;` in 
`LTRScoringQuery`. It is set to true and therefore all previous features are 
added to the response instead of just the model features.
   
   This is the logic (line 210 in LTRScoringQuery):
   ```java
   if (this.extractAllFeatures) {
       features = allFeatures;
   } else{
       features =  modelFeatures;
   }
   ```
   
   I didn't find a way to set it to false with a query param.
   So we would have two options:
   - use this constructor `public LTRScoringQuery(LTRScoringModel 
ltrScoringModel, boolean extractAllFeatures)`
   - adjust the xpath to something like `[ends-with(., '<fieldname>=0.0')]`
   
   I am trying to adjust the xpath, because I would prefer that solution, but I 
was not able to find the right pattern so far.




-- 
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...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to