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

Is there a way to seperate the data flow and the execution flow? #26

Open
liusida opened this issue Jul 8, 2024 · 15 comments
Open

Is there a way to seperate the data flow and the execution flow? #26

liusida opened this issue Jul 8, 2024 · 15 comments

Comments

@liusida
Copy link

liusida commented Jul 8, 2024

Hi @daniel-lewis-ab @atlasan , I am thinking about making a ComfyUI alternative. I know it sounds quite ambitious but I am actually planning it.

Here is the plan, and I compared it with ComfyUI:
https://github.com/liusida/StoneSoup

One key aspect of the new project StoneSoup is to seperate data flow from execution flow, so it'll be natural to support loops and other flowchart features. Do you happen to know how to hack LiteGraph to support that? Maybe a dedicated type of connections.

Or, any documentation/tutorials/demos of LiteGraph would help, because I can't find many docs that explain LiteGraph. And it feels like there are many functionalities hidden in that 10k+ lines of code.

Thanks, guys~

@atlasan
Copy link
Collaborator

atlasan commented Jul 9, 2024

image
image

Hi liusida, actually LG has native event type, alongside whole-tree execution, that would cause matters on how and when one would update the nodes. I had introduced two modes dedicated at flow-blased execution. That has probably never been used and tested outside of me, lucky or unluckily. Apart from that Javi too proposed a recent change in the code integrating deferred execution of events to frame values update for the events reading values.
I would recommend to take serious consideration on everything involved in graph execution as it does not seems an easy task.
My best, and all the best for StoneUp
*

@liusida
Copy link
Author

liusida commented Jul 9, 2024

Thanks @atlasan . I am looking at onTrigger mode. I wish that two events from different nodes can trigger the same target node. Now it can't, right?

image

And also, when using this onTrigger mode when the graph is stopped, I need to runStep once and then click the button to trigger the node. Is this expected?

@liusida
Copy link
Author

liusida commented Jul 9, 2024

I wish one node can be triggered by multiple nodes because if i want to support the basic flow chart like this:

image

@liusida
Copy link
Author

liusida commented Jul 10, 2024

Haha! After modifying litegraph.js a bit, I have this (multiple event links targeting the same node):

image

But it seems not easy to do this without changing the source code (need to copy many lines out in order to override the default behavior).

//BTW, when I tested your on Trigger mode, if I change a node to the OnTrigger mode, two additional slots will appear, but when I change it back to ALWAYS, these two slots don't disappear. Seems like a bug. ^_^ Do you have any plan to redo this part? I'll be quite willing to refine the Trigger mode.

@atlasan
Copy link
Collaborator

atlasan commented Jul 10, 2024

Hi!
Nice thing the multiple connection for input slot! Curious if already cover all the situations (there are many places to be touched..), and curious too if you did just for events of for general slots (that would need careful thinking and per node type enabling to correctly manage the data).
What I do to archive the same thing is to have an event sequencer node before to accept multiple event slots on the left and one or more on the right.
image

One thing to do is to make a default mode for the graph so all the nodes will start with that mode. Additionally depending on the usage one would want to prevent changing mode from the user and leave that on the node type themselves.

The onTrigger mode will create the two arrow shaped slots to facilitate the usage. Slots are marked as optional so user can remove those if not needed. Removing them automatically could be done but I would carefully do that by option or by user confirmation just to prevent accidentally removing connections.
There is an option to auto add those slots by using the optional inputs/outputs, and the one I prefer is to just drag an event slot onto a node, that will create that slot, change the mode, and connect.
image
Check the defaults marked as // [default!] , I had placed those to set my preferences but leaving the defaults as old behaviors to try to be always compatible with older implementations. (Anyway recently we have been diverging quite a bit)
image

@atlasan
Copy link
Collaborator

atlasan commented Jul 10, 2024

About graph running or not things have to be checked and thinking done:

  • an always-mode node will not run if graph is not running (BUT*)
  • events can be fired even if the graph is not running
  • (*) ancestor execution will eventually run nodes even if graph is not running

If you are trying and mixing modes it's better to have the graph running
alternatively one usage way without graph running would be using all onTrigger nodes (easy to do just dragging the events), in this way one would assure proper node execution order and no need to have graph running

@atlasan
Copy link
Collaborator

atlasan commented Jul 10, 2024

Just to know, I use editor_debug.html instead of index.html, that would load different defaults as per my preferences

@liusida
Copy link
Author

liusida commented Jul 10, 2024

The node events/sequence is nice! Thank you! no need for dirty hacking~~

image

It works when I keep the editor running and all the nodes are in default mode (I think it is the ALWAYS mode). If the graph is not running, I need to runStep() to flush the output to the console.
I think I'll just leave the editor running by default. It seems not consuming a lot of resource.

@DrJKL
Copy link

DrJKL commented Jul 10, 2024

Might want to bring this proposal over to https://github.com/litegraph-tsx/litegraph.tsx

@atlasan
Copy link
Collaborator

atlasan commented Jul 10, 2024

Hi @DrJKL !
Which is the proposal you are referring to?

@DrJKL
Copy link

DrJKL commented Jul 10, 2024

Allowing for selective cycles for signals.
@daniel-lewis-ab started that organization/repo for just this kind of collaboration

@atlasan
Copy link
Collaborator

atlasan commented Jul 10, 2024

Yep! I know that, approve.
But what do you mean by selective cycles for signals?
If you mean the event sequence node, that has been there since I have knowledge of it.

@liusida
Copy link
Author

liusida commented Jul 11, 2024

So are we moving to the tsx repo? I've just switched to this one from the original litegraph.js.

Where is the future, @daniel-lewis-ab ?

@daniel-lewis-ab
Copy link
Owner

daniel-lewis-ab commented Jul 11, 2024 via email

@liusida
Copy link
Author

liusida commented Jul 12, 2024

Thanks for the detailed response.

I haven't yet done the ES6 and modularization
changes. I'm hoping to look at it again this weekend, and once I've done
so we'll have a good foundation to figure out what we want to code up next.

I'll wait for your change and switch to the new repo. My code currently works with daniel-lewis-ab/litegraph.js.

My goals do include establishing a way to execute back-end code without
needing ComfyUI, and my thoughts for doing so was:

  1. Set up a node that can submit its inputs like a form.
  2. Set up a REST interface such that for any given python class, all of the
    methods are mapped out as REST endpoints that accept the arguments they
    expect from Litegraph.

Good to know the plan. I am currently trying to make a python back-end for litegraph alone, at least a proof-of-concept. I'll keep you guys updated.

However, I don't think the node should be created by submitting a form, not that easy. Creating new nodes should be done by professional developers, so they can bring all the deep learning functionalities to the litegraph front-end. (PyTorch is the most important python package I am aiming to integrate)

To me, the server side of this whole thing is its own project, because
any interface could use it, and this could use any back-end. In
principle. But I'm strongly in favor of seeing that approach taken
particularly because of the beauty of it.

While I totally agree with the N-N relationship between the front-end and the back-end, I think it is quite hard to define the common protocol. I'll make one back-end for now, but N-N is a great future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants