ModelSurge / sd-webui-comfyui

An extension to integrate ComfyUI workflows into the Webui's pipeline
MIT License
516 stars 32 forks source link

AttributeError: module 'comfy.sd' has no attribute 'ModelPatcher' #207

Open Ainaemaet opened 10 months ago

Ainaemaet commented 10 months ago

Hello,

When trying to use the Webui Checkpoint node I get the following warning in sdwebui-comfyui window, "Error occurred when executing KSamplerAdvanced:"

Followed by the below terminal traceback

ERROR:root:!!! Exception during processing !!! ERROR:root:Traceback (most recent call last): File "/my_path/username/stable-diffusion-webui/extensions/sd-webui-comfyui/ComfyUI/execution.py", line 155, in recursive_execute output_data, output_ui = get_output_data(obj, input_data_all) File "/my_path/username/stable-diffusion-webui/extensions/sd-webui-comfyui/ComfyUI/execution.py", line 85, in get_output_data return_values = map_node_over_list(obj, input_data_all, obj.FUNCTION, allow_interrupt=True) File "/my_path/username/stable-diffusion-webui/extensions/sd-webui-comfyui/ComfyUI/execution.py", line 78, in map_node_over_list results.append(getattr(obj, func)(*slice_dict(input_data_all, i))) File "/my_path/username/stable-diffusion-webui/extensions/sd-webui-comfyui/ComfyUI/nodes.py", line 1389, in sample return common_ksampler(model, noise_seed, steps, cfg, sampler_name, scheduler, positive, negative, latent_image, denoise=denoise, disable_noise=disable_noise, start_step=start_at_step, last_step=end_at_step, force_full_denoise=force_full_denoise) File "/my_path/username/stable-diffusion-webui/extensions/sd-webui-comfyui/ComfyUI/nodes.py", line 1325, in common_ksampler samples = comfy.sample.sample(model, noise, steps, cfg, sampler_name, scheduler, positive, negative, latent_image, File "/my_path/username/stable-diffusion-webui/extensions/sd-webui-comfyui/ComfyUI/comfy/sample.py", line 93, in sample real_model, positive_copy, negative_copy, noise_mask, models = prepare_sampling(model, noise.shape, positive, negative, noise_mask) File "/my_path/username/stable-diffusion-webui/extensions/sd-webui-comfyui/ComfyUI/comfy/sample.py", line 86, in prepare_sampling comfy.model_management.load_models_gpu([model] + models, model.memory_required([noise_shape[0] 2] + list(noise_shape[1:])) + inference_memory) File "/my_path/username/stable-diffusion-webui/extensions/sd-webui-comfyui/lib_comfyui/webui/proxies.py", line 64, in getattr return functools.partial(getattr(comfy.sd.ModelPatcher, item), self) AttributeError: module 'comfy.sd' has no attribute 'ModelPatcher'

Ainaemaet commented 10 months ago

I know you are all very busy, but would it be possible to please get a response on this? I love the extension, but using the regular checkpoint loaders it takes a model that is already in memory in auto, a couple of minutes to load. WebUI checkpoint loader whatever model I want is already there, and I can swap them using auto in 10 seconds or less.

ljleb commented 10 months ago

I understand the importance of this feature. I have started working on temporarily fixing the node yesterday when I saw interest by the community in this issue.

This problem is hard to solve consistently because it relies on very brittle hacks. As you already know, I opened an issue in comfyui for this matter: https://github.com/comfyanonymous/ComfyUI/issues/2407

What I intend to do in the following days:

  1. get the node to work again
  2. get the node to work well (so make the patch more reliable with a companion PR in comfyui directly)

I don't know exactly yet how to get around 2). It doesn't seem like a straightforward thing to achieve given comfyui wasn't designed for this.

Ainaemaet commented 10 months ago

Thank you so much for the response. That's wonderful news. I did leave a comment on your comfyanonymous thread in hopes it might help add a small bit of weight so they at least know it's important to some people... I'd imagine they must be very busy; this extension just brings everything to another level so I hope they will take it seriously.

I really appreciate all you are doing!

ljleb commented 10 months ago

If anyone can give a hand with the stability issue (point 2. above), we might get to it quicker. More stable solutions that don't rely on changing comfyui itself should be preferred over solutions that need to change comfyui. Although I haven't found a way that doesn't rely on direct changes in comfyui.

Typically, to achieve what we want, comfyui would employ a strategy pattern where one implementation is an in-memory model, and the other is for remote models that can be located in a different process. The problem is, there are a lot of direct dependencies on the model class. Comfyui even adds their own custom methods to the class, some of which assume a state dict is directly available. Because of this, making this work in comyfui directly will raise the burden of maintenance in comfyui since it requires either a lot of changes or making two large model classes.

When I get to it, I will experiment with multiple approaches. I cannot guarantee comfyanonymous will accept any potential solution I find.

Ainaemaet commented 10 months ago

I think my own capability would be woefully inadequate for the task (I have some C++ experience and the rest of my programming dates back to QBasic/Pascal) but I will certainly do what I can to draw some attention to it!

Ainaemaet commented 9 months ago

Is there nobody here willing or able to help @ljleb? This seems like something that would be thought of as essential for such an extension, I'm really surprised nobody has stepped up to the plate.

I've been trying to learn python as fast as I can.

PladsElsker commented 9 months ago

Sorry about that. I don't use the extension much anymore, so I have little incentive to fix this. It would take a lot of time for me as well. If you figure it out, do make a PR. We still review them.

Ainaemaet commented 9 months ago

Sorry about that. I don't use the extension much anymore, so I have little incentive to fix this. It would take a lot of time for me as well. If you figure it out, do make a PR. We still review them.

No worries, I will likely try a crack of it out of curiosity and for learning purposes but mmmv given my own busy schedule and limited skillset.

Given the rising popularity of sd-forge and other UI's, is it safe to assume development here isn't a priority?

PladsElsker commented 9 months ago

Given the rising popularity of sd-forge and other UI's, is it safe to assume development here isn't a priority?

I mean, for now, the goal is to keep the repo in a working state at least for the main UIs (SDNext, Webui). If major breaking changes occur for the img2img and txt2img tabs, we should be able to address them pretty easily, and in a short period of time, in most cases.

Adding new features to it, less of a priority right now for me at least, yes (assuming that's what you mean by "developpment isn't a priority").

PladsElsker commented 9 months ago

I wouldn't mind if someone tried to make his own fork of the repo and fix all the basic issues that remain. I think that would amazing, the project isn't in MIT license for nothing.

We'll definitely try our best to keep the repo in a working state until that happens, though.

Ainaemaet commented 8 months ago

I wouldn't mind if someone tried to make his own fork of the repo and fix all the basic issues that remain. I think that would amazing, the project isn't in MIT license for nothing.

We'll definitely try our best to keep the repo in a working state until that happens, though.

Sorry for the late reply - yes, that is what I meant.

It's nice to see at least that dedication to not leaving anyone hanging. I decided to move to native ComfyUI install and not sure if or when I'll look back now that I've gotten the hang of it. Funny thing is, my Auto1111 still loads models quicker XD.

Cheers.