diff --git a/packages/squiggle-lang/__tests__/ReducerInterface/ReducerInterface_Distribution_test.res b/packages/squiggle-lang/__tests__/ReducerInterface/ReducerInterface_Distribution_test.res index a4569a88..33afd311 100644 --- a/packages/squiggle-lang/__tests__/ReducerInterface/ReducerInterface_Distribution_test.res +++ b/packages/squiggle-lang/__tests__/ReducerInterface/ReducerInterface_Distribution_test.res @@ -23,7 +23,7 @@ describe("eval on distribution functions", () => { testEval("-normal(5,2)", "Ok(Normal(-5,2))") }) describe("to", () => { - testEval("5 to 2", "Error(Math Error: Low value must be less than high value.)") + testEval("5 to 2", "Error(Distribution Math Error: Low value must be less than high value.)") testEval("to(2,5)", "Ok(Lognormal(1.1512925464970227,0.27853260523016377))") testEval("to(-2,2)", "Ok(Normal(0,1.2159136638235384))") }) @@ -90,10 +90,13 @@ describe("eval on distribution functions", () => { describe("log", () => { testEval("log(2, uniform(5,8))", "Ok(Sample Set Distribution)") - testEval("log(normal(5,2), 3)", "Error(Math Error: Operation returned complex result)") + testEval( + "log(normal(5,2), 3)", + "Error(Distribution Math Error: Logarithm of input error: First input must completely greater than 0)", + ) testEval( "log(normal(5,2), normal(10,1))", - "Error(Math Error: Operation returned complex result)", + "Error(Distribution Math Error: Logarithm of input error: First input must completely greater than 0)", ) testEval("log(uniform(5,8))", "Ok(Sample Set Distribution)") testEval("log10(uniform(5,8))", "Ok(Sample Set Distribution)") diff --git a/packages/squiggle-lang/__tests__/TS/Symbolic_test.ts b/packages/squiggle-lang/__tests__/TS/Symbolic_test.ts index 1bcb1e2e..b949a513 100644 --- a/packages/squiggle-lang/__tests__/TS/Symbolic_test.ts +++ b/packages/squiggle-lang/__tests__/TS/Symbolic_test.ts @@ -12,7 +12,7 @@ describe("Symbolic mean", () => { expect(squiggleResult.value).toBeCloseTo((x + y + z) / 3); } catch (err) { expect((err as Error).message).toEqual( - "Expected squiggle expression to evaluate but got error: Math Error: Triangular values must be increasing order." + "Expected squiggle expression to evaluate but got error: Distribution Math Error: Triangular values must be increasing order." ); } } diff --git a/packages/squiggle-lang/src/rescript/Distributions/DistributionTypes.res b/packages/squiggle-lang/src/rescript/Distributions/DistributionTypes.res index 3a256ea0..1e7164e1 100644 --- a/packages/squiggle-lang/src/rescript/Distributions/DistributionTypes.res +++ b/packages/squiggle-lang/src/rescript/Distributions/DistributionTypes.res @@ -15,6 +15,7 @@ type error = | PointSetConversionError(SampleSetDist.pointsetConversionError) | SparklineError(PointSetTypes.sparklineError) // This type of error is for when we find a sparkline of a discrete distribution. This should probably at some point be actually implemented | RequestedModeInvalidError + | LogarithmOfDistributionError(string) | OtherError(string) @genType @@ -30,6 +31,7 @@ module Error = { | Unreachable => "Unreachable" | DistributionVerticalShiftIsInvalid => "Distribution Vertical Shift is Invalid" | ArgumentError(s) => `Argument Error ${s}` + | LogarithmOfDistributionError(s) => `Logarithm of input error: ${s}` | TooFewSamples => "Too Few Samples" | OperationError(err) => Operation.Error.toString(err) | PointSetConversionError(err) => SampleSetDist.pointsetConversionErrorToString(err) diff --git a/packages/squiggle-lang/src/rescript/Distributions/GenericDist/GenericDist.res b/packages/squiggle-lang/src/rescript/Distributions/GenericDist/GenericDist.res index 71054fe8..6b730b21 100644 --- a/packages/squiggle-lang/src/rescript/Distributions/GenericDist/GenericDist.res +++ b/packages/squiggle-lang/src/rescript/Distributions/GenericDist/GenericDist.res @@ -187,6 +187,49 @@ module AlgebraicCombination = { ->E.R2.fmap(r => DistributionTypes.SampleSet(r)) } + /* + It would be good to also do a check to make sure that probability mass for the second + operand, at value 1.0, is 0 (or approximately 0). However, we'd ideally want to check + that both the probability mass and the probability density are greater than zero. + Right now we don't yet have a way of getting probability mass, so I'll leave this for later. + */ + let getLogarithmInputError = (t1: t, t2: t, ~toPointSetFn: toPointSetFn): option => { + let firstOperandIsGreaterThanZero = + toFloatOperation(t1, ~toPointSetFn, ~distToFloatOperation=#Cdf(1e-10)) |> E.R.fmap(r => + r > 0. + ) + let secondOperandIsGreaterThanZero = + toFloatOperation(t2, ~toPointSetFn, ~distToFloatOperation=#Cdf(1e-10)) |> E.R.fmap(r => + r > 0. + ) + let items = E.A.R.firstErrorOrOpen([ + firstOperandIsGreaterThanZero, + secondOperandIsGreaterThanZero, + ]) + switch items { + | Error(r) => Some(r) + | Ok([true, _]) => + Some(LogarithmOfDistributionError("First input must completely greater than 0")) + | Ok([false, true]) => + Some(LogarithmOfDistributionError("Second input must completely greater than 0")) + | Ok([false, false]) => None + | Ok(_) => Some(Unreachable) + } + } + + let getInvalidOperationError = ( + t1: t, + t2: t, + ~toPointSetFn: toPointSetFn, + ~arithmeticOperation, + ): option => { + if arithmeticOperation == #Logarithm { + getLogarithmInputError(t1, t2, ~toPointSetFn) + } else { + None + } + } + //I'm (Ozzie) really just guessing here, very little idea what's best let expectedConvolutionCost: t => int = x => switch x { @@ -227,10 +270,16 @@ module AlgebraicCombination = { | Some(Ok(symbolicDist)) => Ok(Symbolic(symbolicDist)) | Some(Error(e)) => Error(OperationError(e)) | None => - switch chooseConvolutionOrMonteCarlo(arithmeticOperation, t1, t2) { - | MonteCarlo => runMonteCarlo(toSampleSetFn, arithmeticOperation, t1, t2) - | Convolution(convOp) => - runConvolution(toPointSetFn, convOp, t1, t2)->E.R2.fmap(r => DistributionTypes.PointSet(r)) + switch getInvalidOperationError(t1, t2, ~toPointSetFn, ~arithmeticOperation) { + | Some(e) => Error(e) + | None => + switch chooseConvolutionOrMonteCarlo(arithmeticOperation, t1, t2) { + | MonteCarlo => runMonteCarlo(toSampleSetFn, arithmeticOperation, t1, t2) + | Convolution(convOp) => + runConvolution(toPointSetFn, convOp, t1, t2)->E.R2.fmap(r => DistributionTypes.PointSet( + r, + )) + } } } } diff --git a/packages/squiggle-lang/src/rescript/Reducer/Reducer_ErrorValue.res b/packages/squiggle-lang/src/rescript/Reducer/Reducer_ErrorValue.res index 48fae58c..96b73fd2 100644 --- a/packages/squiggle-lang/src/rescript/Reducer/Reducer_ErrorValue.res +++ b/packages/squiggle-lang/src/rescript/Reducer/Reducer_ErrorValue.res @@ -21,7 +21,7 @@ let errorToString = err => | REAssignmentExpected => "Assignment expected" | REExpressionExpected => "Expression expected" | REFunctionExpected(msg) => `Function expected: ${msg}` - | REDistributionError(err) => `Math Error: ${DistributionTypes.Error.toString(err)}` + | REDistributionError(err) => `Distribution Math Error: ${DistributionTypes.Error.toString(err)}` | REJavaScriptExn(omsg, oname) => { let answer = "JS Exception:" let answer = switch oname {