Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds Calculator.make(fn) call #2660

Merged
merged 4 commits into from
Dec 6, 2023
Merged

Adds Calculator.make(fn) call #2660

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
881a0eb
Add Calculator.make fn
OAGr Dec 5, 2023
a0455d8
Added documentation and changeset
OAGr Dec 5, 2023
44ba29b
Merged with main
OAGr Dec 6, 2023
0963778
Removed sampleCount hardcode from toCalculator()
OAGr Dec 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/eleven-snakes-deliver.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@quri/squiggle-lang": patch
---

Added Calculator.make(fn) shorthand
26 changes: 15 additions & 11 deletions packages/squiggle-lang/src/fr/calculator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,23 @@ import {
frNumber,
} from "../library/registry/frTypes.js";
import { FnFactory } from "../library/registry/helpers.js";
import { vCalculator } from "../value/index.js";
import { Calculator, Value, vCalculator } from "../value/index.js";

const maker = new FnFactory({
nameSpace: "Calculator",
requiresNamespace: true,
});

const processCalc = (calc: Calculator): Value => {
const _calc = vCalculator(calc);
const error = _calc.getError();
if (error) {
throw error;
} else {
return _calc;
}
};

export const library = [
maker.make({
name: "make",
Expand All @@ -34,23 +44,17 @@ export const library = [
["sampleCount", frOptional(frNumber)]
),
],
([{ fn, title, description, inputs, autorun, sampleCount }]) => {
const calc = vCalculator({
([{ fn, title, description, inputs, autorun, sampleCount }]) =>
processCalc({
fn,
title: title || undefined,
description: description || undefined,
inputs: inputs || [],
autorun: autorun === null ? true : autorun,
sampleCount: sampleCount || undefined,
});
const error = calc.getError();
if (error) {
throw error;
} else {
return calc;
}
}
})
),
makeDefinition([frLambda], ([fn]) => processCalc(fn.toCalculator())),
],
}),
];
30 changes: 29 additions & 1 deletion packages/squiggle-lang/src/reducer/lambda.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { ASTNode } from "../ast/parse.js";
import * as IError from "../errors/IError.js";
import { REArityError, REOther } from "../errors/messages.js";
import { Expression } from "../expression/index.js";
import { VDomain, Value } from "../value/index.js";
import { Calculator, VDomain, Value } from "../value/index.js";
import * as Context from "./context.js";
import { ReducerContext } from "./context.js";
import { Stack } from "./stack.js";
Expand All @@ -16,6 +16,7 @@ import {
import uniq from "lodash/uniq.js";
import { sort } from "../utility/E_A_Floats.js";
import { FRType } from "../library/registry/frTypes.js";
import maxBy from "lodash/maxBy.js";

export type UserDefinedLambdaParameter = {
name: string;
Expand All @@ -33,6 +34,7 @@ export abstract class BaseLambda {
abstract parameterString(): string;
abstract parameterCounts(): number[];
abstract parameterCountString(): string;
abstract toCalculator(): Calculator;

callFrom(
args: Value[],
Expand Down Expand Up @@ -136,6 +138,18 @@ export class UserDefinedLambda extends BaseLambda {
parameterCountString() {
return this.parameters.length.toString();
}

toCalculator(): Calculator {
const only0Params = this.parameters.length === 0;
return {
fn: this,
inputs: this._getParameterNames().map((name) => ({
name: name,
type: "text",
})),
autorun: only0Params,
};
}
}

// Stdlib functions (everything in FunctionRegistry) are instances of this class.
Expand Down Expand Up @@ -193,6 +207,20 @@ export class BuiltinLambda extends BaseLambda {
}
throw new REOther(showNameMatchDefinitions());
}

toCalculator(): Calculator {
const longestSignature = maxBy(this.signatures(), (s) => s.length) || [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a reasonable heuristic, but it could cause some frustration if the longest signature is not the most natural one.

Brainstorming other, more general approaches:

  1. Calculator.make(fn) -> Calculator[] (return all possible versions)
  2. Calculator.make(fn.definitions[i]) -> Calculator, where fn.definitions[i] returns a specialized function with a single signature (but this is awkward because it means that fn.definitions[3][0][0][0][0][0] should be allowed)
  3. Calculator.make(fn, 3) -> Calculator (same as Calculator.make(fn)[3] in (1) above)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the ideas, but I don't think I'm a fan of those.

  1. This could be messy, especially if there are many definitions (you'll get 3-10 on some items). I'd expect it to surprise users.
  2. Agreed it's awkward. Slow to type out.
  3. Also seems weird, especially as the order of definitions might be a bit chaotic.

One nice thing is that this issue should only come into play for builtin functions - users functions only have one definition.

My guess is that we should leave for now. However, here are some options for later:

  1. Maybe, if there are multiple definitions, then if you try Calculator.make() without any definitions, it shows you an error, but gives you a list of definitions as strings. You then can plug them in as, Calculator.make(defString).

  2. We output a Dict, where keys have to do with the definitions somehow.

  3. We output a small dropdown menu of definitions, after which the correct form will appear.

const autorun = longestSignature.length !== 0;
return {
fn: this,
inputs: longestSignature.map((sig, i) => ({
name: `Input ${i + 1}`,
type: sig.getName() === "Bool" ? "checkbox" : "text",
})),
sampleCount: 10000,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, why? Shouldn't this be inherited from environment, like in user-defined lambdas?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted this to be a bit higher than normal, because these are probably fairly small (just one function).

That said, as I think about it, maybe it could be a very expensive function. Can change.

autorun,
};
}
}

export type Lambda = UserDefinedLambda | BuiltinLambda;
1 change: 1 addition & 0 deletions packages/website/src/pages/docs/Api/Calculator.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Calculator.make: ({
autorun?: boolean = true,
sampleCount?: number,
}) => calculator
Calculator.make(fn: ...arguments => any) => calculator
```

``Calculator.make`` takes in a function, a description, and a list of fields. The function should take in the same number of arguments as the number of fields, and the arguments should be of the same type as the default value of the field.
Expand Down