SOLR-17760: solving bug in LTR dense/sparse format (#3354)

* Fixed field value feature

(cherry picked from commit ba981cd)

Author: aruggero

solr/CHANGES.txt

@@ -132,6 +132,8 @@ Bug Fixes
 
 * SOLR-17726: MoreLikeThis to support copy-fields (Ilaria Petreti via Alessandro Benedetti)
 
+* SOLR-16667: Fixed dense/sparse representation in LTR module. (Anna Ruggero, Alessandro Benedetti)
+
 Dependency Upgrades
 ---------------------
 * SOLR-17471: Upgrade Lucene to 9.12.1. (Pierre Salagnac, Christine Poerschke)

solr/modules/ltr/src/java/org/apache/solr/ltr/CSVFeatureLogger.java

@@ -44,7 +44,7 @@ public String makeFeatureVector(LTRScoringQuery.FeatureInfo[] featuresInfo) {
     StringBuilder sb = new StringBuilder(featuresInfo.length * 3);
     boolean isDense = featureFormat.equals(FeatureFormat.DENSE);
     for (LTRScoringQuery.FeatureInfo featInfo : featuresInfo) {
-      if (featInfo != null && (isDense || featInfo.isUsed())) {
+      if (featInfo != null && (isDense || !featInfo.isDefaultValue())) {
         sb.append(featInfo.getName())
             .append(keyValueSep)
             .append(featInfo.getValue())

solr/modules/ltr/src/java/org/apache/solr/ltr/LTRScoringQuery.java

@@ -331,12 +331,12 @@ public long ramBytesUsed() {
   public static class FeatureInfo {
     private final String name;
     private float value;
-    private boolean used;
+    private boolean isDefaultValue;
 
-    FeatureInfo(String n, float v, boolean u) {
-      name = n;
-      value = v;
-      used = u;
+    FeatureInfo(String name, float value, boolean isDefaultValue) {
+      this.name = name;
+      this.value = value;
+      this.isDefaultValue = isDefaultValue;
     }
 
     public void setValue(float value) {
@@ -351,12 +351,12 @@ public float getValue() {
       return value;
     }
 
-    public boolean isUsed() {
-      return used;
+    public boolean isDefaultValue() {
+      return isDefaultValue;
     }
 
-    public void setUsed(boolean used) {
-      this.used = used;
+    public void setIsDefaultValue(boolean isDefaultValue) {
+      this.isDefaultValue = isDefaultValue;
     }
   }
 
@@ -408,7 +408,7 @@ private void setFeaturesInfo() {
         String featName = extractedFeatureWeights[i].getName();
         int featId = extractedFeatureWeights[i].getIndex();
         float value = extractedFeatureWeights[i].getDefaultValue();
-        featuresInfo[featId] = new FeatureInfo(featName, value, false);
+        featuresInfo[featId] = new FeatureInfo(featName, value, true);
       }
     }
 
@@ -440,12 +440,7 @@ private float makeNormalizedFeaturesAndScore() {
       for (final Feature.FeatureWeight feature : modelFeatureWeights) {
         final int featureId = feature.getIndex();
         FeatureInfo fInfo = featuresInfo[featureId];
-        // not checking for finfo == null as that would be a bug we should catch
-        if (fInfo.isUsed()) {
-          modelFeatureValuesNormalized[pos] = fInfo.getValue();
-        } else {
-          modelFeatureValuesNormalized[pos] = feature.getDefaultValue();
-        }
+        modelFeatureValuesNormalized[pos] = fInfo.getValue();
         pos++;
       }
       ltrScoringModel.normalizeFeaturesInPlace(modelFeatureValuesNormalized);
@@ -480,7 +475,7 @@ protected void reset() {
         // need to set default value everytime as the default value is used in 'dense'
         // mode even if used=false
         featuresInfo[featId].setValue(value);
-        featuresInfo[featId].setUsed(false);
+        featuresInfo[featId].setIsDefaultValue(true);
       }
     }
 
@@ -598,7 +593,9 @@ public float score() throws IOException {
               Feature.FeatureWeight scFW = (Feature.FeatureWeight) subScorer.getWeight();
               final int featureId = scFW.getIndex();
               featuresInfo[featureId].setValue(subScorer.score());
-              featuresInfo[featureId].setUsed(true);
+              if (featuresInfo[featureId].getValue() != scFW.getDefaultValue()) {
+                featuresInfo[featureId].setIsDefaultValue(false);
+              }
             }
           }
           return makeNormalizedFeaturesAndScore();
@@ -683,7 +680,9 @@ public float score() throws IOException {
                 Feature.FeatureWeight scFW = (Feature.FeatureWeight) scorer.getWeight();
                 final int featureId = scFW.getIndex();
                 featuresInfo[featureId].setValue(scorer.score());
-                featuresInfo[featureId].setUsed(true);
+                if (featuresInfo[featureId].getValue() != scFW.getDefaultValue()) {
+                  featuresInfo[featureId].setIsDefaultValue(false);
+                }
               }
             }
           }

solr/modules/ltr/src/test/org/apache/solr/ltr/TestSelectiveWeightCreation.java

@@ -142,7 +142,10 @@ public void testScoringQueryWeightCreation() throws IOException, ModelException
     assertEquals("11", searcher.storedFields().document(hits.scoreDocs[1].doc).get("id"));
 
     List<Feature> features = makeFeatures(new int[] {0, 1, 2});
+    List<Feature> expectedNonDefaultFeatures = makeFeatures(new int[] {1, 2});
     final List<Feature> allFeatures = makeFeatures(new int[] {0, 1, 2, 3, 4, 5, 6, 7, 8, 9});
+    List<Feature> expectedNonDefaultAllFeatures =
+        makeFeatures(new int[] {1, 2, 3, 4, 5, 6, 7, 8, 9});
     final List<Normalizer> norms = new ArrayList<>();
     for (int k = 0; k < features.size(); ++k) {
       norms.add(IdentityNormalizer.INSTANCE);
@@ -167,13 +170,13 @@ public void testScoringQueryWeightCreation() throws IOException, ModelException
     LTRScoringQuery.FeatureInfo[] featuresInfo = modelWeight.getFeaturesInfo();
 
     assertEquals(features.size(), modelWeight.getModelFeatureValuesNormalized().length);
-    int validFeatures = 0;
+    int nonDefaultFeatures = 0;
     for (int i = 0; i < featuresInfo.length; ++i) {
-      if (featuresInfo[i] != null && featuresInfo[i].isUsed()) {
-        validFeatures += 1;
+      if (featuresInfo[i] != null && !featuresInfo[i].isDefaultValue()) {
+        nonDefaultFeatures += 1;
       }
     }
-    assertEquals(validFeatures, features.size());
+    assertEquals(expectedNonDefaultFeatures.size(), nonDefaultFeatures);
 
     // when features are requested in the response, weights should be created for all features
     final LTRScoringModel ltrScoringModel2 =
@@ -194,13 +197,13 @@ public void testScoringQueryWeightCreation() throws IOException, ModelException
     assertEquals(features.size(), modelWeight.getModelFeatureValuesNormalized().length);
     assertEquals(allFeatures.size(), modelWeight.getExtractedFeatureWeights().length);
 
-    validFeatures = 0;
+    nonDefaultFeatures = 0;
     for (int i = 0; i < featuresInfo.length; ++i) {
-      if (featuresInfo[i] != null && featuresInfo[i].isUsed()) {
-        validFeatures += 1;
+      if (featuresInfo[i] != null && !featuresInfo[i].isDefaultValue()) {
+        nonDefaultFeatures += 1;
       }
     }
-    assertEquals(validFeatures, allFeatures.size());
+    assertEquals(expectedNonDefaultAllFeatures.size(), nonDefaultFeatures);
 
     assertU(delI("10"));
     assertU(delI("11"));