update laplace api to have hessian_block_size as a non-optional argument#3286
update laplace api to have hessian_block_size as a non-optional argument#3286SteveBronder wants to merge 6 commits intodevelopfrom
Conversation
|
@WardBrian should we just do a shim in stanc where we have both signatures for a commit in main, then merge this, then remove the old signatures |
|
unfortunately because these aren't regular signatures I don't think that is an option, but I'll prepare a branch and we will just have to merge them at the same time again |
|
Do we really want the hessian_block_size argument at the end? I thought @avehtari's suggestion was to put it before the covariance function, since it is the likelihood we're taking the hessian of |
|
Yes, I would group it near the likelihood function |
|
@SteveBronder pr stan-dev/stanc3#1599 is up. It assumes we are following @avehtari's preference and that this argument comes before the covariance functor. |
|
Thanks! I will fix this up today |
|
@WardBrian can you run this against your stanc PR? If that passes then all the tests pass here so we can merge both |
Summary
Based on feedback from @avehtari and @charlesm93, this moves
hessian_block_sizeto be a non-optional argument for the laplace api.This also adds the
generate_laplace_options(Vector)signature so users can passTests
Tests modified for new api but no new tests added.
Side Effects
For stanc3 we will have to update all the signatures for the PR to pass.
Checklist
Copyright holder: Steve Bronder
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)
the basic tests are passing
./runTests.py test/unit)make test-headers)make test-math-dependencies)make doxygen)make cpplint)the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested