pints-team / pints

Probabilistic Inference on Noisy Time Series
http://pints.readthedocs.io
Other
225 stars 33 forks source link

Change detected: poor performance on banana problem #973

Open ben18785 opened 5 years ago

ben18785 commented 5 years ago

@MichaelClerx thanks for pointing these out!

MichaelClerx commented 5 years ago

Likely to be some issue in the derivatives of the banana problem, or maybe in the code that these methods share.

Now seeing MALA also performs badly on the normal test, which is a bit surprising and coudl be a separate issue?

ben18785 commented 5 years ago

@MichaelClerx Just checked this -- it's because there isn't a gradient method implemented for multivariate Gaussians. I am surprised that we get any data from these runs as think it should just fail. Will create a separate issue to implement this (as should only take a few minutes and we should have it).

@MichaelClerx Is there a way to run the functional tests locally for a given method? Think it would be good to incorporate some degree of functional testing in our workflow of implementing new methods. Not sure how best to do this though...

MichaelClerx commented 5 years ago

@MichaelClerx Just checked this -- it's because there isn't a gradient method implemented for multivariate Gaussians. I am surprised that we get any data from these runs as think it should just fail. Will create a separate issue to implement this (as should only take a few minutes and we should have it).

But the TwistedGaussianLogPDF does have an evaluateS1 method. Where do the multivariate gaussian distributions come in?

ben18785 commented 5 years ago

It’s the prior we’re using - a multivariate Gaussian which doesn’t have sensitivities currently.

On 29 Sep 2019, at 12:38, Michael Clerx notifications@github.com wrote:

@MichaelClerx Just checked this -- it's because there isn't a gradient method implemented for multivariate Gaussians. I am surprised that we get any data from these runs as think it should just fail. Will create a separate issue to implement this (as should only take a few minutes and we should have it).

But the TwistedGaussianLogPDF does have an evaluateS1 method. Where do the multivariate gaussian distributions come in?

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

MichaelClerx commented 5 years ago

I didn't know they had a log prior in there! But had a look and it's only used to generate the initial starting points, so the log prior's sensitivities don't come in to it

I am surprised that we get any data from these runs as think it should just fail.

This was right: calling evaluateS1() on a method that doesn't implement it will cause an exception!

MichaelClerx commented 3 years ago

https://github.com/pints-team/functional-testing/issues/29

Not sure if this goes here or in the FT repo, so leaving open just now