Group Nodes

[clarkmcc]

Goal is to replicate this behavior from Blender https://github.com/clarkmcc/ngraph/assets/6639685/d51c1da6-8aaa-4955-ab68-8ef7f70495e7

Here's how group nodes should work:

• I should be able to select any number of nodes in my graph and group them.
• If any of the nodes in the group are connected to nodes outside the group, group inputs and outputs should automatically be created, and edges should be redirected through the node group.
• This will take me to the group node graph where the rest of the unselected nodes will disappear, and I should only see the nodes in my group.
• I should also see two additional nodes, an inputs node, and an outputs node. The inputs node has one output handle, and the outputs node has one input handle.
• I can connect the input node's output handle to the input handle of one of the nodes in my graph. This connection denotes that the group as a whole should have an input of the same type and configuration as the input handle of the node it was connected to. I should be able to do the same thing for the node group's output.
• I should be able to delete the node group, and any connections between nodes outside the group, through the group inputs and outputs, and into the group's nodes should be re-connected once the group is disbanded.
• I should be able to copy a group and paste it to create a new identical group

[sroussey]

I have a somewhat different use case where groups would be useful.

I have a task system that is represented with a DAG. Some of the nodes have sub tasks, so it would be great to optionally show them in the UI. In this particular case, the parent/group task sets the inputs and outputs of the sub-tasks, and are not editable. It is just there so what is going on. But it would be nice from a UI perspective to use the same visuals since the subtasks could be tasks that the user works with directly as well.

[clarkmcc]

Thanks for the feedback. Would sub flows be a better solution for you? They're not collapsable, but they should be supported out of the box today, and they don't have the issue of whether inputs and outputs should be editable.

[sroussey]

Hmmm... maybe. The ones inside I don't want movable or editable (neither the node or the edges). I'll play with it and see.

[sroussey]

I've tried but I get some weird behaviors. I'm afraid that I might be doing activating something of your own group nodes config code.

[sroussey]

Here is where I want to get to:

Any ideas would be appreciated. Thanks!

[clarkmcc]

Okay, so to make sure I'm understanding correctly, you're looking for the group nodes and the child nodes to all be visible (maybe child nodes are collapsable it looks like in your diagram), but you're looking for group inputs and outputs to be owned by the group node, is that correct? In this case, ReactFlow's subflows won't work (at least out of the box).

So here's actually how I'd first experiment to accomplish what you're doing. I'd make the group node a custom node, and in the body of the component, I would actually create a nested NodeGraphEditor component. The nodes and edges for this from come from group node's "node data". Now I'm not quite sure how well this would work or how well it would scale to lots of groups, but that would be my first stab at this problem. I think this approach would be much easier and cleaner than the Blender-style node groups that I'm trying to implement in #11.

I'm not sure how this approach would handle connecting group inputs, to child node inputs though. Can you clarify in your diagram how group you envision connecting the Serial Group Name > Input to the SubNodes inputs?

[sroussey]

Yeah, I just have those there to signify that the outer node owns the inputs and outputs, not to have them editable or anything. In fact, the entire sub-graph will not be editable.

I don't think I'll have really large graph-subgraphs (I may laugh at myself later), so I think the recursive idea is probably the best way to get it started. I can always optimize later.

[sroussey]

Oh, I should add I suppose, that the inside nodes might change depending on the what the inputs are. They are dynamically created and thus owned by the parent.

[sroussey]

And since I am doing a pipeline of tasks (think GitHub Actions), the only real reason I want them visible is so a user can see what is happening when the task pipeline runs.

[sroussey]

I've tried both methods today, and I feel like sub flows will be easier to manage. For both I disable a lot of things to make the sub tasks "read only".

I need to figure out the parent size... how to set it to be able to fit the children.

[sroussey]

So I have re-thought what I might need, and I think the blender model will work. So... ignore everything I said above. :)

On this item:

If any of the nodes in the group are connected to nodes outside the group, group inputs and outputs should automatically be created, and edges should be redirected through the node group.

I have a couple of thoughts.

1. What if you are building a group in isolation? There won't be any existing input and output edges yet.
2. You could take all the unconnected inputs and make them inputs of the group, and all the unconnected outputs and make them outputs of the group.
3. When you talk about adding an input and output node for the group, they can be automatically setup as above. But the user should be able to remove some.
4. This implies some inputs and outputs are be optional. You Input Groups example, if a color is not connected, it can be set in the node itself. So that one is optional. But another might take a whole image as an input, and that is not optional or settable in node that wants it as an input.

Anyhow, thanks for listening to my other ideas above. I will just have different node layouts for when a task run is going vs when editing. I was thinking too hard on what to get one view to work for different purposes, and that just makes it complicated.

Thanks again!

[clarkmcc]

You could take all the unconnected inputs and make them inputs of the group, and all the unconnected outputs and make them outputs of the group.

I think the problem with this approach is that it doesn't scale well with lots of child nodes, or child nodes with lots of inputs and outputs. Take a large node like this for example. What happens if you create a group with just this inside? I don't think duplicating all the inputs and outputs from the child node to the group nodes is the ideal approach. What if you group several of these large nodes? Do you create an input for every unconnected input, for every node?

[sroussey]

Only if required.

Your example has directly settable parameters with default values.

I would not auto connect those inputs. I would let the user chose to if they wanted. But if one of the inputs required something (like an image to apply effects on), that image would auto connect (as the node would be in a broken state without it).

In fact, a group with one element inside is a great way to hide all the settings. You can configure it with various values and “group” that to hide too many settings.

[sroussey]

Ah, you don't have the idea of required. The closest you have is when inputType == null

[clarkmcc]

Yeah, everything has a default. I'm not sure the use-case for a null inputType because if it's null, then there would be any validation about what can be connected to it and you can't provide any manual values. Even custom input types like the color picker used in the storybook allow you to set default values (they show their custom input components rather than just a connected handle.

[sroussey]

There is validation though, right? That is what valueType is from what I understand, so a color will only connect to a color.

[clarkmcc]

Oh yes, yes you are right, I got them mixed up. You always have to have a valueType, that value type can have a null input type like shown on the Value input in the screenshot, but this value type cannot be set anywhere now, even as the output of another node it is only a relationship expressed in the graph, but no explicit values can be passed around.

So back to the question of group inputs, I don't see inputType == null as a common use-case -- just for representing opaque value relationships. Creating inputs automatically for every one of these kinds of inputs I suppose is fine, but we'd be making an assumption about what the user wants connected in the group, and what the user wants to keep null. Not to mention, what if there are two duplicate nodes in the group, how do you disambiguate between the same node inputs and which of them should automatically be connected to the group input node? The following screen recording illustrates this idea of duplicate node inputs.

https://github.com/clarkmcc/ngraph/assets/6639685/16f9ead2-3130-4361-94a7-bd79aa35a4f7

[sroussey]

I still think Input should have something called required where if there is no edge, then the node is non-functional. The auto-connect is a nice option if your nodes are all the kinds that have required inputs, but the UI could work a different way, say by not allowing the group to save if the required inputs of the nodes are not connected to the group input.

[clarkmcc]

then the node is non-functional

What are you envisioning this would look/behave like?

not allowing the group to save if the required inputs of the nodes are not connected

When you say "save" are you referring to persisting the changes to the nodes and edges to state management?

[sroussey]

I mean, not letting you exit the grouping UI you have in the video until errors are solved.

Your use case doesn't have these issues, which is nice. ;)

Say, you have some image editing nodes. You want to group three nodes that you use together to add a watermark:

• InlineImage (this lets you upload an image and it is saved as part of the state of the node, outputs image)
• Blur (inputs: image [required], mask [also type image, but not required], number [blur amount], output is image)
• Text overlay (input: image, text, position, output: image -- this adds some text on top of an image)

User wants to group the three and call it AddBranding. The idea is that it takes an image as input, blurs a location in the image, and adds text on top of that blurred area. So the InlineImage gets an image upload that defines the location to mask for the blur. It gets connected to the ImageBlur mask input. The image blur output goes to the TextOverlay input image. The user types in the name of the brand and location (same as mask location).

Grouping those, the group input is an image and the output is an image (that has the text on top of the image where it is blurred to make it easy to read).

The ImageBlur node can't function without an input image. There is no default value for an image, and trying to blur undefined is not going to work.

If the user does not connect that Imageblur image input to the group input, nothing will work. I wouldn't want them to save that group until the error was cleared (by connected the imageblur image input to something)

[clarkmcc]

Okay, I think I understand what you're suggesting.

This approach assumes that you fully flesh out your group node graphs all in one sitting, and I don't know that I agree with that approach. I'm not sure if you've used ComfyUI's node graph editor before but on the AI image generation side, I may want to go back and forth between the node group and the rest of the graph as I build my graph. I think the restriction that the graph must always be in a "valid" state during construction could be a bad UX for certain use-cases. I'm also not sure how this "validation" requirement would be applied to the root graph, setting aside node groups entirely.

It reminds me of Go's unused variable requirement (I'm a big Go guy). You're building out some stuff, you comment out a few lines for testing, now your program won't compile because there's unused variables, so then you have to do work to get around the system just because you're running some tests, or are iteratively working towards the end goal.

Edit: it sounds like there may just be a different set of requirements based on our different use-cases. Enforcing validity might be problematic for something like Blender or AI image generation graphs, but might be a must for your task pipeline project. Maybe we need to look at supporting both.

[sroussey]

Typescript linters have that option too! I don't use them.

Valid points on validation. Maybe just giving feedback that it is not valid is enough.

I am purposefully not looking at other versions of this idea (there are many), so I can make something unique. It may turn out to be false, and I just redo what others do, but we will see!

[sroussey]

With groups... there are two ways of looking at it. One is that it is all one big graph, and you hide nodes inside the group and show them when you edit. This requires some transforms in the UI space. The other way is that you have a "compound" node that has its own sub-graph, and the outside graph doesn't care. This does require some transformation when you want to process the full graph, though not too hard.

Personally, I am going with the second option. This makes it easy to alter the sub-graph without affecting much else. And fewer nodes to process in the UI.

[sroussey]

BTW: my "sub-task" node group which is the same thing, i have kind working, but i end up not liking the UI.

Colors are crazy.

[clarkmcc]

Option 2 is definitely simpler to implement, though I have questions about it's usability. For example, how does the UI indicate that an input from the group node is piped through to the child node? If the group node just exposes all unconnected inputs from the child nodes, then I think that addresses that problem, but I don't know that that approach is scalable for the Blender-use case.

[sroussey]

Yeah, I am not thrilled with the UI. Also, notice that there is a node around the node in the Subtasks area. There is a handle on the left and a handle on the right. At the moment there is only one handle and they are connected. My ideal, is to create an input and output node instead, and connect them.

This is ugly but should get across what is actually going on:

[sroussey]

Actually, it is more like this (which is what you are thinking:

But I also need an array splitter:

[sroussey]

For the first one, I could instead use whatever you are building. I'll just be pre-configuring them for users.

The second one though, is what I need to work on: the array splitter. Has a single node (which may be a compound/group node) and duplicates based on how many items are in the array. Not sure if that is something you are thinking about, but from a UI perspective it is also a "group".

[clarkmcc]

Yeah I think we're on the same page then on the goal for node groups. I actually have a POC where the node inputs and outputs are being created, the next tricky part was dynamically generating the inputs -- so that's kind of the next big blocker on node groups.

And that is some what related to your second point about array splitters. To make sure I understand, the node would accept an array as input, then allow for any number of output handles to be connected, each taking an index of the array? Is that correct?

If so, I think at a high-level it would just be a matter of generating a new output handle if all current output handles are connected. We would need some APIs built into the library that allow for dynamically changing the inputs and outputs (the node's config). This is where I think there's some crossover between the node group project and this one -- dynamic handles.

[sroussey]

Yeah I think we're on the same page then on the goal for node groups. I actually have a POC where the node inputs and outputs are being created, the next tricky part was dynamically generating the inputs -- so that's kind of the next big blocker on node groups.

You mean your POC input and output nodes are being created? Hard to know which input is which. :p

I saw the "Inheriting Output Field" story, but it seemed to only work for the String and not the Number so far.

And that is some what related to your second point about array splitters. To make sure I understand, the node would accept an array as input, then allow for any number of output handles to be connected, each taking an index of the array? Is that correct?

There are actually two different ways, but useful, of doing something like this.

If so, I think at a high-level it would just be a matter of generating a new output handle if all current output handles are connected. We would need some APIs built into the library that allow for dynamically changing the inputs and outputs (the node's config). This is where I think there's some crossover between the node group project and this one -- dynamic handles.

Ah yes. For this use case I can think of an example:

An image generation node with a prompt text and a single model. The use could connect multiple things to it and it can create an additional variation. You could have a plus sign button to add handles to signify how many you want.

There is another node type: the ArrayForEach node. Takes an array for input and an array for output, and a node that it will duplicate for each item in the array (and that node could be a group node). Sorta illustrated in the last picture above.

Now there are two ways to look at the UI for this: just show the node/s that will be duplicated, or based on the data, show the actual duplicated nodes. So, if I selected three models for text summarization, it would show three copies of the text summarization and the input and output nodes as the "group".

If I create two very different node views: one as the editor (ngraph) and another as the runner (turbo?), then the editor should be simpler. But there is something nice about immediate reactive results. Though immediate is not exactly true these days with gen-ai, as the stuff is expensive and/or slow.