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

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

Open
Ainaemaet opened this issue Jan 23, 2024 · 11 comments
Open

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

Ainaemaet opened this issue Jan 23, 2024 · 11 comments
Labels
bug Something isn't working

Comments

@Ainaemaet
Copy link

Ainaemaet commented Jan 23, 2024

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
Copy link
Author

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
Copy link
Contributor

ljleb commented Jan 26, 2024

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:
comfyanonymous/ComfyUI#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
Copy link
Author

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
Copy link
Contributor

ljleb commented Jan 26, 2024

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Contributor

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.

@PladsElsker PladsElsker added the bug Something isn't working label Feb 26, 2024
@Ainaemaet
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants