AUTOMATIC1111 / stable-diffusion-webui

Stable Diffusion web UI
GNU Affero General Public License v3.0
143.63k stars 27.03k forks source link

fix RestrictedUnpickler #16574

Open wkpark opened 1 month ago

wkpark commented 1 month ago

it does no harm to remove the pytorch_lightening dependency in the safe.py (torch.load)

see also https://github.com/comfyanonymous/ComfyUI/commit/735ac4cf81862b21902b312930ebfc92eef63357

Checklist:

wkpark commented 1 month ago

it does no harm to remove the pytorch_lightening dependency in the safe.py (torch.load)

NO we do use those to load LDSR upscaller models

did you tested? this is a torch.load() fix. without pytorch_lightening in the unpicker, LDSR ckpt model can be loaded without an issue.

w-e-w commented 1 month ago

okay you are right, it dose seems to load it would seem my assumption about how pickle works is wrong

that being said, I still don't see a point of fixing something that isn't broke unlike comfy is not like this change will allows to remove pytorch_lightning as it still used elsewhere

reopening this letting others decide

wkpark commented 1 month ago

okay you are right, it dose seems to load it would seem my assumption about how pickle works is wrong

that being said, I still don't see a point of fixing something that isn't broke unlike comfy is not like this change will allows to remove pytorch_lightning as it still used elsewhere

reopening this letting others decide

As you may have guessed, a number of my recent PRs have been related to optimising startup loading time. some PRs are experimental (can be switched on/off), some PRs have been extracted from my recent Flux1 PR to reduce the total number of commits (not exactly related to the Flux parts)

the import time of pytorch_lightening is about ~5sec on my machine. (almost the same as imports torch) so Im trying to reduce any dependency of pytorch_ligenting from core.

thank you for your concern!