Cleanup and commenting for PR

packages/squiggle-lang/src/js/index.ts

@@ -9,12 +9,12 @@ export type { SamplingInputs, exportEnv, exportDistribution };
 export type { t as DistPlus } from "../rescript/OldInterpreter/DistPlus.gen";
 import {
   genericDist,
+  env,
   resultDist,
   resultFloat,
   resultString,
-} from "../rescript/TSInterface.gen";
+} from "../rescript/TypescriptInterface.gen";
 import {
-  env,
   Constructors_mean,
   Constructors_sample,
   Constructors_pdf,
@@ -59,8 +59,9 @@ export function run(
   return runAll(squiggleString, si, env);
 }
 
-export function resultMap(
-  r:
+//This is clearly not fully typed. I think later we should use a functional library to
+// provide a better Either type and corresponding functions.
+type result =
   | {
       tag: "Ok";
       value: any;
@@ -68,17 +69,9 @@ export function resultMap(
   | {
       tag: "Error";
       value: any;
-      },
-  mapFn: any
-):
-  | {
-      tag: "Ok";
-      value: any;
-    }
-  | {
-      tag: "Error";
-      value: any;
-    } {
+    };
+
+export function resultMap(r: result, mapFn: any): result {
   if (r.tag === "Ok") {
     return { tag: "Ok", value: mapFn(r.value) };
   } else {

packages/squiggle-lang/src/rescript/Distributions/DistributionOperation/DistributionOperation.res

@@ -114,8 +114,8 @@ let rec run = (~env, functionCallInfo: functionCallInfo): outputType => {
       ->E.R2.fmap(r => Float(r))
       ->OutputLocal.fromResult
     | ToString(ToString) => dist->GenericDist.toString->String
-    | ToString(ToSparkline(buckets)) =>
-      GenericDist.toSparkline(dist, ~sampleCount, ~buckets, ())
+    | ToString(ToSparkline(bucketCount)) =>
+      GenericDist.toSparkline(dist, ~sampleCount, ~bucketCount, ())
       ->E.R2.fmap(r => String(r))
       ->OutputLocal.fromResult
     | ToDist(Inspect) => {
@@ -186,8 +186,11 @@ module Output = {
   }
 }
 
+// See comment above GenericDist_Types.Constructors to explain the purpose of this module.
+// I tried having another internal module called UsingDists, similar to how its done in
+// GenericDist_Types.Constructors. However, this broke GenType for me, so beware.
 module Constructors = {
-    module C = GenericDist_Types.Constructors.UsingDists;
+  module C = GenericDist_Types.Constructors.UsingDists
   open OutputLocal
   let mean = (~env, dist) => C.mean(dist)->run(~env)->toFloatR
   let sample = (~env, dist) => C.sample(dist)->run(~env)->toFloatR
@@ -201,12 +204,11 @@ module Constructors = {
     C.truncate(dist, leftCutoff, rightCutoff)->run(~env)->toDistR
   let inspect = (~env, dist) => C.inspect(dist)->run(~env)->toDistR
   let toString = (~env, dist) => C.toString(dist)->run(~env)->toStringR
-    let toSparkline = (~env, dist, buckets) => C.toSparkline(dist, buckets)->run(~env)->toStringR
+  let toSparkline = (~env, dist, bucketCount) => C.toSparkline(dist, bucketCount)->run(~env)->toStringR
   let algebraicAdd = (~env, dist1, dist2) => C.algebraicAdd(dist1, dist2)->run(~env)->toDistR
   let algebraicMultiply = (~env, dist1, dist2) =>
     C.algebraicMultiply(dist1, dist2)->run(~env)->toDistR
-    let algebraicDivide = (~env, dist1, dist2) =>
-      C.algebraicDivide(dist1, dist2)->run(~env)->toDistR
+  let algebraicDivide = (~env, dist1, dist2) => C.algebraicDivide(dist1, dist2)->run(~env)->toDistR
   let algebraicSubtract = (~env, dist1, dist2) =>
     C.algebraicSubtract(dist1, dist2)->run(~env)->toDistR
   let algebraicLogarithm = (~env, dist1, dist2) =>
@@ -216,8 +218,7 @@ module Constructors = {
   let pointwiseAdd = (~env, dist1, dist2) => C.pointwiseAdd(dist1, dist2)->run(~env)->toDistR
   let pointwiseMultiply = (~env, dist1, dist2) =>
     C.pointwiseMultiply(dist1, dist2)->run(~env)->toDistR
-    let pointwiseDivide = (~env, dist1, dist2) =>
-      C.pointwiseDivide(dist1, dist2)->run(~env)->toDistR
+  let pointwiseDivide = (~env, dist1, dist2) => C.pointwiseDivide(dist1, dist2)->run(~env)->toDistR
   let pointwiseSubtract = (~env, dist1, dist2) =>
     C.pointwiseSubtract(dist1, dist2)->run(~env)->toDistR
   let pointwiseLogarithm = (~env, dist1, dist2) =>

packages/squiggle-lang/src/rescript/Distributions/GenericDist/GenericDist.res

@@ -81,11 +81,17 @@ let toPointSet = (
   }
 }
 
-let toSparkline = (t: t, ~sampleCount: int, ~buckets: int=20, unit): result<string, error> =>
+/*
+  PointSetDist.toSparkline calls "downsampleEquallyOverX", which downsamples it to n=bucketCount.
+  It first needs a pointSetDist, so we convert to a pointSetDist. In this process we want the 
+  xyPointLength to be a bit longer than the eventual toSparkline downsampling. I chose 3 
+  fairly arbitrarily.
+ */
+let toSparkline = (t: t, ~sampleCount: int, ~bucketCount: int=20, unit): result<string, error> =>
   t
-  ->toPointSet(~xSelection=#Linear, ~xyPointLength=buckets * 3, ~sampleCount, ())
+  ->toPointSet(~xSelection=#Linear, ~xyPointLength=bucketCount * 3, ~sampleCount, ())
   ->E.R.bind(r =>
-    r->PointSetDist.toSparkline(buckets)->E.R2.errMap(r => Error(GenericDist_Types.Other(r)))
+    r->PointSetDist.toSparkline(bucketCount)->E.R2.errMap(r => Error(GenericDist_Types.Other(r)))
   )
 
 module Truncate = {

packages/squiggle-lang/src/rescript/Distributions/GenericDist/GenericDist.resi

@@ -26,7 +26,7 @@ let toPointSet: (
   ~xSelection: GenericDist_Types.Operation.pointsetXSelection=?,
   unit,
 ) => result<PointSetTypes.pointSetDist, error>
-let toSparkline: (t, ~sampleCount: int, ~buckets: int=?, unit) => result<string, error>
+let toSparkline: (t, ~sampleCount: int, ~bucketCount: int=?, unit) => result<string, error>
 
 let truncate: (
   t,

packages/squiggle-lang/src/rescript/Distributions/GenericDist/GenericDist_Types.res

@@ -98,6 +98,14 @@ module Operation = {
     }
 }
 
+/*
+It can be a pain to write out the genericFunctionCallInfo. The constructors help with this. 
+This code only covers some of genericFunctionCallInfo: many arguments could be called with either a
+float or a distribution. The "UsingDists" module assumes that everything is a distribution.
+This is a tradeoff of some generality in order to get a bit more simplicity.
+I could see having a longer interface in the future, but it could be messy.
+Like, algebraicAddDistFloat vs. algebraicAddDistDist
+*/
 module Constructors = {
   type t = Operation.genericFunctionCallInfo
 
@@ -105,9 +113,9 @@ module Constructors = {
     @genType
     let mean = (dist): t => FromDist(ToFloat(#Mean), dist)
     let sample = (dist): t => FromDist(ToFloat(#Sample), dist)
-    let cdf = (dist, f): t => FromDist(ToFloat(#Cdf(f)), dist)
-    let inv = (dist, f): t => FromDist(ToFloat(#Inv(f)), dist)
-    let pdf = (dist, f): t => FromDist(ToFloat(#Pdf(f)), dist)
+    let cdf = (dist, x): t => FromDist(ToFloat(#Cdf(x)), dist)
+    let inv = (dist, x): t => FromDist(ToFloat(#Inv(x)), dist)
+    let pdf = (dist, x): t => FromDist(ToFloat(#Pdf(x)), dist)
     let normalize = (dist): t => FromDist(ToDist(Normalize), dist)
     let toPointSet = (dist): t => FromDist(ToDist(ToPointSet), dist)
     let toSampleSet = (dist, r): t => FromDist(ToDist(ToSampleSet(r)), dist)
@@ -165,63 +173,3 @@ module Constructors = {
     )
   }
 }
-
-module DistVariant = {
-  type t =
-    | Mean(genericDist)
-    | Sample(genericDist)
-    | Cdf(genericDist, float)
-    | Inv(genericDist, float)
-    | Pdf(genericDist, float)
-    | Normalize(genericDist)
-    | ToPointSet(genericDist)
-    | ToSampleSet(genericDist, int)
-    | Truncate(genericDist, option<float>, option<float>)
-    | Inspect(genericDist)
-    | ToString(genericDist)
-    | ToSparkline(genericDist, int)
-    | AlgebraicAdd(genericDist, genericDist)
-    | AlgebraicMultiply(genericDist, genericDist)
-    | AlgebraicDivide(genericDist, genericDist)
-    | AlgebraicSubtract(genericDist, genericDist)
-    | AlgebraicLogarithm(genericDist, genericDist)
-    | AlgebraicExponentiate(genericDist, genericDist)
-    | PointwiseAdd(genericDist, genericDist)
-    | PointwiseMultiply(genericDist, genericDist)
-    | PointwiseDivide(genericDist, genericDist)
-    | PointwiseSubtract(genericDist, genericDist)
-    | PointwiseLogarithm(genericDist, genericDist)
-    | PointwiseExponentiate(genericDist, genericDist)
-  
-  let toGenericFunctionCallInfo = (t: t) =>
-    switch t {
-    | Mean(d) => Operation.FromDist(ToFloat(#Mean), d)
-    | Sample(d) => FromDist(ToFloat(#Mean), d)
-    | Cdf(d, f) => FromDist(ToFloat(#Cdf(f)), d)
-    | Inv(d, f) => FromDist(ToFloat(#Inv(f)), d)
-    | Pdf(d, f) => FromDist(ToFloat(#Pdf(f)), d)
-    | Normalize(d) => FromDist(ToDist(Normalize), d)
-    | ToPointSet(d) => FromDist(ToDist(ToPointSet), d)
-    | ToSampleSet(d, r) => FromDist(ToDist(ToSampleSet(r)), d)
-    | Truncate(d, left, right) => FromDist(ToDist(Truncate(left, right)), d)
-    | Inspect(d) => FromDist(ToDist(Inspect), d)
-    | ToString(d) => FromDist(ToString(ToString), d)
-    | ToSparkline(d, n) => FromDist(ToString(ToSparkline(n)), d)
-    | AlgebraicAdd(d1, d2) => FromDist(ToDistCombination(Algebraic, #Add, #Dist(d2)), d1)
-    | AlgebraicMultiply(d1, d2) => FromDist(ToDistCombination(Algebraic, #Multiply, #Dist(d2)), d1)
-    | AlgebraicDivide(d1, d2) => FromDist(ToDistCombination(Algebraic, #Divide, #Dist(d2)), d1)
-    | AlgebraicSubtract(d1, d2) => FromDist(ToDistCombination(Algebraic, #Subtract, #Dist(d2)), d1)
-    | AlgebraicLogarithm(d1, d2) =>
-      FromDist(ToDistCombination(Algebraic, #Logarithm, #Dist(d2)), d1)
-    | AlgebraicExponentiate(d1, d2) =>
-      FromDist(ToDistCombination(Algebraic, #Exponentiate, #Dist(d2)), d1)
-    | PointwiseAdd(d1, d2) => FromDist(ToDistCombination(Pointwise, #Add, #Dist(d2)), d1)
-    | PointwiseMultiply(d1, d2) => FromDist(ToDistCombination(Pointwise, #Multiply, #Dist(d2)), d1)
-    | PointwiseDivide(d1, d2) => FromDist(ToDistCombination(Pointwise, #Divide, #Dist(d2)), d1)
-    | PointwiseSubtract(d1, d2) => FromDist(ToDistCombination(Pointwise, #Subtract, #Dist(d2)), d1)
-    | PointwiseLogarithm(d1, d2) =>
-      FromDist(ToDistCombination(Pointwise, #Logarithm, #Dist(d2)), d1)
-    | PointwiseExponentiate(d1, d2) =>
-      FromDist(ToDistCombination(Pointwise, #Exponentiate, #Dist(d2)), d1)
-    }
-}

packages/squiggle-lang/src/rescript/Distributions/PointSetDist/PointSetDist.res

@@ -203,8 +203,8 @@ let operate = (distToFloatOp: Operation.distToFloatOperation, s): float =>
   | #Mean => T.mean(s)
   }
 
-let toSparkline = (t: t, n) =>
+let toSparkline = (t: t, bucketCount) =>
   T.toContinuous(t)
-  ->E.O2.fmap(Continuous.downsampleEquallyOverX(n))
+  ->E.O2.fmap(Continuous.downsampleEquallyOverX(bucketCount))
   ->E.O2.toResult("toContinous Error: Could not convert into continuous distribution")
   ->E.R2.fmap(r => Continuous.getShape(r).ys->Sparklines.create())

packages/squiggle-lang/src/rescript/Distributions/SampleSetDist/SampleSet.res

@@ -146,11 +146,20 @@ let toPointSetDist = (
   samplesParse
 }
 
+//Randomly get one sample from the distribution
 let sample = (t: t): float => {
   let i = E.Int.random(~min=0, ~max=E.A.length(t) - 1)
   E.A.unsafe_get(t, i)
 }
 
+/*
+If asked for a length of samples shorter or equal the length of the distribution,
+return this first n samples of this distribution.
+Else, return n random samples of the distribution.
+The former helps in cases where multiple distributions are correlated.
+However, if n > length(t), then there's no clear right answer, so we just randomly
+sample everything.
+*/
 let sampleN = (t: t, n) => {
   if n <= E.A.length(t) {
     E.A.slice(t, ~offset=0, ~len=n)

packages/squiggle-lang/src/rescript/TSInterface.res

@@ -1,21 +0,0 @@
-@genType
-type functionCallInfo = GenericDist_Types.Operation.genericFunctionCallInfo;
-
-@genType
-type env = DistributionOperation.env;
-
-@genType
-type genericDist = GenericDist_Types.genericDist;
-
-@genType
-type error = GenericDist_Types.error;
-
-@genType
-let runDistributionOperation = DistributionOperation.run;
-
-@genType
-type resultDist = result<genericDist, error>
-@genType
-type resultFloat = result<float, error>
-@genType
-type resultString = result<string, error>

packages/squiggle-lang/src/rescript/TypescriptInterface.res

@@ -0,0 +1,24 @@
+/*
+This is meant as a file to contain @genType declarations as needed for Typescript.
+I would ultimately want to have all @genType declarations here, vs. other files, but
+@genType doesn't play as nicely with renaming Modules and functions as
+would be preferable. 
+
+The below few seem to work fine. In the future there's definitely more work to do here.
+*/
+
+@genType
+type env = DistributionOperation.env
+
+@genType
+type genericDist = GenericDist_Types.genericDist
+
+@genType
+type error = GenericDist_Types.error
+
+@genType
+type resultDist = result<genericDist, error>
+@genType
+type resultFloat = result<float, error>
+@genType
+type resultString = result<string, error>