google-deepmind / torch-distributions

Probability distributions, wrapped for Torch.
BSD 3-Clause "New" or "Revised" License
61 stars 25 forks source link

Pass Cholesky to multivariate gaussian #5

Closed jucor closed 11 years ago

jucor commented 11 years ago

Issue by jucor from Friday Nov 01, 2013 at 18:30 GMT Originally opened as https://github.com/jucor/torch-randomkit/issues/5


jucor commented 11 years ago

Comment by jucor from Saturday Nov 09, 2013 at 08:15 GMT


The more I think of it, the more I want it :-) Either as a separate function, or an optional options table as the last argument. That complicates a bit the argument passing, but that should be doable, since the covariance is never a table.

jucor commented 11 years ago

Comment by d11 from Monday Nov 11, 2013 at 10:36 GMT


Hmm, if we went with the latter, it'd look like the following, right?

multivariateGaussianRand( n | resultTensor, mu, { cholesky = myChol })

another option could be

multivariateGaussianRand( n | resultTensor, mu, cholesky, true )

where the last argument is a flag indicating that the covariance matrix is already decomposed… I'm not sure what I favour for this, really… which is more torch-y? Has anyone given a preference about how to call it?

On 9 Nov 2013, at 08:15, Julien Cornebise notifications@github.com wrote:

The more I think of it, the more I want it :-) Either as a separate function, or an optional options table as the last argument.

— Reply to this email directly or view it on GitHub.

jucor commented 11 years ago

Comment by d11 from Monday Nov 11, 2013 at 10:39 GMT


On second thoughts, the 'flag' option doesn't work so well if we want to add support for passing the precision matrix too.

jucor commented 11 years ago

Comment by jucor from Monday Nov 11, 2013 at 10:39 GMT


I was thinking a hybrid of both:

multivariateGaussianRand( [n | resultTensor,] mu, covOrCholesky [, options])

where options is a table with

jucor commented 11 years ago

Comment by d11 from Monday Nov 11, 2013 at 10:40 GMT


I see - yes, that could work quite well I think.

On 11 November 2013 10:39, Julien Cornebise notifications@github.comwrote:

I was thinking a hybrid of both:

multivariateGaussianRand( [n | resultTensor,] mu, covOrCholesky [, options])

where options is a table with

  • cholesky = true or false (default false) indicates whether we passed a cholesky or a variance
  • and any other setting that we may see fit to extend in the future, e.g. returnCholesky

— Reply to this email directly or view it on GitHubhttps://github.com/jucor/torch-randomkit/issues/5#issuecomment-28188896 .

jucor commented 11 years ago

Comment by jucor from Monday Nov 11, 2013 at 10:41 GMT


Yes, exactly: so we can have

multivariateGaussianRand( [n | resultTensor,] mu, covOrCholeskyOrPrecisionOrCholprecision [, options])

where options would have a flag for cholesky and a flag for precision -- thus allowing to pass the inverse of the Cholesky (although precision and cholesky-precision is more useful for logpdf rather than for the sampling, but at least we'd have a unified interface).

jucor commented 11 years ago

@d11 : done :) But I discovered #10 while doing this one...

d11 commented 11 years ago

Nice! Thanks! We should put a note in the docs, if you didn't already

jucor commented 11 years ago

I'm doing it right now ;-)

On 18 Nov 2013, at 18:05, Dan Horgan notifications@github.com wrote:

Nice! Thanks! We should put a note in the docs, if you didn't already

— Reply to this email directly or view it on GitHub.

jucor commented 11 years ago

And done: http://jucor.github.io/torch-distributions/#toc_13

On 18 Nov 2013, at 18:05, Julien Cornebise julien@deepmind.com wrote:

I'm doing it right now ;-)

On 18 Nov 2013, at 18:05, Dan Horgan notifications@github.com wrote:

Nice! Thanks! We should put a note in the docs, if you didn't already

— Reply to this email directly or view it on GitHub.

d11 commented 11 years ago

Awesome :) On 18 Nov 2013, at 18:09, Julien Cornebise notifications@github.com wrote:

And done: http://jucor.github.io/torch-distributions/#toc_13

On 18 Nov 2013, at 18:05, Julien Cornebise julien@deepmind.com wrote:

I'm doing it right now ;-)

On 18 Nov 2013, at 18:05, Dan Horgan notifications@github.com wrote:

Nice! Thanks! We should put a note in the docs, if you didn't already

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub.