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, nodes): add fields for CLIPEmbedModel and FluxVAEModel #6794

Merged
merged 5 commits into from
Aug 29, 2024

Conversation

maryhipp
Copy link
Collaborator

@maryhipp maryhipp commented Aug 28, 2024

Summary

  • update flux model node to take inputs for CLIP Embed and VAE - this removes the brittle way in which they were hard-coded, which will hopefully make it easier for users to upload and use models that were not directly downloaded from our starter model list
  • update default FLUX workflow to have these fields. this will improve validation in the front-end when a user is missing a model; they will not be able to Invoke and hopefully more easily figure out why.
  • I had to re-download my Flux VAE while working on this but am not sure if it was related to my changes. The error was around the config_path so I may have had an out of date install in my DB.

Related Issues / Discussions

QA Instructions

Restart server (make sure that DB has this default workflow loaded) and run workflow. Should be required to make a selection from your installed models for all four models for the Flux model node.

Merge Plan

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Tests added / updated (if applicable)
  • Documentation added / updated (if applicable)

@github-actions github-actions bot added python PRs that change python files invocations PRs that change invocations services PRs that change app services frontend PRs that change frontend files labels Aug 28, 2024
Copy link
Collaborator

@brandonrising brandonrising left a comment

Choose a reason for hiding this comment

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

lgtm, the flux main and vae components we've added recently make me wonder if there's a way to add fields like this without creating entire components when the Model config type they're managing don't change.

As in it'd be nice if we could have re-used the VAEModelFieldInputComponent, but just pass in the isFluxVAEModelConfig filter somehow since they both manage a VAEModelConfig. Def not something to worry about in this PR though.

@maryhipp maryhipp enabled auto-merge (rebase) August 29, 2024 15:43
@maryhipp maryhipp merged commit 3e569c8 into main Aug 29, 2024
14 checks passed
@maryhipp maryhipp deleted the maryhipp/flux-submodel-fields branch August 29, 2024 15:52
@psychedelicious
Copy link
Collaborator

lgtm, the flux main and vae components we've added recently make me wonder if there's a way to add fields like this without creating entire components when the Model config type they're managing don't change.

It's certainly possible to abstract it further but I don't know if that is valuable:

  • We add stateful model fields infrequently
  • There are subtle differences between some of them that would require the abstractions would to be overly flexible
  • These are simple components with a clear pattern
  • There are already some abstractions (via hooks) that hide away the bulk of the complexity; we wouldn't be saving much code

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 services PRs that change app services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants