Repository navigation
Not copying the model in inference and assigning ema_weights seems incorrect. #2321
Description
Describe the bug
This recent commit 44f6bc8 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
diffusersversion: 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?:
Activity
patrickvonplaten commentedon Feb 13, 2023
@williamberman @patil-suraj could you take a look here?
williamberman commentedon Feb 16, 2023
I think @dasayan05 is correct here. My understanding is that the underlying parameters in the
preparedmodel and theunwrappedmodel 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?
sayakpaul commentedon Feb 16, 2023
Yeah, this is indeed a bug. Probably with #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?
github-actions commentedon Mar 14, 2023
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.
patrickvonplaten commentedon Mar 14, 2023
Is this issue solved or do we still need to do something here? cc @sayakpaul
sayakpaul commentedon Mar 15, 2023
Yeah this issue is solved now with #2302
williamberman commentedon Mar 16, 2023
I think this is still a bug as
store()andrestore()are not called in train_unconditionalpatrickvonplaten commentedon Mar 16, 2023
Thanks for the follow-up PR @williamberman !