-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Update nodes.py so VAEDecode falls back to a tiled method if GPU runs out of memory #5427
base: master
Are you sure you want to change the base?
Conversation
patched VAEDecode if fails due to lack of VRAM fallback to tiled decode method
There is already logic to handle this here: https://github.com/comfyanonymous/ComfyUI/blob/master/comfy/sd.py#L344 Which OS and which pytorch version are you using? |
OS: Windows 10 Pro
I have a similar logic as what you linked, but it never works, at least not on my device. |
In your patch, can you show what kind of exception occurs when there is no exception handler? |
Here is the full error report when I remove my patch from the latest available version of ComfyUI. ComfyUI Error ReportError Details
Stack Trace
System Information
Devices(PowerColor 6600xt 8gb will be reported incorrectly with directml. A1111 has this same issue.)
Logs
Attached WorkflowPlease make sure that workflow does not contain any sensitive information such as API keys or passwords.
Additional Contextnone As you can see, it's a pretty significant error when the AMD GPUs run out of memory. This error is specific to AMD GPUs that do not support ROCm. I have no way of testing if ROCm enabled devices handle or report out-of-memory errors differently. I hope this is enough information to support the need for this change. It's a see here for more information on the OOMException from the modelmanagement.py file ComfyUI/comfy/model_management.py Line 153 in cc9cf6d
|
The original code only catches |
revert change to VAEDecode node.
Move catching of RuntimeError and MemoryError to sd.py
Remove unnecessary parameter to decode method
That is what my PR does, catching the RuntimeError as well as any other MemoryError in the decoding process itself. I have modified the code to reflect this, moved it out of the node and into the See screenshots: |
I believe, at this point, my list of potential side-effects or concerns has been reduced to none. |
Just checking in to see why this hasn't been merged yet. |
Issue
As the title suggests, if the GPU runs out of VRAM during a VAEDecode event, then the entire queued prompt fails. With as many enhancements as ComfyUI has received over the last months, it is all but certain that this failure should no longer exist.
Solution
I have introduced a small change to the VAEDecode node so that it will fallback to a tiled decode process should the GPU run out of memory.
How to test
Run a sufficiently large image generation on a GPU with 8GB of VRAM or less. VAEDecode node will fail due to lack of GPU memory, especially on an AMD GPU that does not support rocm.
Potential side effects or concerns
The VAEDecode node has changed significantly to include batches, which is fine, however the tiled decoding method does not include such support. There is a possibility that the tiled method may fail here. I haven't been able to find any instances where it would fail with simple image generations. In these cases, it may be that the user is simply trying to do too much and should instead switch to a device that has a GPU with more VRAM.