Fixing bug with pearson average computation when skipping values

Author: mneedham

algo/src/test/java/org/neo4j/graphalgo/similarity/WeightedInputTest.java

@@ -86,6 +86,19 @@ public void pearsonNaNRespectsSimilarityCutOff() {
         assertNull(input1.pearson(null, 0.1, input2));
     }
 
+    @Test
+    public void pearsonWithNonOverlappingValues() {
+        double[] weights1 = new double[]{1,          2,3, Double.NaN, 4};   // ave = 10/4 = 2.5
+        double[] weights2 = new double[]{Double.NaN, 2,3, 1,          4};  // ave = 10/4 = 2.5
+
+        WeightedInput input1 = new WeightedInput(1, weights1);
+        WeightedInput input2 = new WeightedInput(2, weights2);
+
+        SimilarityResult similarityResult = input1.pearsonSkip(null, -1.0, input2, Double.NaN);
+
+        assertEquals(1.0, similarityResult.similarity, 0.01);
+    }
+
     @Test
     public void cosineNoCompression() {
         double[] weights1 = new double[]{1, 2, 3, 4, 4, 4, 4, 5, 6};

core/src/main/java/org/neo4j/graphalgo/core/utils/Intersections.java

@@ -109,10 +109,10 @@ public static double sumSquareDeltaSkip(double[] vector1, double[] vector2, int
         double result = 0;
         for (int i = 0; i < len; i++) {
             double weight1 = vector1[i];
-            if (weight1 == skipValue || (skipNan && Double.isNaN(weight1))) continue;
+            if (shouldSkip(weight1, skipValue, skipNan)) continue;
 
             double weight2 = vector2[i];
-            if (weight2 == skipValue || (skipNan && Double.isNaN(weight2))) continue;
+            if (shouldSkip(weight2, skipValue, skipNan)) continue;
 
             double delta = weight1 - weight2;
             result += delta * delta;
@@ -164,9 +164,9 @@ public static double cosineSquareSkip(double[] vector1, double[] vector2, int le
         double yLength = 0d;
         for (int i = 0; i < len; i++) {
             double weight1 = vector1[i];
-            if (weight1 == skipValue || (skipNan && Double.isNaN(weight1))) continue;
+            if (shouldSkip(weight1, skipValue, skipNan)) continue;
             double weight2 = vector2[i];
-            if (weight2 == skipValue || (skipNan && Double.isNaN(weight2))) continue;
+            if (shouldSkip(weight2, skipValue, skipNan)) continue;
 
             dotProduct += weight1 * weight2;
             xLength += weight1 * weight1;
@@ -207,32 +207,36 @@ public static double pearsonSkip(double[] vector1, double[] vector2, int len, do
         boolean skipNan = Double.isNaN(skipValue);
 
         double vector1Sum = 0.0;
+        int vector1Count = 0;
         double vector2Sum = 0.0;
-        int count =0;
+        int vector2Count = 0;
         for (int i = 0; i < len; i++) {
             double weight1 = vector1[i];
             double weight2 = vector2[i];
 
-            if (weight1 == skipValue || (skipNan && Double.isNaN(weight1))) continue;
-            if (weight2 == skipValue || (skipNan && Double.isNaN(weight2))) continue;
+            if(!shouldSkip(weight1, skipValue, skipNan)) {
+                vector1Sum += weight1;
+                vector1Count++;
+            }
 
-            vector1Sum += weight1;
-            vector2Sum += weight2;
-            count++;
+            if (!shouldSkip(weight2, skipValue, skipNan)) {
+                vector2Sum += weight2;
+                vector2Count++;
+            }
         }
 
-        double vector1Mean = vector1Sum / count;
-        double vector2Mean = vector2Sum / count;
+        double vector1Mean = vector1Sum / vector1Count;
+        double vector2Mean = vector2Sum / vector2Count;
 
         double dotProductMinusMean = 0d;
         double xLength = 0d;
         double yLength = 0d;
         for (int i = 0; i < len; i++) {
             double weight1 = vector1[i];
-            if (weight1 == skipValue || (skipNan && Double.isNaN(weight1))) continue;
+            if (shouldSkip(weight1, skipValue, skipNan)) continue;
 
             double weight2 = vector2[i];
-            if (weight2 == skipValue || (skipNan && Double.isNaN(weight2))) continue;
+            if (shouldSkip(weight2, skipValue, skipNan)) continue;
 
             double vector1Delta = weight1 - vector1Mean;
             double vector2Delta = weight2 - vector2Mean;
@@ -245,6 +249,10 @@ public static double pearsonSkip(double[] vector1, double[] vector2, int len, do
         return dotProductMinusMean / Math.sqrt(xLength * yLength);
     }
 
+    private static boolean shouldSkip(double weight, double skipValue, boolean skipNan) {
+        return weight == skipValue || (skipNan && Double.isNaN(weight));
+    }
+
     public static double cosine(double[] vector1, double[] vector2, int len) {
         double dotProduct = 0d;
         double xLength = 0d;

tests/src/test/java/org/neo4j/graphalgo/algo/similarity/PearsonSimilarityTest.java

@@ -48,5 +48,4 @@ public void oppositeVectors() {
         double similarity = similarities.pearsonSimilarity(user1Ratings, user2Ratings);
         assertEquals(-1.0, similarity, 0.01);
     }
-
 }