-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[chore] fix: retain memory utility. #9543
Conversation
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
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.
Lightning fast fix, thanks! I'm not sure about the name retain_memory
though, as retain means to keep something as-is. Maybe free_memory
or something would be better? No problem if not though
I had a different meaning in mind, don't know why. But I get your point. Will rename to |
* fix: retain memory utility. * fix * quality * free_memory.
* fix: retain memory utility. * fix * quality * free_memory.
What does this PR do?
My bad that in
diffusers/src/diffusers/training_utils.py
Line 263 in 534848c
I assumed that the scope of
del
would carry forward but that is not the case. So, even when we calldel obj
it actually just deletes the local reference, it doesn't carry forward. So, we're better off without it and doingdel
explicitly (like the changes done in this PR).Thanks to @a-r-r-o-w for catching this. It didn't go to a release yet, so, I think this shouldn't have any issues.