fix: ensure kplus reach ratio not exceeding 1.0

[ple13]

…ution.

[wfa-reviewable]

This change is 

[SanjayVas]

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ple13)

---

src/main/kotlin/org/wfanet/measurement/measurementconsumer/stats/Variances.kt line 451 at r1 (raw file):

      (maximumFrequency downTo 1).associateWith { frequency ->
        suffixSum += params.relativeFrequencyDistribution.getOrDefault(frequency, 0.0)
        require(suffixSum <= 1.0 + TOLERANCE) {

Shouldn't the tolerance go in both directions? i.e. 1.0 ± tolerance?

Code quote:

1.0 + TOLERANCE

[ple13]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/measurementconsumer/stats/Variances.kt line 451 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Shouldn't the tolerance go in both directions? i.e. 1.0 ± tolerance?

We only need to check for one side as any value in [0.0, 1.0] is valid.

[SanjayVas]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ple13)

[stevenwarejones]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ple13)

---

src/main/kotlin/org/wfanet/measurement/measurementconsumer/stats/Variances.kt line 451 at r1 (raw file):

      (maximumFrequency downTo 1).associateWith { frequency ->
        suffixSum += params.relativeFrequencyDistribution.getOrDefault(frequency, 0.0)
        require(suffixSum <= 1.0 + TOLERANCE) {

I'd prefer this be done with a function isLessThanWithTolerance

[SanjayVas]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/measurementconsumer/stats/Variances.kt line 451 at r1 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

I'd prefer this be done with a function isLessThanWithTolerance

FWIW, the similar function in the Guava library is called fuzzyEquals. In Kotlin, this could be implemented as an extension with the following signature:

fun Double.fuzzyEquals(other: Double, tolerance: Double): Boolean

[ple13]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @SanjayVas and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/measurementconsumer/stats/Variances.kt line 451 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

FWIW, the similar function in the Guava library is called fuzzyEquals. In Kotlin, this could be implemented as an extension with the following signature:

fun Double.fuzzyEquals(other: Double, tolerance: Double): Boolean

Done. Please let me know if you prefer the function to be moved to measurement/common.

[stevenwarejones]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @SanjayVas)

---

src/main/kotlin/org/wfanet/measurement/measurementconsumer/stats/Variances.kt line 451 at r1 (raw file):

Previously, ple13 (Phi) wrote…

Done. Please let me know if you prefer the function to be moved to measurement/common.

i'd prefer it be moved to common

[kungfucraig]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @SanjayVas and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/measurementconsumer/stats/Variances.kt line 451 at r1 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

i'd prefer it be moved to common

Can we just call the Guava function directly?

1. it already exists and does what we want
2. it's a one off call, thus no Kotlin wrapper is needed
3. were not building a reusable math library, so we really need this in common with a set of tests, etc.

[ple13]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @SanjayVas and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/measurementconsumer/stats/Variances.kt line 451 at r1 (raw file):

Previously, kungfucraig (Craig Wright) wrote…

Can we just call the Guava function directly?

1. it already exists and does what we want
2. it's a one off call, thus no Kotlin wrapper is needed
3. were not building a reusable math library, so we really need this in common with a set of tests, etc.

Thanks. I've updated it to use fuzzyCompare.

[kungfucraig]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @SanjayVas and @stevenwarejones)

[SanjayVas]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @stevenwarejones)

[stevenwarejones]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ple13)