Add presets #80

sverhoeven · 2024-11-11T13:52:11Z

Refs #61

TODO

Refs #61

So it can be reused elsewhere. Also added method to find a preset in the presets resource

Do not use pruneDefaults as it is not preset aware

…nce a parameter that was untouched in the perm stays the same

Peter9192 · 2025-01-09T13:58:10Z

Move preset.json to src (no hyperlinks needed; from async to sync); presets from urls to strings
Use full config and Config everywhere (store Experiment/Permutation) except where it must be partial (diff view, form, download, share link)
Dedupe ExperimentConfigForm and PermutationConfigForm

sverhoeven · 2025-01-13T09:13:01Z

Reduce types of config

Config
PartialConfig: Everything optional
NamedConfig: PartialConfig with name and description
ExperimentConfigSchema, nested with permutations and preset
Experiment: Extends ExperimentConfigSchema with output
AppState (anonymous type in apps/class-solid/src/lib/encode.ts) : used for share link, has partial configs to reduce size

TODO

Move name + description to Config
Make Experiment = ExperimentConfigSchema + ExperimentOutput (running, output: {reference, permutations})
replace title in NamedAndDescription with name, to make consistent

sverhoeven · 2025-01-13T10:15:36Z

Where to put the defaults in own file or in json schema?

In JSON schema as default key/value pairs, then each preset needs own JSON schema, on upload need to check preset name to get correct schema.
In json file, then form submit and upload need to use both JSON schema and default object of preset

TODO - ts errors - inline TODOs

sverhoeven · 2025-01-13T12:22:44Z

Where to put the defaults in own file or in json schema?

In JSON schema as default key/value pairs, then each preset needs own JSON schema, on upload need to check preset name to get correct schema.

In json file, then form submit and upload need to use both JSON schema and default object of preset

For now store in JSON schema, but for form validation of permutation need defaults from reference, which is tricky.

Modular forms does not like interface definitions generated by json-schema-to-typescript. So rewrite the interface definitions to type.

…onfig to module with same name

…optional

Peter9192

This PR makes a lot of improvements to the overall code quality. It works well and can in principle be merged as is. There are some things that we could extend upon going forward:

Make it possible to change the preset of an existing experiment
Start from scratch is now the same as start from preset with default preset --> improve user journey (also as part of layout upgrade).

I'm not sure what we finally decided on whether the default config should be in a separate json file or not. I think that might be nice.

apps/class-solid/src/lib/store.ts

Peter9192 · 2025-01-20T11:17:04Z

packages/class/README.md

-# Generate default config file
-pnpx @classmodel/class generate --output config.json
+# Fetch default config file
+curl https://classmodel.github.io/class-web/presets/default.v1.0.0.json | jq .reference > config.json


I had expected a corresponding file in apps/class-solid/public. Are these old changes that need to be reverted?

apps/class-solid/README.md

Peter9192 · 2025-01-20T11:51:47Z

apps/class-solid/README.md

+If you add an preset the `src/lib/presets.ts` file needs to be updated.
+
+An experiment from a preset can be opened from a url like `?preset=<preset-name>`.
+For example to load <src/lib/presets/death-valley.json> use `http://localhost:3000/?preset=Death%20Valley`.


That's cool! But now we have a share URL that says /?experiments=... and the preset url that says /?preset=... which defaults to a single experiment. Makes me wonder if we shouldn't provide one consistent way to encode URLs. See #102

Thanks, yep lets look at the different search params later

Peter9192 · 2025-01-20T12:02:37Z

apps/class-solid/src/components/Experiment.tsx

-  const initialExperiment = () => {
+  const defaultPreset = findPresetByName();
+  const initialExperimentConfig = createMemo(() => {
+    const config = defaultPreset.parse({});


The defaultPreset already has a .config stored in it, right? Why do we create it again here?

Peter9192 · 2025-01-20T13:47:17Z

apps/class-solid/src/lib/presets.ts

+  return { config, schema, validate, parse };
+}
+
+export const presets = presetConfigs.map(loadPreset);


For my brains limited RAM it would help if presets was defined immediately after presetConfigs

Peter9192 · 2025-01-20T13:51:48Z

apps/class-solid/src/lib/store.ts

+        name: config.name ?? `My experiment ${experiments.length}`,
+        description: config.description ?? "Standard experiment",


If name and description are set here, we could leave them blank elsewhere.

Peter9192 · 2025-01-20T13:58:16Z

apps/class-solid/src/lib/store.ts

-    reference: {
-      config: upload.reference,
+    config: {
+      preset: upload.preset ?? preset,


upload.preset is a string, but preset is a Preset, right?

Peter9192 · 2025-01-20T14:01:28Z

apps/class-solid/src/lib/store.ts

+  const refConfig = structuredClone(exp.config.reference);
+  const perm = exp.config.permutations[permutationIndex];
+  const permConfig = structuredClone(perm);


The store is getting quite big for my complexity comfort level. Would it make sense to split it into dedicated modules for experiments, permutations, and analysis?

Peter9192 · 2025-01-20T14:05:45Z

packages/class/src/config_utils.ts

+// biome-ignore lint/suspicious/noExplicitAny: recursion is hard to type
+export function mergeConfigurations(first: any, second: any) {


Would it be better if we keep the config flat but add a 'group' parameter to generate the groups in the web form?

Peter9192 · 2025-01-20T14:15:53Z

PS: I've pushed some commits to #103 while reviewing this PR

Co-authored-by: Peter Kalverla <[email protected]>

It was used to show the preset, but has been replaced by text in the form.

sverhoeven and others added 10 commits November 11, 2024 14:51

Add presets

ef4ff07

Refs #61

color preset button same as others

de00f07

Presist preset when sharing links or downloading/uploading

13d8e82

prune config

7745f75

Add preset to schema

f673732

Merge branch '61-persist-app-state' into presets

09cb218

Add version to preset + try to make preset always set

1076119

Merge remote-tracking branch 'origin/main' into presets

914c3da

Merge remote-tracking branch 'origin/presets' into presets

ee1ac87

cli needs Config not ExperimentConfigSchema, use jq to filter

090bca1

sverhoeven changed the base branch from 61-persist-app-state to main January 8, 2025 11:25

sverhoeven and others added 6 commits January 8, 2025 15:12

Moved presets resource to own module

87b94cc

So it can be reused elsewhere. Also added method to find a preset in the presets resource

Make share link preset aware

9c8cc23

When modifying reference config then also update permutation

a4f7706

Do not use pruneDefaults as it is not preset aware

Use preset config as default values

825e2ac

Combined preset and reference as defaults for editing a permutation

f942324

Perm has full config with reference parameters, after swap new refere…

cc791e6

…nce a parameter that was untouched in the perm stays the same

Big refacfor

e225a7a

TODO - ts errors - inline TODOs

sverhoeven added 8 commits January 17, 2025 12:02

Move config utility function to own module in class package

72190f4

preset is single config not a ExperimentConfig

18db020

Make Config compatible with modular forms

514dc90

Modular forms does not like interface definitions generated by json-schema-to-typescript. So rewrite the interface definitions to type.

Fix reading from url and local storage + Move parsing of experiment c…

e0c4fc0

…onfig to module with same name

Adjust to new schema + Move preset name to form header + make output …

b71489f

…optional

Merge remote-tracking branch 'origin/main' into presets

dd67ef0

Sync docs to implementation

19e9984

Fix tests

dcc3e37

Self review

b285614

sverhoeven marked this pull request as ready for review January 17, 2025 13:03

sverhoeven requested a review from Peter9192 January 17, 2025 13:03

get rid of some partial configs

5c6ba95

Peter9192 mentioned this pull request Jan 20, 2025

URL params #102

Open

Simplify initialExperimentConfig

8f4e429

Peter9192 approved these changes Jan 20, 2025

View reviewed changes

Peter9192 mentioned this pull request Jan 20, 2025

Review presets #103

Merged

sverhoeven and others added 5 commits January 21, 2025 11:59

spellcheck

b2cc225

Co-authored-by: Peter Kalverla <[email protected]>

add llm disclaimer

3838927

spellcheck

acd213b

Co-authored-by: Peter Kalverla <[email protected]>

Merge remote-tracking branch 'origin/presets' into presets

cb3a894

cast

10535e1

Co-authored-by: Peter Kalverla <[email protected]>

sverhoeven mentioned this pull request Jan 21, 2025

Flatten configuration #104

Closed

sverhoeven added 3 commits January 21, 2025 13:10

Typo in component name

a2707a8

No need to set name+description already set in add form

361df65

When uploading without preset fallback to name of default preset

db415e8

sverhoeven mentioned this pull request Jan 21, 2025

Split store module into experiment, permutation, analysis store modules. #105

Open

sverhoeven added 5 commits January 21, 2025 14:41

Merge remote-tracking branch 'origin/presets' into presets

ec5d964

Merge remote-tracking branch 'origin/review-presets' into presets

59ab045

Not enough to do shallow copy, need full.

61d76cb

Format

36f1bab

Use generate to make a config

26df268

This was referenced Jan 21, 2025

Change preset of a experiment #106

Open

Deduplicate code of PermutationConfigForm and ExperimentConfigForm #107

Open

Remove star icon

333f79f

It was used to show the preset, but has been replaced by text in the form.

sverhoeven merged commit a868f09 into main Jan 21, 2025
4 checks passed

sverhoeven deleted the presets branch January 21, 2025 14:18

sverhoeven mentioned this pull request Jan 21, 2025

use description field in permutation form #38

Closed

		name: config.name ?? `My experiment ${experiments.length}`,
		description: config.description ?? "Standard experiment",

		// biome-ignore lint/suspicious/noExplicitAny: recursion is hard to type
		export function mergeConfigurations(first: any, second: any) {

Add presets #80

Add presets #80

Uh oh!

Conversation

sverhoeven commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Peter9192 commented Jan 9, 2025 • edited by sverhoeven Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sverhoeven commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sverhoeven commented Jan 13, 2025

Uh oh!

sverhoeven commented Jan 13, 2025

Uh oh!

Peter9192 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Peter9192 commented Jan 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sverhoeven commented Nov 11, 2024 •

edited

Loading

Peter9192 commented Jan 9, 2025 •

edited by sverhoeven

Loading

sverhoeven commented Jan 13, 2025 •

edited

Loading