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

Forge was updated today - Errors on inference when Model Mixer isn't enabled #161

Open
CCpt5 opened this issue Jul 28, 2024 · 7 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@CCpt5
Copy link

CCpt5 commented Jul 28, 2024

Hey!

Forge lllyasviel was very kind and provided a much needed update to Forge today (including upgrading GRadio to 4.0). I installed MMixer from the installer and w/o even enabling it I've noticed errors in the console:

https://github.com/lllyasviel/stable-diffusion-webui-forge/commits/main/

---
To load target model SDXL
Begin to load 1 model
Reuse 1 loaded models
[Memory Management] Current Free GPU Memory (MB) =  15968.17822265625
[Memory Management] Model Memory (MB) =  0.0
[Memory Management] Minimal Inference Memory (MB) =  1024.0
[Memory Management] Estimated Remaining GPU Memory (MB) =  14944.17822265625
Moving model(s) has taken 0.02 seconds
100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 22/22 [00:05<00:00,  3.97it/s]
*** Error running before_process: D:\stable-diffusion-webui-forge\extensions\sd-webui-model-mixer\scripts\model_mixer.py                                                                                                                           | 132/56100 [00:37<3:54:53,  3.97it/s]
    Traceback (most recent call last):
      File "D:\stable-diffusion-webui-forge\modules\scripts.py", line 836, in before_process
        script.before_process(p, *script_args)
    TypeError: ModelMixerScript.before_process() missing 6 required positional arguments: 'enabled', 'model_a', 'base_model', 'mm_max_models', 'mm_finetune', and 'mm_states'
@wkpark wkpark self-assigned this Jul 28, 2024
@wkpark wkpark mentioned this issue Jul 30, 2024
2 tasks
@wkpark wkpark added the bug Something isn't working label Jul 30, 2024
@wkpark
Copy link
Owner

wkpark commented Jul 31, 2024

several gradio4 related bugs fixed.
but the current forge seems too unstable.

There is a change that causes compatibility issues with the A1111 webui.
It's better to wait until it's more stable and the compatibility issues have been resolved.


  • low level gradio component Generate button hooking will not work. - for example, AutoMerger does not work with the latest forge.

@CCpt5
Copy link
Author

CCpt5 commented Jul 31, 2024

Ok great, thanks for working on it! I'm sure it'll settle in at some point, i'll use A1111 or any merging at this point. Can close if you want or leave open. Thx.

@CCpt5
Copy link
Author

CCpt5 commented Aug 20, 2024

Developer lllyasviel (Forge) has been on an incredible coding run and it seems like he's getting it to the point of stable.

Curious about your thoughts on Flux merging? Should it be possible with 64(+) system ram?

@wkpark
Copy link
Owner

wkpark commented Aug 21, 2024

Developer lllyasviel (Forge) has been on an incredible coding run and it seems like he's getting it to the point of stable.

Curious about your thoughts on Flux merging? Should it be possible with 64(+) system ram?

current Forge system is incompatible with the original webui.
(it looks like forge use newly adapted the comfyui's backend engine)
so, model-mixer does not support Flux merging, currently.

SD1.x, SD2.x and SDXL is basically the same structure but SD3 and flux are totally different.
so, webui specific features will not work.

The basic principle of merging models is simple, but getting the model mixer to work with flux seems to be not a simple task,

  • disable some webui specific features for flux or refactor to support all features.
    • disable block level merge
    • or make another/modular flux specific block level merge routines.
    • disable partial merge feature ... and so on.
    • or refactor to support partial merge

and about system memory,

  • SD1.x, SD2.x - read whole weights of Model-A into RAM, and also read model-B, C, D... into RAM sequantially.
  • for SDXL, whole model-A weights into RAM and open model-B,C,D ... using safetensors.torch.safe_open().

for FLUX, we can use safe_open() always to reduce ram usage.

@inflatebot
Copy link

Yeah, I wouldn't hold my breath WRT Forge becoming compatible with A1111 again, at least not for a long time. lllyasviel's been rewriting the whole thing, moving the codebase into a proper inheritance tree and yeeting the old LDM library into the stratosphere. I had to port NegPiP by hand. While it didn't take that long, it was tedious, because despite Forge currently having moved to the new backend already, there's not actually any documentation on it, and it's way more complex than I'm comfortable with. I basically had to print out the relevant objects' members one at a time, restarting Forge and trying a generation, until I could drill down to the parameters that matched the old ones what needed replacing. I have no idea why he didn't just provide a temporary mapping so that the extensions that survived the Gradio 4 transition didn't spontaneously explode. Or, y'know, do that work in a branch?

Anyways, if you're actually any good at Python unlike me, it shouldn't be that big of a problem. Don't mind me having a complain in your repo.

(They're not using ComfyUI as a backend by the way, that's a misconception.)

@wkpark
Copy link
Owner

wkpark commented Sep 18, 2024

yes. forge's backend not compatible with comyui at all.
but many parts of forge engine seems based on comfy's work

  • comfy's ops.py -> forge's operations.py - naming is similar, overall structure are very similar but operations.py use @contextlib.contextmanager monkey patch method
  • comfy's memory_management.py -> forge's memory_management.py - looks like copy and pasted but modified a lot.
  • (originally I said, "it looks like" forge use "newly adapted" the comfyui's backend engine. its not the same "comfyui's engine" but I admit It could be misunderstood)

and thanks for your comments!

@inflatebot
Copy link

(originally I said, "it looks like" forge use "newly adapted" the comfyui's backend engine. its not the same "comfyui's engine" but I admit It could be misunderstood)

I see, it was me that misunderstood you, then. Apologies for that!

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