-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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): graph builder #6361
feat(ui): graph builder #6361
Conversation
c4069d8
to
1817914
Compare
9006df4
to
cf900d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unable to test for a day or two but imagine id not catch the edge cases anyways. Best to put out an RC for it?
Let's get it into |
This dynamically generated schema object maps node types to their pydantic schemas. This makes it much simpler to infer node types in the UI.
This stateful class provides abstractions for building a graph. It exposes graph methods like adding and removing nodes and edges. The methods are documented, tested, and strongly typed.
Provides methods for manipulating a graph's metadata.
Simpler types and API surface.
…RACollectionLoader These simplify loading multiple LoRAs. Instead of requiring chained lora loader nodes, configure each LoRA (model & weight) with a selector, collect them, then send the collection to the collection loader to apply all of the LoRAs to the UNet/CLIP models. The collection loaders accept a single lora or collection of loras.
No good reason to have it be separate. A bit cleaner this way.
ad6a816
to
15d118d
Compare
I've done a third read-through of this PR, tidied a couple things. Merging this so we can get some testing via main and fail forward if needed. |
Summary
This PR adds internal graph-building utilities to the UI and uses them for the generation tab graph builders.
Python changes
InvocationOutputMap
object is added to the OpenAPI schema. This maps invocation types to their output classes.Current chaining method:
New collect method:
LoRASelectorInvocation
: Select a LoRA model and provide its weight.LoRACollectionLoader
/SDXLLoRACollectionLoader
: Given a collection of LoRAs, the UNet model and the CLIP model(s), adds all the LoRAs to the models.Frontend Changes
A stateful
Graph
class abstracts adding nodes and edges to graphs. Its methods have pretty solid type safety and hinting. It's also far more compact and readable. It is fully tested viavitest
coverage - which was added in this PR.trimmed.graphb.uilder.mov
Some handy new types (finally) allow for inspection of a node's output fields:
Screen.Recording.2024-05-13.at.10.15.38.pm.mov
A
MetadataUtil
class manages metadata for a graph using the new graph methods. Its API is basically the same as the old metadata helpers.The Generation tab graphs have been rewritten using the new utilities. Canvas graphs are untouched.
The new utilities require the passing of the nodes themselves to graph building helpers - as opposed to node IDs. This simplifies the graph building logic and allows for type hints when connecting edges and such. For example, compare the new seamless helper with the old one. It's half the size and is type safe!
Because the
Graph
helper provides a generic abstraction for building graphs, its API could have a separate implementation that builds workflows. This is one step closer to the intuitive internal API where we send workflows instead of graphs to the backend.TODO
Revise docstrings for the
Graph
class - I changed the API at some point and haven't fixed the docstringsMore thorough testing. As far as I can tell, the generation tab is fully working - all features work in all combinations I have tested. But it's very possible I've missed some combination.
Explore adding larger-scale tests for the actual graph outputs.vitest
does support a snapshot mode, there's probably some way to grab different redux states and test the full graph building logic. TBH I'm not sure how useful this would be, because when we update the graph, we'll need to redo the test snapshot anyways...I don't think there is much point in testing the graph builders end-to-end. Any change to the graph requires the expected graph be recreated in the tests. Even for this PR, where I've rewritten the graph builders entirely, there have differences that would cause tests to fail. For example, I changed how LoRAs are connected and how
is_intermediate
is applied. We'd need a dozen test cases and most of them would have to be carefully recreated. I think our time is better spent testing in the app.Testing improvements
vitest
has a coverage plugin and UI plugin, which are both set up. To use the fancy UI, runpnpm test:ui
. It's pretty nifty:Screen.Recording.2024-05-14.at.8.44.16.pm.mov
Related Issues / Discussions
n/a
QA Instructions
Once out of draft - test all combinations of settings and features on the generation tab. I mostly expect loud failures for things that aren't correct.
Merge Plan
n/a
Checklist