fix: ensure kplus reach ratio not exceeding 1.0 (#1868)

…ution.
world-federation-of-advertisers · Oct 23, 2024 · 34eb548 · 34eb548
1 parent 19a32bd
commit 34eb548
Showing 1 changed file with 17 additions and 4 deletions.
diff --git a/src/main/kotlin/org/wfanet/measurement/measurementconsumer/stats/Variances.kt b/src/main/kotlin/org/wfanet/measurement/measurementconsumer/stats/Variances.kt
@@ -16,7 +16,9 @@
 
 package org.wfanet.measurement.measurementconsumer.stats
 
+import com.google.common.math.DoubleMath.fuzzyCompare
 import kotlin.math.max
+import kotlin.math.min
 import kotlin.math.pow
 import kotlin.random.Random
 import kotlin.random.asJavaRandom
@@ -81,6 +83,8 @@ interface Variances {
 
 /** Default implementation of [Variances]. */
 object VariancesImpl : Variances {
+  private const val TOLERANCE = 1E-6
+
   /**
    * Computes the variance of a reach measurement that is computed using the deterministic count
    * distinct methodology.
@@ -441,11 +445,14 @@ object VariancesImpl : Variances {
     val maximumFrequency = params.measurementParams.maximumFrequency
 
     var suffixSum = 0.0
-    // There is no estimate of zero-frequency reach
+    // There is no estimate of zero-frequency reach.
     val kPlusRelativeFrequencyDistribution: Map<Int, Double> =
       (maximumFrequency downTo 1).associateWith { frequency ->
         suffixSum += params.relativeFrequencyDistribution.getOrDefault(frequency, 0.0)
-        suffixSum
+        require(fuzzyCompare(suffixSum, 1.0, TOLERANCE) <= 0) {
+          "kPlus relative frequency must not exceed 1, but got $suffixSum."
+        }
+        min(1.0, suffixSum)
       }
 
     val relativeVariances: Map<Int, Double> =
@@ -554,7 +561,10 @@ object VariancesImpl : Variances {
     val kPlusRelativeFrequencyDistribution: Map<Int, Double> =
       (maximumFrequency downTo 1).associateWith { frequency ->
         suffixSum += frequencyParams.relativeFrequencyDistribution.getOrDefault(frequency, 0.0)
-        suffixSum
+        require(fuzzyCompare(suffixSum, 1.0, TOLERANCE) <= 0) {
+          "kPlus relative frequency must not exceed 1, but got $suffixSum."
+        }
+        min(1.0, suffixSum)
       }
 
     val countVariances: Map<Int, Double> =
@@ -656,7 +666,10 @@ object VariancesImpl : Variances {
     val kPlusRelativeFrequencyDistribution: Map<Int, Double> =
       (maximumFrequency downTo 1).associateWith { frequency ->
         suffixSum += params.relativeFrequencyDistribution.getOrDefault(frequency, 0.0)
-        suffixSum
+        require(fuzzyCompare(suffixSum, 1.0, TOLERANCE) <= 0) {
+          "kPlus relative frequency must not exceed 1, but got $suffixSum."
+        }
+        min(1.0, suffixSum)
       }
 
     val countVariances: Map<Int, Double> =