Merge 7877a17 into 3ee6134

.changeset/violet-kangaroos-begin.md

@@ -0,0 +1,5 @@
+---
+"@quri/squiggle-lang": patch
+---
+
+Fix pointwise combination precision issues on discrete PointSet dists. This was affecting `mixture` and pointwise operators.

.github/workflows/ci.yml

@@ -34,7 +34,7 @@ jobs:
       - name: Install dependencies
         run: pnpm install
       - name: Turbo run
-        run: npx turbo run build test lint bundle
+        run: npx turbo run build test lint
 
   coverage:
     name: Coverage

packages/components/package.json

@@ -35,11 +35,9 @@
     "mermaid": "^10.6.0",
     "prettier": "^3.0.3",
     "react": "^18.2.0",
-    "react-error-boundary": "^4.0.11",
     "react-hook-form": "^7.47.0",
     "react-markdown": "^9.0.0",
     "react-resizable": "^3.0.5",
-    "vscode-uri": "^3.0.8",
     "zod": "^3.22.4"
   },
   "devDependencies": {
@@ -64,7 +62,6 @@
     "@types/node": "^20.8.10",
     "@types/react": "^18.2.34",
     "@types/react-resizable": "^3.0.6",
-    "@types/webpack-bundle-analyzer": "^4.6.2",
     "@typescript-eslint/eslint-plugin": "^6.9.1",
     "@typescript-eslint/parser": "^6.9.1",
     "canvas": "^2.11.2",
@@ -83,13 +80,8 @@
     "storybook": "^7.5.2",
     "tailwindcss": "^3.3.5",
     "ts-jest": "^29.1.1",
-    "ts-loader": "^9.5.0",
     "typescript": "^5.2.2",
-    "vite": "^4.5.0",
-    "web-vitals": "^3.5.0",
-    "webpack": "^5.89.0",
-    "webpack-bundle-analyzer": "^4.9.1",
-    "webpack-cli": "^5.1.4"
+    "vite": "^4.5.0"
   },
   "peerDependencies": {
     "react": "^16.8.0 || ^17 || ^18",
@@ -103,13 +95,11 @@
     "build:lezer": "cd ./src/languageSupport; mkdir -p generated; lezer-generator ./squiggle.grammar --output generated/squiggle.ts",
     "build:storybook": "storybook build",
     "build": "pnpm run build:lezer && pnpm run build:ts && pnpm run build:css && pnpm run build:storybook",
-    "bundle": "webpack",
-    "all": "pnpm bundle && pnpm build",
     "lint": "pnpm lint:prettier && pnpm eslint",
     "lint:prettier": "prettier --check .",
     "eslint": "eslint --ignore-path .gitignore .",
     "format": "prettier --write .",
-    "prepack": "pnpm run build:ts && pnpm run bundle",
+    "prepack": "pnpm run build",
     "test": "NODE_OPTIONS=--experimental-vm-modules jest",
     "test:debug": "node --inspect-brk --experimental-vm-modules node_modules/.bin/jest --runInBand",
     "test:profile": "node --cpu-prof --experimental-vm-modules node_modules/.bin/jest --runInBand"
@@ -159,18 +149,6 @@
       }
     ]
   },
-  "browserslist": {
-    "production": [
-      ">0.2%",
-      "not dead",
-      "not op_mini all"
-    ],
-    "development": [
-      "last 1 chrome version",
-      "last 1 firefox version",
-      "last 1 safari version"
-    ]
-  },
   "files": [
     "dist",
     "src",

packages/squiggle-lang/src/PointSet/Continuous.ts

@@ -298,8 +298,6 @@ export class ContinuousShape implements PointSet<ContinuousShape> {
   }
 }
 
-export const Analysis = {};
-
 const emptyIntegral = () =>
   new ContinuousShape({
     xyShape: {
@@ -378,8 +376,6 @@ export const combinePointwise = <E>(
   );
 };
 
-export const getShape = (t: ContinuousShape) => t.xyShape;
-
 export const sum = (continuousShapes: ContinuousShape[]): ContinuousShape => {
   return continuousShapes.reduce((x, y) => {
     const result = combinePointwise(x, y, (a, b) => Result.Ok(a + b));

packages/squiggle-lang/src/PointSet/Discrete.ts

@@ -248,39 +248,17 @@ export const empty = () =>
     integralCache: emptyIntegral(),
   });
 
-// unused
-export const isFloat = (t: DiscreteShape): boolean => {
-  if (t.xyShape.ys.length === 1 && t.xyShape.ys[0] === 1) {
-    return true;
-  }
-  return false;
-};
-
-export const getShape = (t: DiscreteShape) => t.xyShape;
-
-export const combinePointwise = <E>(
+export function combinePointwise<E>(
   t1: DiscreteShape,
   t2: DiscreteShape,
-  fn: (v1: number, v2: number) => Result.result<number, E>,
-  integralSumCachesFn: (v1: number, v2: number) => number | undefined = () =>
-    undefined
-): Result.result<DiscreteShape, E> => {
-  const combiner = XYShape.PointwiseCombination.combine;
-
-  // const combinedIntegralSum = Common.combineIntegralSums(
-  //   integralSumCachesFn,
-  //   t1.integralSumCache,
-  //   t2.integralSumCache
-  // );
-
-  // TODO: does it ever make sense to pointwise combine the integrals here?
-  // It could be done for pointwise additions, but is that ever needed?
-
+  fn: (v1: number, v2: number) => Result.result<number, E>
+): Result.result<DiscreteShape, E> {
+  // TODO: should we also combine the integrals here?
   return Result.fmap(
-    combiner(XYShape.XtoY.discreteInterpolator, fn, t1.xyShape, t2.xyShape),
-    (x) => new DiscreteShape({ xyShape: x })
+    XYShape.PointwiseCombination.combineDiscrete(fn, t1.xyShape, t2.xyShape),
+    (xyShape) => new DiscreteShape({ xyShape })
   );
-};
+}
 
 /* This multiples all of the data points together and creates a new discrete distribution from the results.
  Data points at the same xs get added together. It may be a good idea to downsample t1 and t2 before and/or the result after. */

packages/squiggle-lang/src/PointSet/Mixed.ts

@@ -363,8 +363,7 @@ export const combinePointwise = <E>(
   const reducedDiscrete = Discrete.combinePointwise(
     t1.toDiscrete(),
     t2.toDiscrete(),
-    fn,
-    integralSumCachesFn
+    fn
   );
 
   const reducedContinuous = Continuous.combinePointwise(

packages/squiggle-lang/src/PointSet/MixedPoint.ts

@@ -1,19 +1,26 @@
+// MixedPoints are used to represent PDFs and scores.
 export type MixedPoint = {
   continuous: number;
   discrete: number;
 };
 
-export const makeContinuous = (f: number): MixedPoint => ({
-  continuous: f,
-  discrete: 0,
-});
+export function makeContinuous(f: number): MixedPoint {
+  return {
+    continuous: f,
+    discrete: 0,
+  };
+}
 
-export const makeDiscrete = (f: number): MixedPoint => ({
-  continuous: 0,
-  discrete: f,
-});
+export function makeDiscrete(f: number): MixedPoint {
+  return {
+    continuous: 0,
+    discrete: f,
+  };
+}
 
-export const add = (p1: MixedPoint, p2: MixedPoint): MixedPoint => ({
-  continuous: p1.continuous + p2.continuous,
-  discrete: p1.discrete + p2.discrete,
-});
+export function add(p1: MixedPoint, p2: MixedPoint): MixedPoint {
+  return {
+    continuous: p1.continuous + p2.continuous,
+    discrete: p1.discrete + p2.discrete,
+  };
+}

packages/squiggle-lang/src/PointSet/PointSet.ts

@@ -80,7 +80,7 @@ export const combinePointwise = <E>(
       integralSumCachesFn
     );
   } else if (t1 instanceof DiscreteShape && t2 instanceof DiscreteShape) {
-    return Discrete.combinePointwise(t1, t2, fn, integralSumCachesFn);
+    return Discrete.combinePointwise(t1, t2, fn);
   } else {
     return Mixed.combinePointwise(
       t1.toMixed(),

packages/squiggle-lang/src/XYShape.ts

@@ -398,9 +398,6 @@ export const XtoY = {
       ); // should never happen
     }
   },
-  /* Returns a between-points-interpolating function that can be used with PointwiseCombination.combine.
-     For discrete distributions, the probability density between points is zero, so we just return zero here. */
-  discreteInterpolator: (() => 0) as Interpolator,
 };
 
 export const XsConversion = {
@@ -532,6 +529,55 @@ export const PointwiseCombination = {
     return Result.Ok({ xs: outX, ys: outY });
   },
 
+  combineDiscrete<E>(
+    fn: (a: number, b: number) => Result.result<number, E>,
+    t1: XYShape,
+    t2: XYShape
+  ): Result.result<XYShape, E> {
+    let i1 = 0,
+      i2 = 0;
+    const xs1 = t1.xs,
+      xs2 = t2.xs;
+    const ys1 = t1.ys,
+      ys2 = t2.ys;
+    const len1 = xs1.length,
+      len2 = xs2.length;
+
+    const xs: number[] = [];
+    const ys: number[] = [];
+    while (i1 < len1 || i2 < len2) {
+      let x: number, y1: number, y2: number;
+      if (i2 == len2 || (i1 < len1 && xs1[i1] < xs2[i2])) {
+        // take from xs1
+        x = xs1[i1];
+        y1 = ys1[i1];
+        y2 = 0;
+        i1++;
+      } else if (i1 < len1 && xs1[i1] == xs2[i2]) {
+        // combine
+        x = xs1[i1];
+        y1 = ys1[i1];
+        y2 = ys2[i2];
+        i1++;
+        i2++;
+      } else {
+        // take from xs2
+        x = xs2[i2];
+        y1 = 0;
+        y2 = ys2[i2];
+        i2++;
+      }
+      const result = fn(y1, y2);
+      if (!result.ok) {
+        return result; // combiner function has failed
+      }
+      xs.push(x);
+      ys.push(result.value);
+    }
+
+    return Result.Ok({ xs, ys });
+  },
+
   addCombine(interpolator: Interpolator, t1: XYShape, t2: XYShape): XYShape {
     const result = PointwiseCombination.combine(
       interpolator,
@@ -577,47 +623,6 @@ export const Range = {
 
     return { xs: newXs, ys: newYs };
   },
-
-  // Unused code:
-
-  // I'm really not sure this part is actually what we want at this point.
-  // ((lastX, lastY), (nextX, nextY))
-  // type ZippedRange = [[number, number], [number, number]];
-  // derivative(t: XYShape) {
-  //   return Range.mapYsBasedOnRanges(t, Range.delta_y_over_delta_x);
-  // },
-
-  // nextX([, [nextX]]: ZippedRange) {
-  //   return nextX;
-  // },
-
-  // rangePointAssumingSteps([[, lastY], [nextX]]: ZippedRange) {
-  //   return [nextX, lastY];
-  // },
-
-  // rangeAreaAssumingTriangles([[lastX, lastY], [nextX, nextY]]: ZippedRange) {
-  //   return ((nextX - lastX) * (lastY + nextY)) / 2;
-  // },
-
-  // //Todo: figure out how to without making new array.
-  // rangeAreaAssumingTrapezoids([[lastX, lastY], [nextX, nextY]]: ZippedRange) {
-  //   return (nextX - lastX) * (Math.min(lastY, nextY) + (lastY + nextY) / 2);
-  // },
-
-  // delta_y_over_delta_x([[lastX, lastY], [nextX, nextY]]: ZippedRange) {
-  //   return (nextY - lastY) / (nextX - lastX);
-  // },
-
-  // mapYsBasedOnRanges(
-  //   t: XYShape,
-  //   fn: (r: ZippedRange) => number
-  // ): [number, number][] | undefined {
-  //   const ranges = E_A.toRanges(E_A.zip(t.xs, t.ys));
-  //   if (!ranges.ok) {
-  //     return undefined; // probably length=1
-  //   }
-  //   return ranges.value.map((r) => [Range.nextX(r), fn(r)]);
-  // },
 };
 
 export const Analysis = {

packages/squiggle-lang/src/dist/PointSetDist.ts

@@ -100,7 +100,7 @@ export class PointSetDist extends BaseDist {
       );
     }
     const downsampled = continuous.downsampleEquallyOverX(bucketCount);
-    return Result.Ok(createSparkline(Continuous.getShape(downsampled).ys));
+    return Result.Ok(createSparkline(downsampled.xyShape.ys));
   }
 
   // PointSet-only methods