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. **Edit:** Ok, I took a break and realized my mistake (the break seemed to be necessary *smh*). I took another look at the jsonpath-adjustment. A pattern like this `"['response']['docs'][0][?(@['[fv]']=~ /.* <fieldName> =0.0.*/i )]"` should work but it would require an additional dependency for `jayway.jsonpath` and we would have to manually call `JsonPath.read(...)` and assert that the result is not null. I am not sure, whether that's nice, but maybe you want to build up upon that / can generate an idea from here? -- 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