-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Base of modular backend #6606
Base of modular backend #6606
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work 💪
I did an initial round of review. I haven't looked at diffusion_backend.py
yet - I just focused on the new backend 'infrastructure'.
I did a really quick smoke test. SD1.5 worked, SDXL failed. I'll do much more testing once this is closer to being merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another round of review - I got to the remaining code that I hadn't looked at yet.
invokeai/backend/stable_diffusion/diffusion/conditioning_data.py
Outdated
Show resolved
Hide resolved
invokeai/backend/stable_diffusion/diffusion/conditioning_data.py
Outdated
Show resolved
Hide resolved
invokeai/backend/stable_diffusion/diffusion/conditioning_data.py
Outdated
Show resolved
Hide resolved
I tested and wasn’t able to replicate getting discrepancies on both sd1.5 and sdxl (I.e., all the same) |
I dug into the image discrepancies some more. Notes from my tests:
I wish we understood the source of the non-determinism better, but given that it's present on main as well, I have no reason to believe that this PR introduces any new reproducibility issues. |
## Summary Fix function call that we forgot to update in #6606 ## QA Instructions Run a TiledMultiDiffusionDenoiseLatents invocation and make sure it doesn't crash. ## Checklist - [x] _The PR has a short but descriptive title, suitable for a changelog_ - [x] _Tests added / updated (if applicable)_ - [x] _Documentation added / updated (if applicable)_
## Summary Rescale CFG code from #6577. ## Related Issues / Discussions #6606 https://invokeai.notion.site/Modular-Stable-Diffusion-Backend-Design-Document-e8952daab5d5472faecdc4a72d377b0d ## QA Instructions Run with and without set `USE_MODULAR_DENOISE` environment. ~~Note: for some reasons there slightly different output from run to run, but I able sometimes to get same output on main and this branch.~~ Fix presented in #6641. ## Merge Plan ~~Nope.~~ Merge #6641 firstly, to be able see output difference properly. If you think that there should be some kind of tests - feel free to add. ## Checklist - [x] _The PR has a short but descriptive title, suitable for a changelog_ - [ ] _Tests added / updated (if applicable)_ - [ ] _Documentation added / updated (if applicable)_
## Summary FreeU code from #6577. Also fix issue with sometimes slightly different output. ## Related Issues / Discussions #6606 https://invokeai.notion.site/Modular-Stable-Diffusion-Backend-Design-Document-e8952daab5d5472faecdc4a72d377b0d ## QA Instructions Run with and without set `USE_MODULAR_DENOISE` environment. ## Merge Plan Nope. If you think that there should be some kind of tests - feel free to add. ## Checklist - [x] _The PR has a short but descriptive title, suitable for a changelog_ - [ ] _Tests added / updated (if applicable)_ - [ ] _Documentation added / updated (if applicable)_
## Summary ControlNet code from #6577. ## Related Issues / Discussions #6606 https://invokeai.notion.site/Modular-Stable-Diffusion-Backend-Design-Document-e8952daab5d5472faecdc4a72d377b0d ## QA Instructions Run with and without set `USE_MODULAR_DENOISE` environment. ## Merge Plan Merge #6641 firstly, to be able see output difference properly. If you think that there should be some kind of tests - feel free to add. ## Checklist - [x] _The PR has a short but descriptive title, suitable for a changelog_ - [ ] _Tests added / updated (if applicable)_ - [ ] _Documentation added / updated (if applicable)_
## Summary Seamless code from #6577. ## Related Issues / Discussions #6606 https://invokeai.notion.site/Modular-Stable-Diffusion-Backend-Design-Document-e8952daab5d5472faecdc4a72d377b0d ## QA Instructions Run with and without set `USE_MODULAR_DENOISE` environment. ## Merge Plan Nope. If you think that there should be some kind of tests - feel free to add. ## Checklist - [x] _The PR has a short but descriptive title, suitable for a changelog_ - [ ] _Tests added / updated (if applicable)_ - [ ] _Documentation added / updated (if applicable)_
## Summary T2I Adapter code from #6577. ## Related Issues / Discussions #6606 https://invokeai.notion.site/Modular-Stable-Diffusion-Backend-Design-Document-e8952daab5d5472faecdc4a72d377b0d ## QA Instructions Run with and without set `USE_MODULAR_DENOISE` environment. ## Merge Plan Nope. If you think that there should be some kind of tests - feel free to add. ## Checklist - [x] _The PR has a short but descriptive title, suitable for a changelog_ - [ ] _Tests added / updated (if applicable)_ - [ ] _Documentation added / updated (if applicable)_
## Summary Code for inpainting and inpaint models handling from #6577. Separated in 2 extensions as discussed briefly before, so wait for discussion about such implementation. ## Related Issues / Discussions #6606 https://invokeai.notion.site/Modular-Stable-Diffusion-Backend-Design-Document-e8952daab5d5472faecdc4a72d377b0d ## QA Instructions Run with and without set `USE_MODULAR_DENOISE` environment. Try and compare outputs between backends in cases: - Normal generation on inpaint model - Inpainting on inpaint model - Inpainting on normal model ## Merge Plan Nope. If you think that there should be some kind of tests - feel free to add. ## Checklist - [x] _The PR has a short but descriptive title, suitable for a changelog_ - [ ] _Tests added / updated (if applicable)_ - [ ] _Documentation added / updated (if applicable)_
## Summary Code for lora patching from #6577. Additionally made it the way, that lora can patch not only `weight`, but also `bias`, because saw some loras which doing it. ## Related Issues / Discussions #6606 https://invokeai.notion.site/Modular-Stable-Diffusion-Backend-Design-Document-e8952daab5d5472faecdc4a72d377b0d ## QA Instructions Run with and without set `USE_MODULAR_DENOISE` environment. ## Merge Plan Replace old lora patcher with new after review done. If you think that there should be some kind of tests - feel free to add. ## Checklist - [x] _The PR has a short but descriptive title, suitable for a changelog_ - [ ] _Tests added / updated (if applicable)_ - [ ] _Documentation added / updated (if applicable)_
Summary
Base code of new modular backend from #6577.
Contains normal generation and regional prompts support.
Also preview extension included to test if extensions logic works.
Related Issues / Discussions
https://invokeai.notion.site/Modular-Stable-Diffusion-Backend-Design-Document-e8952daab5d5472faecdc4a72d377b0d
QA Instructions
Run with and without set
USE_MODULAR_DENOISE
environment.Currently only normal and regional conditionings supported, so just generate some images and compare with main output.
Merge Plan
Discuss a bit more about injection point names?
As if for example in future unet will be overridable, current
pre_unet
/post_unet
assumes to name override asunet
what feels a bit odd.Also
apply_cfg
- future implementation could ignore/not use cfg, so in this casecombine_noise_predictions
/combine_noise
seems more suitable.Checklist