rgthree / rgthree-comfy

Making ComfyUI more comfortable!
MIT License
1.01k stars 77 forks source link

KSampler Config is no longer working for sampler_name since Oct 05 #41

Open rodrigolourenco234 opened 11 months ago

rodrigolourenco234 commented 11 months ago

Since ComfyUI update from Oct 5, the KSampler Config Module is no longer able to connect sampler_name or scheduler with any other module. Tested with default KSamplers and a few other like Impact FaceDetailer and EfficientNode.

rodrigolourenco234 commented 11 months ago

image

rgthree commented 11 months ago

Yea, looks like there's a lot of churn there in the type.. not sure what to do, they've broken it twice in 6fc7314 and then changed it again in b9b178b.

I think the place they'll end up is way better than what they had, but I'm not sure the current design takes anything into consideration that isn't the specific PrimitiveNode, which doesn't help this node (or any other node with list outputs)

rodrigolourenco234 commented 11 months ago

Found a workaround, if you use a reroute (rgthree) and connect it first to the KSampler for instance, and then to KSampler Config, you can use for the session. However, you lose the connection when you load a workflow.

image

rgthree commented 11 months ago

Interesting, maybe I can fix the disconnecting from my end while they figure out what they want to do with list types more broadly.

FWIW, I created an issue here: https://github.com/comfyanonymous/ComfyUI/issues/1674

rodrigolourenco234 commented 11 months ago

That would be awesome for now! Thank you!

rgthree commented 11 months ago

Alright, reroute nodes shouldn't disconnect with the latest. Still have to connect ksampler first.. it's kind of a bug that it works, but I won't fix it for now until comfyui addresses the list types. :)

Keeping this open since it's not final.

DrJKL commented 4 months ago
FaceDetailerPipe:
    - Return type mismatch between linked nodes: scheduler, ['normal', 'karras', 'exponential', 'sgm_uniform', 'simple', 'ddim_uniform'] != ['normal', 'karras', 'exponential', 'sgm_uniform', 'simple', 'ddim_uniform', 'AYS SDXL', 'AYS SD1', 'AYS SVD']

Source of the issue: https://github.com/ltdrdata/ComfyUI-Impact-Pack/blob/Main/modules/impact/core.py#L44
Commit: https://github.com/ltdrdata/ComfyUI-Impact-Pack/commit/1b4b8305b48c6e5adb494c08c33b3447272173a4

Igneom commented 1 month ago
FaceDetailerPipe:
    - Return type mismatch between linked nodes: scheduler, ['normal', 'karras', 'exponential', 'sgm_uniform', 'simple', 'ddim_uniform'] != ['normal', 'karras', 'exponential', 'sgm_uniform', 'simple', 'ddim_uniform', 'AYS SDXL', 'AYS SD1', 'AYS SVD']

Source of the issue: https://github.com/ltdrdata/ComfyUI-Impact-Pack/blob/Main/modules/impact/core.py#L44 Commit: ltdrdata/ComfyUI-Impact-Pack@1b4b830

I've been having the same issue with schedulers, but coming from the efficiency nodes.

Commit: https://github.com/jags111/efficiency-nodes-comfyui/commit/0ee8860d6b65cd3b297d5fb23378bd970617fa13

Not proficient at all with python, and have no idea how this could be made compatible with efficiency and impact nodes at the same time, specially since it seems to be a comfyui problem, as said previously.

But, since I mostly prefer the efficiency KSampler, I fixed on my end by adding their SCHEDULERS line to ksample_config.py, and updating the scheduler types to point to it. At first I tried to import it directly from the efficiency nodes, but my lack of knowledge in python didn't help much.

Of course, trying to use any of the non default schedulers on regular KSampler, will throw an error. Possibly getting it from impact pack can fix it for both samplers, since it has more options on impact, but the same error would show up, if choosing a non supported scheduler for the sampler to which it's connected.

zierf commented 1 month ago

Unfortunately, the ComfyUI-Inspire-Pack is also affected. The scheduler cannot be connected directly there either.

The workaround with a reroute node looked promising, but something seems to get lost on the way to the image preview and no new image arrives there.


At least I stumbled upon a workaround for the Inspire Pack and Impact Pack found by doomndoom on Reddit. You can connect the scheduler via the Impact Scheduler Adapter (from the Impact Pack).

20240824_165409


However, this workaround does not work for the Efficiency Nodes nor for _ComfyUI_tinyterraNodes_.

20240824_165849