Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in pfNonlinBS? #34

Closed
mdbauer opened this issue Mar 2, 2018 · 5 comments
Closed

Bug in pfNonlinBS? #34

mdbauer opened this issue Mar 2, 2018 · 5 comments

Comments

@mdbauer
Copy link

@mdbauer mdbauer commented Mar 2, 2018

There appears to be a bug in line 73 of pfnonlinbs.cpp. For calculating the standard deviation of the filtering distribution, the auxiliary information passed to the integrand function should be the mean, not the standard deviation. That is, the second resSD(n) in

resSD(n) = sqrt(Sampler.Integrate(integrand_var_x, (void*)&resSD(n)));

should be replaced by resMean(n).

(If I may also note a tiny inconsistency between the default parameters of simNonlin() and the parameters assumed in pfNonlinBS(): for the variance of the initial observation, the simulation uses 10 while the bootstrap filter assumes 4. But this discrepancy is immaterial.)

Thank you for all the amazing work you guys contribute to statistics, finance, and #rstats. I'm a huge fan!

@adamjohansen
Copy link
Collaborator

@adamjohansen adamjohansen commented Mar 2, 2018

Quite so! Thank you for pointing that out... embarrassing but at least easily fixed. I'll get to it over the weekend with any luck.

I can't recall any good reason for the inconsistency that you mention, so perhaps we should tidy that up at the same time.

@adamjohansen adamjohansen mentioned this issue Mar 3, 2018
@tibi77
Copy link
Contributor

@tibi77 tibi77 commented Mar 13, 2018

Why is this not closed?

@eddelbuettel
Copy link
Collaborator

@eddelbuettel eddelbuettel commented Mar 13, 2018

Because we overlooked adding a closes #34 tag. Thanks for noticing!

@mdbauer
Copy link
Author

@mdbauer mdbauer commented Mar 15, 2018

I should note that with this bugfix the figure on Dirk's RcppSMC page looks very different. The current, incorrect figure has confidence intervals that are too wide and always include zero. The new, corrected figure would look something like this:
nonlinbsfilter

@eddelbuettel
Copy link
Collaborator

@eddelbuettel eddelbuettel commented Mar 15, 2018

Thanks for catching that too. I replaced the underlying png file with a new one. Should be better now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.