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.

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

feat(ui): more batch data types #7545

Merged
merged 39 commits into from
Jan 17, 2025
Merged

Conversation

psychedelicious
Copy link
Collaborator

@psychedelicious psychedelicious commented Jan 10, 2025

Summary

  • String batch node
  • String generator SPLIT ON CHAR
  • Integer batch node
  • Integer range generator START-STEP-COUNT
  • Stretch goal: Integer range generator MIN-MAX (RANDOM)
  • Stretch goal: Integer range generator START-END-COUNT
  • Stretch goal: Integer range generator PARSE STRING
  • Float batch node
  • Float range generator START-STEP-COUNT
  • Stretch goal: Float range generator MIN-MAX (RANDOM)
  • Stretch goal: Float range generator START-END-COUNT
  • Stretch goal: Float range generator PARSE STRING
  • Image collection supports duplicate images

A followup PR will add support for pairwise batching.

Supporting internal changes:

  • Reorganize & abstract field validation logic

QA Instructions

Try it out!

Generator UI:

Screen.Recording.2025-01-13.at.9.06.07.pm.mov

Merge Plan

Targeting v5.6.1.

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

@github-actions github-actions bot added python PRs that change python files invocations PRs that change invocations frontend PRs that change frontend files services PRs that change app services python-tests PRs that change python tests labels Jan 10, 2025
@psychedelicious
Copy link
Collaborator Author

My first go at the generators made them an alternative value for collection fields that was embedded in the workflow. So in the workflow, a float collection would either be a list of floats or the generator config (e.g. start, step and count). When you load a saved workflow, if the collection was set up as a generator, you'd get the generator UI instead of a list of numbers.

This worked but it modified the workflow schema, which is undesirable as it increases maintenance burden, complexity and implies some amount of lock-in.

I reverted those changes and made the generators a purely UI thing. The frontend logic is much simpler and the UX is similar. The only downside is that the generator isn't saved to the workflow, which was kinda nice.

Overall this simpler approach feels right to me. It also makes it much easier to add more complex generators - like non-linear ranges. It also opens the door to using dynamic prompts as a generator, which is pretty cool.

@psychedelicious psychedelicious enabled auto-merge (rebase) January 17, 2025 01:02
Unfortunately we cannot do strict floats or ints.

The batch data models don't specify the value types, it instead relies on pydantic parsing. JSON doesn't differentiate between float and int, so a float `1.0` gets parsed as `1` in python.

As a result, we _must_ accept mixed floats and ints for BatchDatum.items.

Tests and validation updated to handle this.

Maybe we should update the BatchDatum model to have a `type` field? Then we could parse as float or int, depending on the inputs...
@psychedelicious psychedelicious force-pushed the psyche/feat/ui/more-batch-types branch from b6aae16 to 8a29a11 Compare January 17, 2025 01:02
@psychedelicious psychedelicious merged commit 9265716 into main Jan 17, 2025
15 checks passed
@psychedelicious psychedelicious deleted the psyche/feat/ui/more-batch-types branch January 17, 2025 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend PRs that change frontend files invocations PRs that change invocations python PRs that change python files python-tests PRs that change python tests services PRs that change app services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants