Browse Source

Fix moving function linear weighted avg (#118516) (#118751)

Fix moving function linear weighted avg

Co-authored-by: Quentin Deschamps <quentindeschamps18@orange.fr>
Ignacio Vera 10 months ago
parent
commit
36826a4da1

+ 6 - 0
docs/changelog/118516.yaml

@@ -0,0 +1,6 @@
+pr: 118435
+summary: Fix moving function linear weighted avg
+area: Aggregations
+type: bug
+issues:
+  - 113751

+ 2 - 0
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\\].*")
 }

+ 21 - 10
modules/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: { aggregations.@timestamp.buckets: 6 }
   - is_false: aggregations.@timestamp.buckets.0.d.value
-  - close_to: { aggregations.@timestamp.buckets.1.d.value: { value: 0.500, error: 0.0005 } }
-  - close_to: { aggregations.@timestamp.buckets.2.d.value: { value: 1.250, error: 0.0005 } }
-  - close_to: { aggregations.@timestamp.buckets.3.d.value: { value: 1.000, error: 0.0005 } }
-  - close_to: { aggregations.@timestamp.buckets.4.d.value: { value: 2.250, error: 0.0005 } }
-  - close_to: { aggregations.@timestamp.buckets.5.d.value: { value: 3.500, error: 0.0005 } }
+  - close_to: { aggregations.@timestamp.buckets.1.d.value: { value: 1.000, error: 0.0005 } }
+  - close_to: { aggregations.@timestamp.buckets.2.d.value: { value: 1.667, error: 0.0005 } }
+  - close_to: { aggregations.@timestamp.buckets.3.d.value: { value: 1.333, error: 0.0005 } }
+  - close_to: { aggregations.@timestamp.buckets.4.d.value: { value: 3.000, error: 0.0005 } }
+  - close_to: { aggregations.@timestamp.buckets.5.d.value: { value: 4.667, error: 0.0005 } }
 
   - do:
       search:
@@ -301,11 +312,11 @@ linearWeightedAvg:
   - match: { hits.total.value: 6 }
   - length: { aggregations.@timestamp.buckets: 6 }
   - is_false: aggregations.@timestamp.buckets.0.d.value
-  - close_to: { aggregations.@timestamp.buckets.1.d.value: { value: 0.500, error: 0.0005 } }
-  - close_to: { aggregations.@timestamp.buckets.2.d.value: { value: 1.250, error: 0.0005 } }
-  - close_to: { aggregations.@timestamp.buckets.3.d.value: { value: 1.143, error: 0.0005 } }
-  - close_to: { aggregations.@timestamp.buckets.4.d.value: { value: 2.286, error: 0.0005 } }
-  - close_to: { aggregations.@timestamp.buckets.5.d.value: { value: 3.429, error: 0.0005 } }
+  - close_to: { aggregations.@timestamp.buckets.1.d.value: { value: 1.000, error: 0.0005 } }
+  - close_to: { aggregations.@timestamp.buckets.2.d.value: { value: 1.667, error: 0.0005 } }
+  - close_to: { aggregations.@timestamp.buckets.3.d.value: { value: 1.333, error: 0.0005 } }
+  - close_to: { aggregations.@timestamp.buckets.4.d.value: { value: 2.667, error: 0.0005 } }
+  - close_to: { aggregations.@timestamp.buckets.5.d.value: { value: 4.000, error: 0.0005 } }
 
 ---
 ewma:

+ 3 - 0
server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java

@@ -40,6 +40,8 @@ public final class 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 @@ public final class 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);

+ 2 - 2
server/src/main/java/org/elasticsearch/search/aggregations/pipeline/MovingFunctions.java

@@ -100,7 +100,7 @@ public class MovingFunctions {
      */
     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 class MovingFunctions {
                 current += 1;
             }
         }
-        return totalWeight == 1 ? Double.NaN : avg / totalWeight;
+        return totalWeight == 0 ? Double.NaN : avg / totalWeight;
     }
 
     /**

+ 1 - 1
server/src/test/java/org/elasticsearch/search/aggregations/pipeline/MovFnWhitelistedFunctionTests.java

@@ -326,7 +326,7 @@ public class MovFnWhitelistedFunctionTests extends ESTestCase {
             }
 
             double avg = 0;
-            long totalWeight = 1;
+            long totalWeight = 0;
             long current = 1;
 
             for (double value : window) {