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

SageAttention not a normal model patch #173

Open
blepping opened this issue Jan 16, 2025 · 1 comment
Open

SageAttention not a normal model patch #173

blepping opened this issue Jan 16, 2025 · 1 comment

Comments

@blepping
Copy link

Your PatchSageAttention is not a normal model patch - if the user bypasses/mutes/bypasses it that won't disable the effect. You should warn the user so they don't accidentally leave it enabled. Tooltips/description in the node itself and a section in the README would be a good idea. This is the approach I took in my own nodes: https://github.com/blepping/ComfyUI-bleh#blehglobalsageattention

It's better to use something like a sampler wrapper for that kind of thing because that way it can safely be reverted with a context manager or try/finally block.

@kijai
Copy link
Owner

kijai commented Jan 16, 2025

Yeah that's true, it's mostly been an experimental node, I did look at yours at one point, but also wanted to try the different 2.0.0 modes to see if they make a difference. I'm very much not well versed in the patching system so this is what it is, you're right about the warning though and that should definitely be there.

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

No branches or pull requests

2 participants