crowsonkb / k-diffusion

Karras et al. (2022) diffusion models for PyTorch
MIT License
2.21k stars 371 forks source link

fix mistakes in calculation FID: sqrt root matrix, eps 1e-6, equation #78

Closed shonenkov closed 9 months ago

shonenkov commented 11 months ago

maybe I was mistaken, I checked tests for the calculation sqrt root matrix, FID in k-diffusion didnt work how I expected: https://docs.scipy.org/doc/scipy/reference/generated/scipy.linalg.sqrtm.html

and made some fixes for torch implementation according to diagonalization method: https://en.wikipedia.org/wiki/Square_root_of_a_matrix

crowsonkb commented 10 months ago

Do you get different FID values using the way you calculate FID in this pull request? How different are they?

shonenkov commented 9 months ago

yes, 8.1 (with bug in sqrt root matrix) vs 7.3 (clean-fid / fix) -- I am not comparing with official FID implementation

also I would like to mention, that "clean-fid" is not so clean (maybe I was mistaken): see: https://github.com/GaParmar/clean-fid/blob/main/cleanfid/inception_torchscript.py#L52-L53 it is supposed to be 127.5 (expected image 255), but it doesnt affect too much;

anyway thanks for both repos (clean-fid and yours) - I like them - it contains unique authentic parts of code

shonenkov commented 9 months ago

execution speed via torch.linalg.eig is much slower torch.linalg.eigh -- so this PR is not good idea to use for training; only to calculate correct final metrics