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

use von_mises_lpdf directly instead of von_mises2_lpdf after next stan update #1633

Closed
venpopov opened this issue Mar 23, 2024 · 3 comments
Closed

Comments

@venpopov
Copy link
Contributor

venpopov commented Mar 23, 2024

Currently the von_mises() family is not vectorized if kappa is predicted, because the von_mises_lpdf is numerically unstable for large kappas, and you use the normal approximation for kappa > 100, which requires an if else check.

I have submitted a PR in stan math library that improves the stability of the von_mises_lpdf and which will make the von_mises_lpdf2 unneccessary, as I describe in the issue here.

Once that is accepted and released, you could simplify the von mises family, which my tests show it speeds it up, and also avoids scale=0 warning when 1/sqrt(kappa) passed to normal_lpdf is 0. Some tests described here at the end.

It's probably going to be a while until the next release and also that will work just for the cmdstar backend until rstan catches up, but I'm nothing it here for when the time is right.

@venpopov venpopov changed the title use von_mises_lpdf after next stan update use von_mises_lpdf directly instead of von_mises2_lpdf after next stan update Mar 23, 2024
@of2
Copy link

of2 commented Aug 13, 2024

+1 boosting this - thanks for fixing this issue in Stan @venpopov! Keen for it to be updated in BRMS - after seeing your threads on the Stan forums + the pull request here stan-dev/math#3036 I was able to resolve some hiccups in my models by editing the stan code generated by BRMS to use von_mises_lpdf instead of von_mises2_lpdf

@paul-buerkner
Copy link
Owner

Let's just change it now :-)

@paul-buerkner paul-buerkner added this to the brms 2.22.0 milestone Aug 13, 2024
paul-buerkner added a commit that referenced this issue Aug 13, 2024
@paul-buerkner
Copy link
Owner

Done :-)

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

No branches or pull requests

3 participants