flowgmm icon indicating copy to clipboard operation
flowgmm copied to clipboard

Cov matrix used instead of precision matrix?

Open Parskatt opened this issue 4 years ago • 3 comments

At: https://github.com/izmailovpavel/flowgmm/blob/422ff5dc15c5d2df6ada4787994d5915e11fcd4b/flow_ssl/distributions.py#L22-L25

You seem to insert a precision matrix, but reading the documentation:

https://github.com/pytorch/pytorch/blob/8b248af35d43c97d0e437f6f4ff0fbd4da5700c8/torch/distributions/multivariate_normal.py#L119

It seems like if you do not specify the matrix as a precision matrix, it will be assumed to be a covariance matrix (by argument order).

Is this intended?

Parskatt avatar Nov 26 '20 16:11 Parskatt

Hi @Parskatt, thank you for noticing this! I believe you are correct, and this was not intended. Practically, I believe this does not make a difference, but we need to rename the parameter.

izmailovpavel avatar Dec 03 '20 17:12 izmailovpavel

Yeah, probably it doesn't matter since you initialize inv_std so that the softplus puts it at 1. Maybe its slightly easier to get a singular distribution (i.e. close to zero variance) with the covariance parameterization, don't think it should be too bad though :)

Parskatt avatar Dec 03 '20 18:12 Parskatt

Right :) We are also typically not training these covariance parameters and just keep them fixed at 1. throughout. Definitely a mistake on our side though.

izmailovpavel avatar Dec 03 '20 18:12 izmailovpavel