diffusers
diffusers copied to clipboard
Not copying the model in inference and assigning ema_weights seems incorrect.
Describe the bug
This recent commit https://github.com/huggingface/diffusers/commit/44f6bc81c764897406952bf75263b866de7c8cd3 regarding the PR #2166 removed copying the model while doing inference. Is this correct ? Aren't we substituting EMA weight to the underlying model (in the next line) which is being trained ?
cc: @patil-suraj @pcuenca
PS: @stathius noticed this bug.
Reproduction
The example file itself.
Logs
No response
System Info
-
diffusers
version: 0.13.0.dev0 - Platform: Linux-5.4.0-26-generic-x86_64-with-glibc2.31
- Python version: 3.9.16
- PyTorch version (GPU?): 1.13.1+cu117 (True)
- Huggingface_hub version: 0.11.1
- Transformers version: 4.26.0
- Accelerate version: 0.15.0
- xFormers version: not installed
- Using GPU in script?:
- Using distributed or parallel set-up in script?:
@williamberman @patil-suraj could you take a look here?
I think @dasayan05 is correct here. My understanding is that the underlying parameters in the prepared
model and the unwrapped
model are still the same. EMAModel.copy_to will then copy the ema tensors over the non-ema tensors in the existing parameters. So every time we do this sample, we then start training over the ema values.
@patil-suraj @pcuenca is this correct?
Yeah, this is indeed a bug. Probably with https://github.com/huggingface/diffusers/pull/2302 merged, we can modify the script accordingly. A working example of how it might look like in practice is available here. Is is what you had in mind?
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.
Please note that issues that do not follow the contributing guidelines are likely to be ignored.
Is this issue solved or do we still need to do something here? cc @sayakpaul
Yeah this issue is solved now with https://github.com/huggingface/diffusers/pull/2302
I think this is still a bug as store()
and restore()
are not called in train_unconditional
Thanks for the follow-up PR @williamberman !