Laplace: Add mean argument to helper distributions #3210

WardBrian · 2025-06-17T15:16:33Z

Summary

Closes #3202.

Adds a mean argument to the helpers. It is templated such that it can be a vector or scalar.

The poisson_log_2 helper is redundant with this change and is therefore removed.

Additionally, it fixes the y_index argument to be 1-indexed to match the rest of Stan

Tests

Existing tests updated to pass 0 as the mean. It would be nice to add some tests
with a non-zero mean, if there are suggestions (@avehtari @charlesm93)

Side Effects

Release notes

Checklist

Copyright holder: (fill in copyright holder information)

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
- unit tests pass (to run, use: ./runTests.py test/unit)
- header checks pass, (make test-headers)
- dependencies checks pass, (make test-math-dependencies)
- docs build, (make doxygen)
- code passes the built in C++ standards checks (make cpplint)
the code is written in idiomatic C++ and changes are documented in the doxygen
the new changes are tested

avehtari · 2025-06-17T15:45:51Z

This example https://discourse.mc-stan.org/t/embedded-laplace-numerical-problem/39700 uses non-zero mean (mu) and Poisson. So with the helper it should look something like (not certain what is the argument order now)

laplace_marginal_poisson_log_lpmf(y | mu, [1], cov_fun, (sigmaz, N), theta_0)

WardBrian · 2025-06-17T16:03:19Z

Thanks @avehtari.

Writing that test made me realize something else: The y_index argument to laplace_marginal_poisson_log_lpmf is currently 0-indexed rather than 1-indexed. Is that intentional @charlesm93 @SteveBronder?

test/unit/math/laplace/laplace_marginal_poisson_log_lpmf_test.cpp

charlesm93 · 2025-06-24T04:46:18Z

Writing that test made me realize something else: The y_index argument to laplace_marginal_poisson_log_lpmf is currently 0-indexed rather than 1-indexed. Is that intentional @charlesm93 @SteveBronder?

It should be 1-indexed to be consistent with the rest of Stan.

WardBrian · 2025-06-24T20:07:50Z

@charlesm93 y_index is now 1-indexed and I removed the newly redundant poisson_2_log helper

SteveBronder · 2025-07-01T18:28:03Z

stan/math/mix/prob/laplace_marginal_neg_binomial_2_log_lpmf.hpp

-    const ThetaVec& theta_0, CovarFun&& covariance_function,
-    CovarArgs&& covar_args, double tolerance, int max_num_steps,
+    const Mean& mean, CovarFun&& covariance_function, CovarArgs&& covar_args,
+    const ThetaVec& theta_0, double tolerance, int max_num_steps,


@charlesm93 @avehtari so with this change the user will always have to supply a mean. Are there a lot of instances where the mean would be zero? aka do we want a signature without a mean as well?

Re-reading the OP and discussion in #3202, it seems like the consensus is to have the signature with the mean be the only one at the moment. We can add one that allows a scalar (as @charlesm93 requested) to make it easier to pass e.g. 0 instead of a rep_vector(0, N), or a signature without it, but both of those would require some extra work as the compiler isn't currently set up for 'overloads' of the laplace functions as easily

it seems like the consensus is to have the signature without the mean be the only one at the moment.

Sorry @WardBrian I'm confused. Does that mean we should close this PR for now?

That means we should merge this PR... I meant "with"

lol icic got it

WardBrian · 2025-07-02T14:23:07Z

@SteveBronder similar to the last PR, the tests for math are passing, the failure is just because the compiler update needs to be merged simultaneously. So this is ready to review

SteveBronder

Few small things but looks good!

SteveBronder · 2025-07-07T19:32:45Z

stan/math/mix/prob/laplace_latent_bernoulli_logit_rng.hpp

 * \laplace_common_args
 * \rng_arg
 * \msg_arg
 */
-template <typename CovarFun, typename CovarArgs, typename RNG>
+template <typename Mean, typename CovarFun, typename CovarArgs, typename RNG,
+          require_eigen_vector_t<Mean>* = nullptr>


Should we delete this so that it is easier to make into a scalar later?

Suggested change

require_eigen_vector_t<Mean>* = nullptr>

>

SteveBronder · 2025-07-07T19:34:28Z

stan/math/mix/prob/laplace_latent_poisson_log_rng.hpp

 inline Eigen::VectorXd laplace_latent_tol_poisson_log_rng(
    const std::vector<int>& y, const std::vector<int>& y_index,
-    CovarFun&& covariance_function, CovarArgs&& covar_args, ThetaVec&& theta_0,
-    const double tolerance, const int max_num_steps,
+    const Mean& mean, CovarFun&& covariance_function, CovarArgs&& covar_args,


Suggested change

const Mean& mean, CovarFun&& covariance_function, CovarArgs&& covar_args,

Mean&& mean, CovarFun&& covariance_function, CovarArgs&& covar_args,

SteveBronder · 2025-07-07T19:34:42Z

stan/math/mix/prob/laplace_latent_poisson_log_rng.hpp

    const int hessian_block_size, const int solver,
    const int max_steps_line_search, RNG& rng, std::ostream* msgs) {
  laplace_options_user_supplied ops{hessian_block_size,    solver,
                                    max_steps_line_search, tolerance,
                                    max_num_steps,         value_of(theta_0)};
  return laplace_base_rng(poisson_log_likelihood{},
-                          std::forward_as_tuple(y, y_index),
+                          std::forward_as_tuple(y, y_index, mean),


Suggested change

std::forward_as_tuple(y, y_index, mean),

std::forward_as_tuple(y, y_index, std::forward<Mean>(mean)),

This and the forwarding for all the functions

SteveBronder · 2025-07-08T14:32:09Z

test/unit/math/laplace/laplace_poisson_log_rng_test.cpp

@@ -4,12 +4,9 @@

 #include <boost/random/mersenne_twister.hpp>
 #include <boost/math/distributions.hpp>
+#include "Eigen/src/Core/Matrix.h"


Does this need to be here?

No, it was 'helpfully' inserted by autocomplete in vscode...

SteveBronder

One header include needs changed then this is good!

WardBrian · 2025-07-08T14:35:35Z

@SteveBronder stan-dev/stanc3#1523 still needs to be reviewed as well so they can be merged together

WardBrian requested a review from SteveBronder June 17, 2025 15:16

WardBrian mentioned this pull request Jun 17, 2025

Add mean argument to pre-specified laplace likelihoods stan-dev/stanc3#1523

Merged

2 tasks

WardBrian commented Jun 17, 2025

View reviewed changes

test/unit/math/laplace/laplace_marginal_poisson_log_lpmf_test.cpp Outdated Show resolved Hide resolved

WardBrian mentioned this pull request Jun 17, 2025

Release CmdStan 2.37 stan-dev/cmdstan#1324

Open

26 tasks

WardBrian force-pushed the laplace/helpers-mean-argument branch from d3fdfac to 6b19281 Compare July 1, 2025 14:01

SteveBronder reviewed Jul 1, 2025

View reviewed changes

Add mean to laplace helper distributions

0acf4e4

WardBrian force-pushed the laplace/helpers-mean-argument branch from 6b19281 to 0acf4e4 Compare July 1, 2025 18:30

stan-dev deleted a comment from SteveBronder Jul 1, 2025

SteveBronder requested changes Jul 7, 2025

View reviewed changes

Perfect forwarding, template cleanup

ad0e910

WardBrian requested a review from SteveBronder July 7, 2025 20:32

SteveBronder reviewed Jul 8, 2025

View reviewed changes

SteveBronder previously approved these changes Jul 8, 2025

View reviewed changes

Remove extraneous include

3ab644f

WardBrian dismissed SteveBronder’s stale review via 3ab644f July 8, 2025 14:41

SteveBronder approved these changes Jul 8, 2025

View reviewed changes

WardBrian merged commit c8beeaf into develop Jul 9, 2025
33 of 34 checks passed

WardBrian deleted the laplace/helpers-mean-argument branch July 9, 2025 13:33

	const Mean& mean, CovarFun&& covariance_function, CovarArgs&& covar_args,
	Mean&& mean, CovarFun&& covariance_function, CovarArgs&& covar_args,

	std::forward_as_tuple(y, y_index, mean),
	std::forward_as_tuple(y, y_index, std::forward<Mean>(mean)),

Uh oh!

Laplace: Add mean argument to helper distributions #3210

Laplace: Add mean argument to helper distributions #3210

Uh oh!

Conversation

WardBrian commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

Side Effects

Release notes

Checklist

Uh oh!

avehtari commented Jun 17, 2025

Uh oh!

WardBrian commented Jun 17, 2025

Uh oh!

Uh oh!

charlesm93 commented Jun 24, 2025

Uh oh!

WardBrian commented Jun 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WardBrian Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveBronder Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WardBrian Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WardBrian commented Jul 2, 2025

Uh oh!

SteveBronder left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SteveBronder left a comment

Choose a reason for hiding this comment

Uh oh!

WardBrian commented Jul 8, 2025

Uh oh!

Uh oh!

Uh oh!

WardBrian commented Jun 17, 2025 •

edited

Loading

WardBrian Jul 2, 2025 •

edited

Loading

SteveBronder Jul 7, 2025 •

edited

Loading

WardBrian Jul 7, 2025 •

edited

Loading