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

remove shared imoprt quick_setting form installer #7

Conversation

w-e-w
Copy link
Contributor

@w-e-w w-e-w commented Oct 17, 2023

remove shared imoprt quick_setting form installer

reason 1
trying to modify quicksettings_list in install it doesn't work
it will always be "UI Config not initialized"
the installer does not have access to which configuration file is supposed to be used
so we can't even directly read the json file

reason 2
modules.shared is it extremely heavy import, and as installer is run on every launch, it can significantly slow down webui launch

with shared without shared
image image

tip: the startup profiler can be found at the footer of webUI

whether or not to add sd_unet to the quicksettings_list for the user is a debatable question
on the one hand most people might consider it essential when using TRT, but others found it quote "intrusive"
also people might have special use cases, so at the very least it should be optional

if you wish to make modifications to quicksettings_list, I believe before_ui_callback is the place to do it
https://github.com/AUTOMATIC1111/stable-diffusion-webui/blob/5ef669de080814067961f28357256e8fe27544f4/modules/script_callbacks.py#L277
use on_before_ui(callback) to add a function there
https://github.com/AUTOMATIC1111/stable-diffusion-webui/blob/5ef669de080814067961f28357256e8fe27544f4/modules/script_callbacks.py#L464
which would get executed at
https://github.com/AUTOMATIC1111/stable-diffusion-webui/blob/5ef669de080814067961f28357256e8fe27544f4/webui.py#L61

@contentis
Copy link
Collaborator

Using before_ui_callback, won't this be launched each time webui is started? Ideally, this should only be done once to not overwrite a users settings if he disables it, reducing the "intrusiveness"

A suggestion on where to best call on_before_ui(callback), in ui_trt.py ?

@w-e-w
Copy link
Contributor Author

w-e-w commented Oct 18, 2023

some updated thoughts
I think sd_unetshould not be add to quick setting at all
the Automatic option should automatically select the TRT unet if available
which means if a person just want to use TRT they should just set it to Automatic

the only issue is that if one wants to disable TRT (maybe because they want to dynamically choose whitch LoRA to use) they will have to set sd_unet it to None


Using before_ui_callback, won't this be launched each time webui is started? Ideally, this should only be done once to not overwrite a users settings if he disables it, reducing the "intrusiveness"

yes but, it should be a simple matter to implement a check o do this only once

@contentis
Copy link
Collaborator

Ideally, Automatic should work, but do you have any idea how? :D

I had a version at one point that had [TRT] Automatic as an option. But that caused a bunch of issues that I hadn't had time to address. Additionally, there is the problem that LoRA is kind of a workaround and currently requires the user to set the SD Unet for that.

@w-e-w
Copy link
Contributor Author

w-e-w commented Oct 18, 2023

strange I was just testing the automatic option and it did work fine for me

also going by the commit date AUTOMATIC added the custom unit sd_unet was added one day before his implementation of TRT was released
specifically for TRT
https://github.com/AUTOMATIC1111/stable-diffusion-webui/blame/5ef669de080814067961f28357256e8fe27544f4/modules/sd_unet.py#L18-L31

@contentis
Copy link
Collaborator

strange I was just testing the automatic option, and it did work fine for me

I mean, of course, this was fully intentional 👀

@w-e-w
Copy link
Contributor Author

w-e-w commented Oct 18, 2023

strange I was just testing the automatic option, and it did work fine for me
I mean, of course, this was fully intentional 👀

ahh, kind of lost, maybe I miss read something but good work I guess? 🙃


depending on the situation maybe get_unet_option(option=None) can be hijacked by TRT extension
but ideally it might be possible to build an extra logic to handle LoRA

also even if automatic lora works flawlessly, I think there still need to be a way to disable TRT boath manually and programmably by other extensions
as I recall some extensions work on the concept of changing unet weights during generation

@contentis
Copy link
Collaborator

ahh, kind of lost, maybe I miss read something but good work I guess? 🙃

Tried to be funny, didn't seem to have worked out 😄


For now, I removed the shard import and config mod in the hot_fix branch.

One general issue I'm facing for LoRA and hires.fix, that I don't know ahead of time what configuration is going to be used. I only get the forward call, and based on that I need to decide what to do. Ideally I'd when a user hits generate I'd like to know what settings will be used to make better decisions on what to load. And for LoRA ideally just apply the weights JIT

@w-e-w
Copy link
Contributor Author

w-e-w commented Oct 18, 2023

not sure if this helps
there is a script call back after_extra_networks_activate using it you would have access to extra_network_data
a data structure that contains all network type name and weight

https://github.com/AUTOMATIC1111/stable-diffusion-webui/blob/5ef669de080814067961f28357256e8fe27544f4/modules/scripts.py#L151-L164

if you require something to get stuff working it is totally possible to make PRs to add extra callbacks to the generation pipeline

@contentis
Copy link
Collaborator

hmm, that might work... Before that I'll need to redo the core refitting to be "torch-compatible", and maybe then we can do JIT LoRA with no extra steps. But this might be a while.

If it is okay with you, I'd close this PR in favour of #43 which will hopefully be merged later today.

Thank you again for your input!

@w-e-w
Copy link
Contributor Author

w-e-w commented Oct 18, 2023

good luck, if you have other questions feel free to ask me
not sure if your company policy would allow but maybe it's a good idea for you guys to join AUTOMATIC discord server
you will be able to find people are more knowledgeable on other aspects
invitation link can be found at https://github.com/AUTOMATIC1111/stable-diffusion-webui/wiki/Contributing

I can close it myself 😜

@w-e-w w-e-w closed this Oct 18, 2023
@w-e-w w-e-w deleted the remove-shared-imoprt-quick_setting-form-installer branch October 18, 2023 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants