-
Notifications
You must be signed in to change notification settings - Fork 469
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
Memory-efficient attention (also code cleaned up and a colab added) #103
Conversation
Update license, readme, models, model card, requirements and sampling script.
now i can run 704x704 images on my 1050ti and 640x640 in turbo |
Hi, a big thank you for the commits, I am travelling right now, will merge these changes as soon as I get stable internet. Thanks again! |
It would probably be better to remove the cleanups and "beautifying" commits from this PR. Along with the other unrelated changes (Add diffusers, colab notebooks, etc). They're unrelated to the attention.py change and all the whitespace and code-rearrangements are going to make future PRs and code merges a major pain. |
We just replace the code on attention.py with the new one and that's all right? |
LICENSE
Outdated
Copyright (c) 2022 Robin Rombach and Patrick Esser and contributors | ||
|
||
CreativeML Open RAIL-M |
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.
?????
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.
From the CompVis SD github. This looks like it was merged then brought over here.
I would not merge this PR without combing through it. There's a lot of bullshit in here. Gotta love the Rick Astley image, which makes me wonder if the attention.py changes even work. Unfortunately I recently accidentally nuked my main install so I can port the relevant changes over one by one, so that might be a bit :v |
The license update, Rick Astley image, etc are all from the original SD github. It looks like the original commiter made their changes then rebased the CompVis repo on top of it for some reason. Core SD rickrolls you when you trigger its NSFW filter. https://github.com/CompVis/stable-diffusion/blob/main/scripts/txt2img.py#L79 |
Maybe it's better for @basujindal to merge the changes from the original SD repo first, and then @neonsecret can then add their changes as a PR? Because otherwise when going through the git history, it will seem as if @neonsecret changed the license and added a Rickroll image when in fact it was the original repo changing first. They say PRs should be clean from other stuff that's not specific to the main change being made. |
well I can roll back those changes to the essential ones and then we can merge mine and then merge the original SD |
LICENSE
Outdated
|
||
2. Grant of Copyright License. Subject to the terms and conditions of this License, each Contributor hereby grants to You a perpetual, worldwide, non-exclusive, no-charge, royalty-free, irrevocable copyright license to reproduce, prepare, publicly display, publicly perform, sublicense, and distribute the Complementary Material, the Model, and Derivatives of the Model. | ||
3. Grant of Patent License. Subject to the terms and conditions of this License and where and as applicable, each Contributor hereby grants to You a perpetual, worldwide, non-exclusive, no-charge, royalty-free, irrevocable (except as stated in this paragraph) patent license to make, have made, use, offer to sell, sell, import, and otherwise transfer the Model and the Complementary Material, where such license applies only to those patent claims licensable by such Contributor that are necessarily infringed by their Contribution(s) alone or by combination of their Contribution(s) with the Model to which such Contribution(s) was submitted. If You institute patent litigation against any entity (including a cross-claim or counterclaim in a lawsuit) alleging that the Model and/or Complementary Material or a Contribution incorporated within the Model and/or Complementary Material constitutes direct or contributory patent infringement, then any patent licenses granted to You under this License for the Model and/or Work shall terminate as of the date such litigation is asserted or filed. |
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.
sell, sell, import
typo
"offer to sell" and "sell" are distinct
This reverts commit b713734.
Can someone explain why everywhere I see line: I don't think person by default wants to share ideas. And connect to gradio website.. So really suggestion to add only improvements, not random code :E |
it's to be able access your gradio remotely |
up to you guys what changes to apply |
There's another optimization you can do for less memory in there by changing
into
saves quite a bit of memory |
I don't think it will be noticeable |
For me it allows me to go up a few steps in resolution, I can go completely insane if I do the the softmax even more split up
but not sure if it's always 16? I do run on a 3090 though... |
for me in a bigger resolution the problem is with other modules, but l dont think rewriting the entire module is possible |
FWIW I am seeing a significant performance degradation (I cherry picked 47f8784) on a 16GB M1 MacMini: from < 3s/it to >7s/it. Others might want to double check and make sure this is not just my environment. |
for me speed is the same |
attn = sim.softmax(dim=-1) | ||
# attention, what we cannot get enough of, by halves | ||
sim[4:] = sim[4:].softmax(dim=-1) | ||
sim[:4] = sim[:4].softmax(dim=-1) |
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.
This leads to different similarity results though, no? -> do the outputs still look good without retraining the model?
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.
exactly the same
okay guys you know what, ima do a separate branch with only attention change and do a PR for it separately |
closing this because #117 will be more relevant |
The inpaint mask fix should have a pull request too IMO, that's fairly important |
Apologies... where is the Colab? |
Oh, I tried that (I thought that the basujindal fork was a Colab now) and it fails to launch an Xterm window at the end of setting up the Colab. |
I just tried this and unfortunately observed no changes to memory utilization, and a very slight decrease in performance. |
Uses less memory, now can generate 576x1280 images with 6 gb vram