Fix moving function linear weighted avg (elastic#118516)

Fix moving function linear weighted avg # Conflicts: # modules/aggregations/build.gradle
iverase · Dec 16, 2024 · cbd8453 · cbd8453
1 parent 93ffc84
commit cbd8453
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 13 deletions.
diff --git a/docs/changelog/118516.yaml b/docs/changelog/118516.yaml
@@ -0,0 +1,6 @@
+pr: 118435
+summary: Fix moving function linear weighted avg
+area: Aggregations
+type: bug
+issues:
+  - 113751
diff --git a/modules/aggregations/build.gradle b/modules/aggregations/build.gradle
@@ -57,6 +57,8 @@ tasks.named("yamlRestTestV7CompatTransform").configure { task ->
 
   // Something has changed with response codes
   task.skipTest("search.aggregation/20_terms/IP test", "Hybrid t-digest produces different results.")
+  // Maths changed
+  task.skipTest("aggregations/moving_fn/linearWeightedAvg", "math was wrong in previous versions")
 
   task.addAllowedWarningRegex("\\[types removal\\].*")
 }

diff --git a/...les/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/moving_fn.yml b/...les/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/moving_fn.yml
@@ -255,6 +255,17 @@ linearWeightedAvg:
   - skip:
       features: close_to
 
+  - requires:
+      test_runner_features: [capabilities]
+
+  - requires:
+      capabilities:
+        - method: POST
+          path: /_search
+          parameters: [method, path, parameters, capabilities]
+          capabilities: [moving_fn_right_math]
+      reason: "math not fixed yet"
+
   - do:
       search:
         index: no_gaps
@@ -275,11 +286,11 @@ linearWeightedAvg:
   - match: { hits.total.value: 6 }
   - length: { [email protected]: 6 }
   - is_false: [email protected]
-  - close_to: { [email protected]: { value: 0.500, error: 0.0005 } }
-  - close_to: { [email protected]: { value: 1.250, error: 0.0005 } }
-  - close_to: { [email protected]: { value: 1.000, error: 0.0005 } }
-  - close_to: { [email protected]: { value: 2.250, error: 0.0005 } }
-  - close_to: { [email protected]: { value: 3.500, error: 0.0005 } }
+  - close_to: { [email protected]: { value: 1.000, error: 0.0005 } }
+  - close_to: { [email protected]: { value: 1.667, error: 0.0005 } }
+  - close_to: { [email protected]: { value: 1.333, error: 0.0005 } }
+  - close_to: { [email protected]: { value: 3.000, error: 0.0005 } }
+  - close_to: { [email protected]: { value: 4.667, error: 0.0005 } }
 
   - do:
       search:
@@ -301,11 +312,11 @@ linearWeightedAvg:
   - match: { hits.total.value: 6 }
   - length: { [email protected]: 6 }
   - is_false: [email protected]
-  - close_to: { [email protected]: { value: 0.500, error: 0.0005 } }
-  - close_to: { [email protected]: { value: 1.250, error: 0.0005 } }
-  - close_to: { [email protected]: { value: 1.143, error: 0.0005 } }
-  - close_to: { [email protected]: { value: 2.286, error: 0.0005 } }
-  - close_to: { [email protected]: { value: 3.429, error: 0.0005 } }
+  - close_to: { [email protected]: { value: 1.000, error: 0.0005 } }
+  - close_to: { [email protected]: { value: 1.667, error: 0.0005 } }
+  - close_to: { [email protected]: { value: 1.333, error: 0.0005 } }
+  - close_to: { [email protected]: { value: 2.667, error: 0.0005 } }
+  - close_to: { [email protected]: { value: 4.000, error: 0.0005 } }
 
 ---
 ewma:

diff --git a/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java b/server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java
@@ -40,6 +40,8 @@ private SearchCapabilities() {}
     private static final String RANK_VECTORS_SCRIPT_ACCESS = "rank_vectors_script_access";
     /** Initial support for rank-vectors maxSim functions access. */
     private static final String RANK_VECTORS_SCRIPT_MAX_SIM = "rank_vectors_script_max_sim_with_bugfix";
+    /** Fixed the math in {@code moving_fn}'s {@code linearWeightedAvg}. */
+    private static final String MOVING_FN_RIGHT_MATH = "moving_fn_right_math";
 
     private static final String RANDOM_SAMPLER_WITH_SCORED_SUBAGGS = "random_sampler_with_scored_subaggs";
     private static final String OPTIMIZED_SCALAR_QUANTIZATION_BBQ = "optimized_scalar_quantization_bbq";
@@ -56,6 +58,7 @@ private SearchCapabilities() {}
         capabilities.add(RANDOM_SAMPLER_WITH_SCORED_SUBAGGS);
         capabilities.add(OPTIMIZED_SCALAR_QUANTIZATION_BBQ);
         capabilities.add(KNN_QUANTIZED_VECTOR_RESCORE);
+        capabilities.add(MOVING_FN_RIGHT_MATH);
         if (RankVectorsFieldMapper.FEATURE_FLAG.isEnabled()) {
             capabilities.add(RANK_VECTORS_FIELD_MAPPER);
             capabilities.add(RANK_VECTORS_SCRIPT_ACCESS);

diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovingFunctions.java b/server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovingFunctions.java
@@ -100,7 +100,7 @@ public static double stdDev(double[] values, double avg) {
      */
     public static double linearWeightedAvg(double[] values) {
         double avg = 0;
-        long totalWeight = 1;
+        long totalWeight = 0;
         long current = 1;
 
         for (double v : values) {
@@ -110,7 +110,7 @@ public static double linearWeightedAvg(double[] values) {
                 current += 1;
             }
         }
-        return totalWeight == 1 ? Double.NaN : avg / totalWeight;
+        return totalWeight == 0 ? Double.NaN : avg / totalWeight;
     }
 
     /**

diff --git a/...st/java/org/elasticsearch/search/aggregations/pipeline/MovFnWhitelistedFunctionTests.java b/...st/java/org/elasticsearch/search/aggregations/pipeline/MovFnWhitelistedFunctionTests.java
@@ -326,7 +326,7 @@ public void testLinearMovAvg() {
             }
 
             double avg = 0;
-            long totalWeight = 1;
+            long totalWeight = 0;
             long current = 1;
 
             for (double value : window) {