From 2df470839ccbe7c57190b4fa5f444a77f7ea775a Mon Sep 17 00:00:00 2001 From: Ozzie Gooen Date: Wed, 11 Oct 2023 15:41:00 -0700 Subject: [PATCH] Better split UserDefinedLambda and BuiltInLambda --- .../Calculator/calculatorReducer.ts | 9 +-- .../src/components/Calculator/index.tsx | 3 +- .../src/components/FunctionChart/index.tsx | 3 +- .../SquiggleViewer/ExpressionViewer.tsx | 2 +- .../__tests__/library/plot_test.ts | 4 +- packages/squiggle-lang/src/fr/mixture.ts | 2 +- packages/squiggle-lang/src/fr/plot.ts | 10 +++- packages/squiggle-lang/src/index.ts | 2 +- .../src/library/registry/core.ts | 30 +--------- .../src/public/SqValue/SqCalculator.ts | 9 +++ .../src/public/SqValue/SqLambda.ts | 56 ++++++++++++++----- packages/squiggle-lang/src/reducer/index.ts | 8 +-- packages/squiggle-lang/src/reducer/lambda.ts | 42 ++++++-------- packages/squiggle-lang/src/value/index.ts | 30 ++++++---- 14 files changed, 111 insertions(+), 99 deletions(-) diff --git a/packages/components/src/components/Calculator/calculatorReducer.ts b/packages/components/src/components/Calculator/calculatorReducer.ts index d8dc960f8d..ca11fa7138 100644 --- a/packages/components/src/components/Calculator/calculatorReducer.ts +++ b/packages/components/src/components/Calculator/calculatorReducer.ts @@ -27,14 +27,9 @@ export type CalculatorState = { // This function is used to determine if a calculator has changed. // It's obviously not perfect - it doesn't capture changes within the calculator function, but this would be much more complicated. -export function calculatorHash(calc: SqCalculator): string { - const rowData = JSON.stringify(calc.fields); - const paramData = (calc.parameters || []).join(","); - return rowData + paramData + calc.description; -} export function hasSameCalculator(state: CalculatorState, calc: SqCalculator) { - return calculatorHash(calc) === state.hash; + return calc.hashString === state.hash; } export function allFields(state: CalculatorState): FieldValue[] { @@ -65,7 +60,7 @@ export function initialCalculatorState( fieldNames: calculator.fields.map((row) => row.name), fields, fn: {}, - hash: calculatorHash(calculator), + hash: calculator.hashString, }; } diff --git a/packages/components/src/components/Calculator/index.tsx b/packages/components/src/components/Calculator/index.tsx index 14d9475a63..79a7865439 100644 --- a/packages/components/src/components/Calculator/index.tsx +++ b/packages/components/src/components/Calculator/index.tsx @@ -16,7 +16,6 @@ import { Action, useViewerContext } from "../SquiggleViewer/ViewerProvider.js"; import { CalculatorAction, CalculatorState, - calculatorHash, calculatorReducer, hasSameCalculator, initialCalculatorState, @@ -106,7 +105,7 @@ export const Calculator: FC = ({ useEffect(() => { const calculatorChanged = prevCalculator !== null && - calculatorHash(calculator) !== calculatorHash(prevCalculator); + calculator.hashString !== prevCalculator.hashString; if (calculatorChanged) { calculatorDispatch({ diff --git a/packages/components/src/components/FunctionChart/index.tsx b/packages/components/src/components/FunctionChart/index.tsx index 6928fad489..ca206e5839 100644 --- a/packages/components/src/components/FunctionChart/index.tsx +++ b/packages/components/src/components/FunctionChart/index.tsx @@ -59,7 +59,8 @@ export const FunctionChart: FC = ({ ); } - const domain = parameters[0].domain; + const definitions = fn.definitions(); + const domain = definitions[0][0]?.domain; const min = domain?.min ?? settings.functionChartSettings.start; const max = domain?.max ?? settings.functionChartSettings.stop; diff --git a/packages/components/src/components/SquiggleViewer/ExpressionViewer.tsx b/packages/components/src/components/SquiggleViewer/ExpressionViewer.tsx index 64206695a6..0b33c9800a 100644 --- a/packages/components/src/components/SquiggleViewer/ExpressionViewer.tsx +++ b/packages/components/src/components/SquiggleViewer/ExpressionViewer.tsx @@ -188,7 +188,7 @@ export const getBoxProps = (
fn( - {truncateStr(value.value.parameterNames().join(", "), 15)} + {truncateStr(value.value.parameterString(), 15)} )
diff --git a/packages/squiggle-lang/__tests__/library/plot_test.ts b/packages/squiggle-lang/__tests__/library/plot_test.ts index caf12d5e49..9520d93da6 100644 --- a/packages/squiggle-lang/__tests__/library/plot_test.ts +++ b/packages/squiggle-lang/__tests__/library/plot_test.ts @@ -47,7 +47,7 @@ describe("Plot", () => { `Plot.numericFn({ fn: {|x, y| x * y} })`, - "Plots only work with functions that have one parameter. This function has 2 parameters." + "Error(Error: Plots only work with functions that have one parameter. This function only supports [2] parameters.)" ); testPlotResult( @@ -129,7 +129,7 @@ describe("Plot", () => { `Plot.distFn({ fn: {|x,y| x to x + y} })`, - "Plots only work with functions that have one parameter. This function has 2 parameters." + "Error(Error: Plots only work with functions that have one parameter. This function only supports [2] parameters.)" ); }); diff --git a/packages/squiggle-lang/src/fr/mixture.ts b/packages/squiggle-lang/src/fr/mixture.ts index 2cb72a05f2..b6c81be09d 100644 --- a/packages/squiggle-lang/src/fr/mixture.ts +++ b/packages/squiggle-lang/src/fr/mixture.ts @@ -1,5 +1,5 @@ import { BaseDist } from "../dist/BaseDist.js"; -import { DistError, argumentError } from "../dist/DistError.js"; +import { argumentError } from "../dist/DistError.js"; import * as SymbolicDist from "../dist/SymbolicDist.js"; import * as distOperations from "../dist/distOperations/index.js"; import { Env } from "../dist/env.js"; diff --git a/packages/squiggle-lang/src/fr/plot.ts b/packages/squiggle-lang/src/fr/plot.ts index 8795327a1f..2e9d55ba38 100644 --- a/packages/squiggle-lang/src/fr/plot.ts +++ b/packages/squiggle-lang/src/fr/plot.ts @@ -15,7 +15,7 @@ import { frString, } from "../library/registry/frTypes.js"; import { FnFactory } from "../library/registry/helpers.js"; -import { Lambda } from "../reducer/lambda.js"; +import { Lambda, UserDefinedLambda } from "../reducer/lambda.js"; import { LabeledDistribution, Scale, VDomain, vPlot } from "../value/index.js"; const maker = new FnFactory({ @@ -78,7 +78,13 @@ function extractDomainFromOneArgFunction(fn: Lambda): VDomain | undefined { )}] parameters.` ); } - const domain = fn.getParameters()[0]?.domain; + + let domain; + if (fn instanceof UserDefinedLambda) { + domain = fn.parameters[0]?.domain; + } else { + domain = undefined; + } // We could also verify a domain here, to be more confident that the function expects numeric args. // But we might get other numeric domains besides `NumericRange`, so checking domain type here would be risky. return domain; diff --git a/packages/squiggle-lang/src/index.ts b/packages/squiggle-lang/src/index.ts index a2509a7fd5..44e28fd3cf 100644 --- a/packages/squiggle-lang/src/index.ts +++ b/packages/squiggle-lang/src/index.ts @@ -28,7 +28,7 @@ export { type SqDistribution, } from "./public/SqValue/SqDistribution/index.js"; export { type SqDomain } from "./public/SqValue/SqDomain.js"; -export { SqLambda } from "./public/SqValue/SqLambda.js"; +export { SqLambda, type SqLambdaParameter } from "./public/SqValue/SqLambda.js"; export { SqDictValue } from "./public/SqValue/index.js"; export { SqDistFnPlot, diff --git a/packages/squiggle-lang/src/library/registry/core.ts b/packages/squiggle-lang/src/library/registry/core.ts index e3805a10cf..d6a57020b7 100644 --- a/packages/squiggle-lang/src/library/registry/core.ts +++ b/packages/squiggle-lang/src/library/registry/core.ts @@ -1,12 +1,6 @@ -import { REOther, RESymbolNotFound } from "../../errors/messages.js"; -import { ReducerContext } from "../../reducer/context.js"; import { BuiltinLambda, Lambda } from "../../reducer/lambda.js"; import { Value } from "../../value/index.js"; -import { - FnDefinition, - fnDefinitionToString, - tryCallFnDefinition, -} from "./fnDefinition.js"; +import { FnDefinition } from "./fnDefinition.js"; export type FRFunction = { name: string; @@ -68,28 +62,6 @@ export class Registry { return [...this.fnNameDict.keys()]; } - call(fnName: string, args: Value[], context: ReducerContext): Value { - const definitions = this.fnNameDict.get(fnName); - if (definitions === undefined) { - throw new RESymbolNotFound(fnName); - } - const showNameMatchDefinitions = () => { - const defsString = definitions - .map(fnDefinitionToString) - .map((def) => ` ${fnName}${def}\n`) - .join(""); - return `There are function matches for ${fnName}(), but with different arguments:\n${defsString}`; - }; - - for (const definition of definitions) { - const callResult = tryCallFnDefinition(definition, args, context); - if (callResult !== undefined) { - return callResult; - } - } - throw new REOther(showNameMatchDefinitions()); - } - makeLambda(fnName: string): Lambda { if (!this.fnNameDict.has(fnName)) { throw new Error(`Function ${fnName} doesn't exist in registry`); diff --git a/packages/squiggle-lang/src/public/SqValue/SqCalculator.ts b/packages/squiggle-lang/src/public/SqValue/SqCalculator.ts index 4050c9a6e7..1f1c2825eb 100644 --- a/packages/squiggle-lang/src/public/SqValue/SqCalculator.ts +++ b/packages/squiggle-lang/src/public/SqValue/SqCalculator.ts @@ -34,6 +34,15 @@ export class SqCalculator { return this._value.description; } + // This function is used to determine if a calculator has changed. + // It's obviously not perfect - it doesn't capture changes within the calculator function, but this would be much more complicated. + + get hashString(): string { + const rowData = JSON.stringify(this._value.fields); + const paramData = this._value.fn.toString() || ""; + return rowData + paramData + this._value.description; + } + get fields(): { name: string; default: string; diff --git a/packages/squiggle-lang/src/public/SqValue/SqLambda.ts b/packages/squiggle-lang/src/public/SqValue/SqLambda.ts index 7a66318018..98078939b1 100644 --- a/packages/squiggle-lang/src/public/SqValue/SqLambda.ts +++ b/packages/squiggle-lang/src/public/SqValue/SqLambda.ts @@ -2,15 +2,48 @@ import { Env } from "../../dist/env.js"; import { IRuntimeError } from "../../errors/IError.js"; import { getStdLib } from "../../library/index.js"; import { createContext } from "../../reducer/context.js"; -import { Lambda } from "../../reducer/lambda.js"; +import { + BuiltinLambda, + Lambda, + UserDefinedLambda, +} from "../../reducer/lambda.js"; import * as Result from "../../utility/result.js"; import { result } from "../../utility/result.js"; import { SqError, SqOtherError, SqRuntimeError } from "../SqError.js"; import { SqValueContext } from "../SqValueContext.js"; -import { wrapDomain } from "./SqDomain.js"; +import { SqDomain, wrapDomain } from "./SqDomain.js"; import { SqValue, wrapValue } from "./index.js"; +export type SqLambdaParameter = { + name: string; + domain?: SqDomain; + typeName?: string; +}; + +function lambdaToSqLambdaParameters(lambda: Lambda): SqLambdaParameter[][] { + if (lambda instanceof UserDefinedLambda) { + return [ + lambda.parameters.map((param) => { + return { + name: param.name, + domain: param.domain ? wrapDomain(param.domain.value) : undefined, + }; + }), + ]; + } else if (lambda instanceof BuiltinLambda) { + return lambda.definitions().map((def) => + def.map((p, index) => ({ + name: index.toString(), + domain: undefined, + typeName: p.getName(), + })) + ); + } else { + throw new Error("Unknown lambda type"); + } +} + export class SqLambda { constructor( public _value: Lambda, // public because of SqFnPlot.create @@ -28,19 +61,12 @@ export class SqLambda { return new SqLambda(value.value); } - parameters() { - return this._value.getParameters().map((parameter) => { - return { - name: parameter.name, - domain: parameter.domain - ? wrapDomain(parameter.domain.value) - : undefined, - }; - }); + paramCounts() { + return this._value.paramCounts(); } - parameterNames() { - return this.parameters().map((parameter) => parameter.name); + definitions(): SqLambdaParameter[][] { + return lambdaToSqLambdaParameters(this._value); } call(args: SqValue[], env?: Env): result { @@ -68,4 +94,8 @@ export class SqLambda { toString() { return this._value.toString(); } + + parameterString() { + return this._value.parameterString(); + } } diff --git a/packages/squiggle-lang/src/reducer/index.ts b/packages/squiggle-lang/src/reducer/index.ts index af18296058..673f687a15 100644 --- a/packages/squiggle-lang/src/reducer/index.ts +++ b/packages/squiggle-lang/src/reducer/index.ts @@ -28,7 +28,7 @@ import { vVoid, } from "../value/index.js"; import * as Context from "./context.js"; -import { LambdaParameter, SquiggleLambda } from "./lambda.js"; +import { UserDefinedLambdaParameter, UserDefinedLambda } from "./lambda.js"; export type ReducerFn = ( expression: Expression, @@ -93,7 +93,7 @@ const evaluateBlock: SubReducerFn<"Block"> = (statements, context) => { * We could call `bindings.extend()` here, but we don't, since scopes are costly and bindings are immutable anyway. * So we just have to be careful to throw away block's bindings at the end of a block scope and return the original context. * Note: We'll have to remove this optimization if we add any kind of `locals()` (like in Python) function or debugging utilities. - * See also: similar note in `SquiggleLambda` constructor. + * See also: similar note in `UserDefinedLambda` constructor. */ let currentContext = context; let currentValue: Value = vVoid(); @@ -207,7 +207,7 @@ const evaluateLambda: SubReducerFn<"Lambda"> = ( context, ast ) => { - const parameters: LambdaParameter[] = []; + const parameters: UserDefinedLambdaParameter[] = []; for (const parameterExpression of expressionValue.parameters) { let domain: VDomain | undefined; // Processing annotations, e.g. f(x: [3, 5]) = { ... } @@ -238,7 +238,7 @@ const evaluateLambda: SubReducerFn<"Lambda"> = ( }); } const value = vLambda( - new SquiggleLambda( + new UserDefinedLambda( expressionValue.name, parameters, context.stack, diff --git a/packages/squiggle-lang/src/reducer/lambda.ts b/packages/squiggle-lang/src/reducer/lambda.ts index ff34ee86ce..6fdbea0f6a 100644 --- a/packages/squiggle-lang/src/reducer/lambda.ts +++ b/packages/squiggle-lang/src/reducer/lambda.ts @@ -2,12 +2,7 @@ import { LocationRange } from "peggy"; import { ASTNode } from "../ast/parse.js"; import * as IError from "../errors/IError.js"; -import { - REArityError, - REDomainError, - REOther, - RESymbolNotFound, -} from "../errors/messages.js"; +import { REArityError, REDomainError, REOther } from "../errors/messages.js"; import { Expression } from "../expression/index.js"; import { VDomain, Value } from "../value/index.js"; import * as Context from "./context.js"; @@ -20,8 +15,9 @@ import { } from "../library/registry/fnDefinition.js"; import uniq from "lodash/uniq.js"; import { sort } from "../utility/E_A_Floats.js"; +import { FRType } from "../library/registry/frTypes.js"; -export type LambdaParameter = { +export type UserDefinedLambdaParameter = { name: string; domain?: VDomain; // should this be Domain instead of VDomain? }; @@ -31,8 +27,8 @@ type LambdaBody = (args: Value[], context: ReducerContext) => Value; export abstract class Lambda { constructor(public body: LambdaBody) {} + abstract type: string; abstract getName(): string; - abstract getParameters(): LambdaParameter[]; abstract toString(): string; abstract parameterString(): string; abstract paramCounts(): number[]; @@ -68,14 +64,15 @@ export abstract class Lambda { } // User-defined functions, e.g. `add2 = {|x, y| x + y}`, are instances of this class. -export class SquiggleLambda extends Lambda { - parameters: LambdaParameter[]; +export class UserDefinedLambda extends Lambda { + public parameters: UserDefinedLambdaParameter[]; location: LocationRange; name?: string; + type = "UserDefinedLambda"; constructor( name: string | undefined, - parameters: LambdaParameter[], + parameters: UserDefinedLambdaParameter[], stack: Stack, body: Expression, location: LocationRange @@ -119,10 +116,6 @@ export class SquiggleLambda extends Lambda { return this.name || ""; } - getParameters() { - return this.parameters; - } - _getParameterNames() { return this.parameters.map((parameter) => parameter.name); } @@ -142,34 +135,35 @@ export class SquiggleLambda extends Lambda { // Stdlib functions (everything in FunctionRegistry) are instances of this class. export class BuiltinLambda extends Lambda { - definitions: FnDefinition[]; + type = "BuiltinLambda"; + _definitions: FnDefinition[]; constructor( public name: string, definitions: FnDefinition[] ) { super((args, context) => this._call(args, context)); - this.definitions = definitions; + this._definitions = definitions; } getName() { return this.name; } - getParameters(): LambdaParameter[] { - return [{ name: "..." }]; - } - toString() { return this.name; } parameterString() { - return "..."; + return this._definitions.map(fnDefinitionToString).join(" | "); + } + + definitions(): FRType[][] { + return this._definitions.map((d) => d.inputs); } _call(args: Value[], context: ReducerContext): Value { - const definitions = this.definitions; + const definitions = this._definitions; const showNameMatchDefinitions = () => { const defsString = definitions .map(fnDefinitionToString) @@ -188,6 +182,6 @@ export class BuiltinLambda extends Lambda { } paramCounts() { - return sort(uniq(this.definitions.map((d) => d.inputs.length))); + return sort(uniq(this._definitions.map((d) => d.inputs.length))); } } diff --git a/packages/squiggle-lang/src/value/index.ts b/packages/squiggle-lang/src/value/index.ts index a3822adc49..52a2796c65 100644 --- a/packages/squiggle-lang/src/value/index.ts +++ b/packages/squiggle-lang/src/value/index.ts @@ -6,7 +6,7 @@ import { REDictPropertyNotFound, REOther, } from "../errors/messages.js"; -import { Lambda } from "../reducer/lambda.js"; +import { Lambda, UserDefinedLambda } from "../reducer/lambda.js"; import * as DateTime from "../utility/DateTime.js"; import { ImmutableMap } from "../utility/immutableMap.js"; import { Domain } from "./domain.js"; @@ -130,17 +130,23 @@ class VLambda extends BaseValue implements Indexable { get(key: Value) { if (key.type === "String" && key.value === "parameters") { - const parameters = this.value.getParameters(); - return vArray( - parameters.map((parameter) => { - const fields: [string, Value][] = [["name", vString(parameter.name)]]; - if (parameter.domain) { - fields.push(["domain", parameter.domain]); - } - - return vDict(ImmutableMap(fields)); - }) - ); + if (this.value instanceof UserDefinedLambda) { + return vArray( + this.value.parameters.map((parameter) => { + const fields: [string, Value][] = [ + ["name", vString(parameter.name)], + ]; + if (parameter.domain) { + fields.push(["domain", parameter.domain]); + } + return vDict(ImmutableMap(fields)); + }) + ); + } else { + throw new REOther( + "VLambda: Can't access parameters on a builtin lambda" + ); + } } throw new REOther("No such field"); }