Skip to content

update options for laplace#932

Open
SteveBronder wants to merge 1 commit intoissue/873-embeddedLaplacefrom
fix/laplace-1
Open

update options for laplace#932
SteveBronder wants to merge 1 commit intoissue/873-embeddedLaplacefrom
fix/laplace-1

Conversation

@SteveBronder
Copy link
Contributor

Summary

A few small changes for the laplace docs. We use a tuple for the options now

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): Simons Foundation

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

@avehtari
Copy link
Member

I commented this in Stan gathering, but adding here for the record. I think it would be good to have hessian_block_size as main level argument in both laplace_marginal and laplace_marginal_tol. It is essential part of the likelihood Hessian computation, and if user forgets to set it when it needs to be larger than 1, user gets wrong results without any warning. Having it as explicit argument near likelihood function, would mean the user would need to at least explicitly set it in order to be able to use Laplace method. This would also reduce the need to use laplace_marginal_tol.

@avehtari
Copy link
Member

in which branch I can find generate_laplace_options? I did run make stan-update in cmdstan, but stanc still doesn't know about the new function

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants