When using Apply Advanced ControlNet, muting that node, my workflow appears to remain cached

[ghostsquad]

I was previously using the vanilla Apply ControlNet (Advanced) node. I have a workflow, using the rgthree contexts, allows me to mute groups to enable/disable various mutations (such as the positive/negative conditioning from controlnets).

Though, switching to this repo's Apply Advanced ControlNet, it seems that even when this node is muted (and the seed is fixed), the workflow remains cached, and I end up with the image that was just made (when that controlnet was active).

I've verified the behavior by restarting ComfyUI between runs. I was successfully able to generate 2 very different images (one when controlnet was enabled, another disabled). But muting or unmuting this node doesn't seem to do anything. (For clarity, I am in fact muting, not bypassing).

[Kosinkadink]

Interesting. I have no custom JavaScript code or anything in this repo, so I think this may be a bug with something else. I have not had any issues with using the node with both mute and bypass.

I know that someone recently reported an issue where combining nodes into one that include Apply ControlNet nodes that makes the negative and positive inputs flip when bypassing, and that too is a bug with vanilla ComfyUI JavaScript code that I have no control over. Since your issue pops up when using an rgthree feature, I think your issue may originate there.

[ghostsquad]

Ya, I read that issue. Wow, this is a head scratcher. I was about to update this issue with an minimally example workflow, but my example workflow isn't exhibiting this behavior. There's something else going on...

[ghostsquad]

I found the offending node. (kinda). When using the KSampler Adv. (Efficient) AND when Apply Advanced ControlNet does NOT have the model input/output set, that's how to reproduce this.

As soon as you send the model (in and out) to the AAC, KSampler Adv. (Efficient) sees a change.

I made a workflow to show this. You can (kinda) see the top side does not pass the model (purple line) to controlnet. The bottom side does.

Here's run 1 (both disabled)

and here's the rerun, enabling both top and bottom. (this second image is an export of the workflow, so you can use that to reproduce/test on your side).

I think it's worth looking at the code here to see if there's any unexpected logic in regards to that option model.

[ghostsquad]

I added vanilla ksamplers to this workflow as well. This shows some weird behavior between the efficient KSampler and these ControlNet nodes.

[Kosinkadink]

There is nothing remarkable about the code side of things of Apply Advances ControlNet with the model besides 2 things: 1) the model is optional 2) if CN strength is set to 0.0, just like the inputted conds, the ModelPatcher obj won't get cloned and will just pass the same object along.

I don't think most nodes ever have an optional model output, so there might be some assumption made in the rgthree nodes that do the caching.

Ive planned on making a version of the Apply Advanced ControlNet node that makes model required so that it can be the new standard node since ControlLLLITE (and at some point other types) of CNs are now supported. I can release that node later today to see if that one will get around whatever assumption rgthree code may be making.

[ghostsquad]

Well, at least I have something that works. I'm sure someone else will scratch their head a bit if using the same combination of things. For reference, the Efficiency KSampler isn't related to rgthree.

[ghostsquad]

Regarding model_optional.clone(). That seems like a mistake. That seems like it would use up a lot more memory. Why clone it? https://github.com/Kosinkadink/ComfyUI-Advanced-ControlNet/blob/main/adv_control/nodes.py#L157

[Kosinkadink]

Another thing that is unique for the ControlLLLite implementation is that I require that the controlnet objects and patches applied to the model must be able to find each other. Without getting into the weeds of what this means, the inconsistency you see may be caused by order of execution - if it uses a model and conds that don't pair up, ControlLLLite will not work as expected.

And the clone is not a mistake; in ComfyUI, that "model" you see there is not the actual model, but the wrapper, known as a ModelPatcher. When you use something like Load Checkpoint node, a single instance of the model is created. As it goes into nodes like Load LoRA, or the Apply Advanced ControlNet nodes, the ModelPatcher that stores the patches to apply during sampling gets cloned, and the new patches are added to the wrapper.

During sampling, it checks if the actual model the ModelPatcher is currently loaded, and if so, it calls unpatch_model on the currently-loaded ModelPatcher to clean the underlying model, and then the relevant ModelPatcher applies it's patches to the underlying model.

Tl;dr cloning ModelPatcher does not clone the actual model, and is required so that each node with a model output that makes a change to the ModelPatcher does not impact the ModelPatchers of the nodes earlier in the chain.

[ghostsquad]

Ah, thanks for that explanation!

[Kosinkadink]

My hunch is the cause is the mismatch of the control net objs and the model patches due to whatever rgthree does that I was not aware could be possible. There is nothing they could do on their end to fix it really, so I would need to change my code to make that not be a requirement.

I think I've got an idea for how I could refactor my code to remove the need for model and conds to match, while maintaining compatibility with tiled samplers. I should have time next week to try it out, I'll keep you posted.

[ghostsquad]

Thanks for your hard work and time!

[Kosinkadink]

I finally had the opportunity to refactor the ControlLLLite code to 1) not require the model_optional input and 2) apply the patches at sampling time, meaning no need for the hacky implementation that needs the controlnet and attn patch objects to keep track of each other between the Apply Adv. ControlNet node and the relevant KSamplers.