RFC Discussion: execution mode in sequential processing #43
Replies: 2 comments 2 replies
-
|
update: better explanations, for loop pseudo-solution |
Beta Was this translation helpful? Give feedback.
0 replies
-
|
I'm very excited about the possibility of using different final prompts for each generation in a batch, as well as XY-grids in reasonable and simple ways, natively. I'm unfortunately not smart enough to have any thoughts or opinions about the plan or alternative suggestions in here, but I wanted to say thanks for putting this together @geroldmeisinger |
Beta Was this translation helpful? Give feedback.
2 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
RFC Discussion: execution mode in sequential processing
Author: @geroldmeisinger | Status: 🟡 Under Review
📋 Quick Links
📄 Current Proposal
RFC: Execution mode in sequential processing
Summary
ComfyUI provides data lists which cause nodes to process multiple times. This is very useful for workflows which make use of multiple assets (images, prompts, etc.). However, the "multiple processing of a single node" stalls the workflow on the node and ComfyUI should pass on execution to the next node after each result and provide a way to control execution mode in sequential processing.
Motivation
Simple example
In the following example, the node
String OutputListis marked asOUTPUT_IS_LIST=True(a data list) and tellsKSamplerto fetch and process the items sequentially. Notice how theKSamplerexecutes 4 times:519414734-303115d3-7c28-42e8-bb52-d02e7cc1022b.mp4
I built a whole node pack around this useful but underutilized feature and it is very well received. It allows easy and native multi-prompt workflows, but again and again, users hit roadblocks because it only works well for small workflows but with larger and more advanced workflows the execution remains at one node for a very long time and also potentially builds up memory usage and OOM.
XYZ-GridPlots
We often want to make image grids to compare how different parameters effect generation.
In this example the
KSamplerprocesses ALL combinations before the next nodes (VAE Decode > Save Image) which can take a very long time and you could loose all progress if something happens. Instead, we want to see and save the intermediate results of each image immediately.Iterate models
We often want to iterate over a list of models to compare how different checkpoints or LoRAs effect generation.
In this example the node
Load Checkpointwill load ALL checkpoints upfront before proceeding, which eventually leads to OOM.Bake parameters into workflow
We also want to bake parameters into the workflow JSON from a multi-image generating workflow to see which concrete parameter was used to generate a specific image. Otherwise, every image stores the same (multi-image generating) workflow.
In this example the node
FormattedStringmakes use ofextra_pnginfoin the second (dynprompt) textfield to store the actually used prompt in the workflow. But this doesn't work because the node is processed for all items first and only stores the last entry before eventually proceeding toSaveImage. When you load any workflow from all the images they all only show the last entry.Multi-model workflows
Another usecase to consider are multi-model workflows. Let's say we have a list of prompts and want to process them in a powerful "iterate prompts -> generate image -> generate video -> upscale video" pipeline, but only one step will ever fit into VRAM. Thus we want to do something like "load image model -> generate ALL images -> unload image model -> load video model -> generate ALL videos -> unload video model -> upscale ALL videos". So this actually a counter-example because in this case we want the process the same node multiple times and thus we need some control over how the multi-processing should work in different use-cases.
Detailed design
I'm not a core developer and cannot provide implementation details, only describe how it would appear to the user. Please note that "node expansion" already provides some features and is discussed in "Alternative Solutions" below!
Change 1 - Pass on execution immediately
The first change I propose is to change the execution scheme to pass on execution immediately after each result instead of processing the same node multiple times at once.
I would argue that all the examples above except "Multi-model workflows" are the more common case and "Multi-model workflow" is very special and advanced. The default behaviour should cover the more common and simple cases, because advanced workflows are designed by advanced users anyway, who know what to do. And secondly, then it doesn't require "Change 4 - Passthrough inputs for ALL output nodes".
The next two proposed changes concern how the execution mode can be changed for single nodes and group of nodes. If "pass on execution immediately" is the default execution mode, all the simple examples would work out-of-the-box and only the "multi-model workflows" would require a configuration on 2 single KSamplers to work. If we don't change it, all the simple examples would require a configuration on a GROUP of nodes.
But whatever the default case is, there has to be a way to change execution mode, either for one node or a group of nodes. This is discussed in the next two proposed changes.
Change 2 - Control execution for a single node
The other point to discuss would be - whether or not the default execution scheme is actually changed - HOW we set a single node or group of nodes to the other execution mode. The single node case is trivial, just add a context menu entry and some icon that the node uses a different execution mode.
Change 3 - Control execution for a group of nodes
For a group of nodes the change is more complicated because we have to have a way to define which nodes are covered and a visual to see which nodes are effected. The following discusses different options on how the execution mode can be changed for a group of nodes:
Option a) Subgraphs
Add an option for subgraph "treat as sequential unit".
Option b) Add Group (box)
When the box is set as "sequential unit" all nodes within the box execute differently (the actual UI should be different of course, like a context menu entry and a icon which visualizes the change)
Option c) onExecuted/onTrigger
I don't know what the original intention for this feature was and I don't really understand it but it could be used to the tell
KSampler: "after you have executed, trigger thisSaveImagenode immediately".. which then looks upstream until it finds theKSamplerand fetches the item downstream. It only needs to be actually implemented.Also see:
There is a PR which implemented this and I tried it here: Comfy-Org/ComfyUI#7443 (comment) but it didn't work. Maybe all it takes is that onExecuted fires on every item.
And would this work with the example "Iterate models"? e.g. "load 'checkpoint 1' onExecuted -> trigger unload 'checkpoint 1', then proceed with 'checkpoint 2'". Or would the caching strategy already cover the unloading of checkpoint 1 after usage?
Option d) Group nodes
(Just for completeness sake, not seriously considered)
Change 4 - Passthrough inputs for ALL output nodes
The following change is only required, if the execution scheme stays as it is. It is almost possible to display the intermediate results with for-loops already, except that output nodes would need to passthrough the inputs as an output knob, in order for the for-loop to consider the node as a part of the node expansion (see "Alternatives - node expansion at runtime").
Drawbacks
I don't know. From an execution point of view the processing would jump around in the graph back and forth multiple times:
a) single diffusion workflow: executes the same as now
b) multiple inputs (from a data list) into a single diffusion workflow: fetch one item, execute the whole graph, then jump back to the input list, fetch the next item, repeat. the output node (Save Image) is populated one-by-one.
c) multiple inputs with a node that has
INPUT_IS_LIST=true: same asbexcept all results are collected in this node before proceeding downstreamd) multiple inputs with a
KSamplerthat is set toexecute multiple times(see "multi-model workflow"): same as c except all the results are collected in the KSampler before proceeding downstreamSo maybe this could effect:
On the other hand, it already works with node expansion, so everything should work out-of-the-box already?
Yes, see "ad-hoc node expansion" but this solution is either complicated or ambigious, and has some drawbacks.
I consider the impact of teaching and migrating small, because it only applies to advanced workflows and the change is simply "from now on, nodes pass on execution immediately".
onExecuted/onTrigger could be effected by this
has been discussed
Alternatives
a) custom node expansion
The only solution so far I've found is to copy the node pattern, e.g.
KSampler > VAE Decode > Save Image, as a custom node with node expansion in code:While this works with the default pattern in the simple example, it would require to reimplement each deviation as a new custom node in code.
etc.
And you have to implement this in code and restart the server for every change.
b) ad-hoc node expansion in editor
Because node expansion already works we could provide a feature "create node-expansion from selected nodes" which simply generates the node expansion code and provides it as a custom node (or blueprint or something). This could be implemented as a plugin or new feature.
c) node expansion at runtime
Another solution to show the intermediate results that ALMOST works is "node expansion at runtime" a.k.a for-loops.
I don't know how exactly for-loops work in ComfyUI, but it probably defines a start and end point, then searches for a path through the links, selects all the nodes involved and instantiates a copy of them via node expansion for every list item. The problem why this doesn't work is because every output node would need to passthrough their input in order to be considered a part of the node expansion, hence why their is a custom
Save Image Passthroughnode in this example. This could probably be fixed by adding a "and also include this output node"-knob to the for-loop but then the user's brain would explode. No one (NO ONE!) knows how for-loops actually work or how they are used, including the custom node developers themselves. So this not really a solution either.Related discussion:
d) The PrimitiveInt control_after_generate=increment pattern
A well known pattern to emulate multi-processing is the
Primitive Intnode withcontrol_after_generate=increment. You basically get a counter that increases everytime you run a prompt. When you hook it up as a index in a list selector node, it iterates over entries across multiple prompts. In the Run toolbox you can set the amount of prompts to the number of items in your list to iterate the whole list. This pattern essentially cancels out the effect of output lists and will only ever process one item at a time.For any serious multi-image workflows the current sequential processing always runs into road blocks. Either we have to wait a long time to see intermediate results, where the progress could be lost, or it runs into OOM.
Adoption strategy
I don't see data lists wildly used right now although they should be as they are very powerful. As such I think breaking changes are minimal and only effect multi-image-multi-model workflows with model unloading (see examples above), which are very unlikely to exist.
Unresolved questions
onExecuted/onTrigger
I don't know how they work, maybe this is the solution already, but there is no documentation and they are not implemented yet. Maybe all it takes is that onExecuted fires for every item.
💬 Discussion Guidelines: Share feedback, concerns, and suggestions below. Use reply threads to keep conversations organized.
Beta Was this translation helpful? Give feedback.
All reactions