Skip to content

Commit

Permalink
Better split UserDefinedLambda and BuiltInLambda
Browse files Browse the repository at this point in the history
  • Loading branch information
OAGr committed Oct 11, 2023
1 parent ecf2641 commit 2df4708
Show file tree
Hide file tree
Showing 14 changed files with 111 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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[] {
Expand Down Expand Up @@ -65,7 +60,7 @@ export function initialCalculatorState(
fieldNames: calculator.fields.map((row) => row.name),
fields,
fn: {},
hash: calculatorHash(calculator),
hash: calculator.hashString,
};
}

Expand Down
3 changes: 1 addition & 2 deletions packages/components/src/components/Calculator/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { Action, useViewerContext } from "../SquiggleViewer/ViewerProvider.js";
import {
CalculatorAction,
CalculatorState,
calculatorHash,
calculatorReducer,
hasSameCalculator,
initialCalculatorState,
Expand Down Expand Up @@ -106,7 +105,7 @@ export const Calculator: FC<Props> = ({
useEffect(() => {
const calculatorChanged =
prevCalculator !== null &&
calculatorHash(calculator) !== calculatorHash(prevCalculator);
calculator.hashString !== prevCalculator.hashString;

if (calculatorChanged) {
calculatorDispatch({
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/components/FunctionChart/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ export const FunctionChart: FC<FunctionChartProps> = ({
</MessageAlert>
);
}
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ export const getBoxProps = (
<div>
fn(
<span className="opacity-60">
{truncateStr(value.value.parameterNames().join(", "), 15)}
{truncateStr(value.value.parameterString(), 15)}
</span>
)
</div>
Expand Down
4 changes: 2 additions & 2 deletions packages/squiggle-lang/__tests__/library/plot_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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.)"
);
});

Expand Down
2 changes: 1 addition & 1 deletion packages/squiggle-lang/src/fr/mixture.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down
10 changes: 8 additions & 2 deletions packages/squiggle-lang/src/fr/plot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/squiggle-lang/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 1 addition & 29 deletions packages/squiggle-lang/src/library/registry/core.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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`);
Expand Down
9 changes: 9 additions & 0 deletions packages/squiggle-lang/src/public/SqValue/SqCalculator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
56 changes: 43 additions & 13 deletions packages/squiggle-lang/src/public/SqValue/SqLambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<SqValue, SqError> {
Expand Down Expand Up @@ -68,4 +94,8 @@ export class SqLambda {
toString() {
return this._value.toString();
}

parameterString() {
return this._value.parameterString();
}
}
8 changes: 4 additions & 4 deletions packages/squiggle-lang/src/reducer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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]) = { ... }
Expand Down Expand Up @@ -238,7 +238,7 @@ const evaluateLambda: SubReducerFn<"Lambda"> = (
});
}
const value = vLambda(
new SquiggleLambda(
new UserDefinedLambda(
expressionValue.name,
parameters,
context.stack,
Expand Down
Loading

0 comments on commit 2df4708

Please sign in to comment.