CR comments

This commit is contained in:
Quinn Dougherty 2022-04-13 19:58:36 -04:00
parent b0ff2c19f5
commit 99d1c345c5
2 changed files with 39 additions and 26 deletions

View File

@ -13,7 +13,7 @@ open Expect
open TestHelpers open TestHelpers
module Internals = { module Internals = {
let epsilon = 1e3 let epsilon = 1e1
let mean = GenericDist_Types.Constructors.UsingDists.mean let mean = GenericDist_Types.Constructors.UsingDists.mean
@ -47,8 +47,8 @@ module Internals = {
dist2': SymbolicDistTypes.symbolicDist, dist2': SymbolicDistTypes.symbolicDist,
~epsilon: float, ~epsilon: float,
) => { ) => {
let dist1 = dist1'->DistributionTypes.Symbolic // ->DistributionTypes.Other let dist1 = dist1'->DistributionTypes.Symbolic
let dist2 = dist2'->DistributionTypes.Symbolic // ->DistributionTypes.Other let dist2 = dist2'->DistributionTypes.Symbolic
let received = let received =
distOp(dist1, dist2)->E.R2.fmap(mean)->E.R2.fmap(run)->E.R2.fmap(toFloat)->E.R.toExn distOp(dist1, dist2)->E.R2.fmap(mean)->E.R2.fmap(run)->E.R2.fmap(toFloat)->E.R.toExn
let expected = floatOp(runMean(dist1), runMean(dist2)) let expected = floatOp(runMean(dist1), runMean(dist2))
@ -81,19 +81,23 @@ describe("Means invariant", () => {
describe("for addition", () => { describe("for addition", () => {
let testAdditionMean = testOperationMean(algebraicAdd, "algebraicAdd", \"+.", ~epsilon) let testAdditionMean = testOperationMean(algebraicAdd, "algebraicAdd", \"+.", ~epsilon)
testAll("of two of the same distribution", distributions, dist => { testAll("with two of the same distribution", distributions, dist => {
E.R.liftM2(testAdditionMean, dist, dist)->E.R.toExn E.R.liftM2(testAdditionMean, dist, dist)->E.R.toExn
}) })
testAll("of two different distributions", pairsOfDifferentDistributions, dists => { testAll("with two different distributions", pairsOfDifferentDistributions, dists => {
let (dist1, dist2) = dists let (dist1, dist2) = dists
E.R.liftM2(testAdditionMean, dist1, dist2)->E.R.toExn E.R.liftM2(testAdditionMean, dist1, dist2)->E.R.toExn
}) })
testAll("of two difference distributions", pairsOfDifferentDistributions, dists => { testAll(
let (dist1, dist2) = dists "with two different distributions in swapped order",
E.R.liftM2(testAdditionMean, dist2, dist1)->E.R.toExn pairsOfDifferentDistributions,
}) dists => {
let (dist1, dist2) = dists
E.R.liftM2(testAdditionMean, dist2, dist1)->E.R.toExn
},
)
}) })
describe("for subtraction", () => { describe("for subtraction", () => {
@ -104,19 +108,23 @@ describe("Means invariant", () => {
~epsilon, ~epsilon,
) )
testAll("of two of the same distribution", distributions, dist => { testAll("with two of the same distribution", distributions, dist => {
E.R.liftM2(testSubtractionMean, dist, dist)->E.R.toExn E.R.liftM2(testSubtractionMean, dist, dist)->E.R.toExn
}) })
testAll("of two different distributions", pairsOfDifferentDistributions, dists => { testAll("with two different distributions", pairsOfDifferentDistributions, dists => {
let (dist1, dist2) = dists let (dist1, dist2) = dists
E.R.liftM2(testSubtractionMean, dist1, dist2)->E.R.toExn E.R.liftM2(testSubtractionMean, dist1, dist2)->E.R.toExn
}) })
testAll("of two different distributions", pairsOfDifferentDistributions, dists => { testAll(
let (dist1, dist2) = dists "with two different distributions in swapped order",
E.R.liftM2(testSubtractionMean, dist2, dist1)->E.R.toExn pairsOfDifferentDistributions,
}) dists => {
let (dist1, dist2) = dists
E.R.liftM2(testSubtractionMean, dist2, dist1)->E.R.toExn
},
)
}) })
describe("for multiplication", () => { describe("for multiplication", () => {
@ -127,18 +135,22 @@ describe("Means invariant", () => {
~epsilon, ~epsilon,
) )
testAll("of two of the same distribution", distributions, dist => { testAll("with two of the same distribution", distributions, dist => {
E.R.liftM2(testMultiplicationMean, dist, dist)->E.R.toExn E.R.liftM2(testMultiplicationMean, dist, dist)->E.R.toExn
}) })
testAll("of two different distributions", pairsOfDifferentDistributions, dists => { testAll("with two different distributions", pairsOfDifferentDistributions, dists => {
let (dist1, dist2) = dists let (dist1, dist2) = dists
E.R.liftM2(testMultiplicationMean, dist1, dist2)->E.R.toExn E.R.liftM2(testMultiplicationMean, dist1, dist2)->E.R.toExn
}) })
testAll("of two different distributions", pairsOfDifferentDistributions, dists => { testAll(
let (dist1, dist2) = dists "with two different distributions in swapped order",
E.R.liftM2(testMultiplicationMean, dist2, dist1)->E.R.toExn pairsOfDifferentDistributions,
}) dists => {
let (dist1, dist2) = dists
E.R.liftM2(testMultiplicationMean, dist2, dist1)->E.R.toExn
},
)
}) })
}) })

View File

@ -3,11 +3,12 @@ open Expect
let expectErrorToBeBounded = (received, expected, ~epsilon) => { let expectErrorToBeBounded = (received, expected, ~epsilon) => {
let distance = Js.Math.abs_float(received -. expected) let distance = Js.Math.abs_float(received -. expected)
let error = if expected < epsilon ** 2.5 { let expectedAbs = Js.Math.abs_float(expected)
distance /. epsilon let normalizingDenom = Js.Math.max_float(
} else { expectedAbs,
distance /. Js.Math.abs_float(expected) epsilon < 1.0 ? epsilon ** 2.0 : epsilon ** -2.0,
} )
let error = distance /. normalizingDenom
error->expect->toBeLessThan(epsilon) error->expect->toBeLessThan(epsilon)
} }