-
Notifications
You must be signed in to change notification settings - Fork 159
Description
Description
While reviewing the Gaussian Ball Walk implementation (gaussian_ball_walk.hpp), I noticed that several implicit assumptions about the input polytope and step-size parameters are not explicitly validated. In certain valid but edge-case scenarios (e.g., low-dimensional or nearly degenerate polytopes, or invalid Gaussian scale parameters), these assumptions can lead to ineffective step sizes and silent stagnation of the Markov chain.
As with other random walks, this issue is not about crashes, but about silent correctness failures, which are particularly problematic for sampling algorithms.
Where this was found
File:
include/random_walks/gaussian_ball_walk.hppIn particular:
- compute_delta()
- constructors handling user-provided parameters
- update_delta()
Observed assumptions and potential failure modes
1. Step-size computation lacks basic validation
return (NT(4) * (P.InnerBall()).second)
/ std::sqrt(std::max(NT(1), a) * NT(P.dimension()));This computation implicitly assumes:
- P.dimension() > 0
- The polytope has a non-zero inner ball
- The Gaussian scale parameter a is valid
However:
- There is no guard for P.dimension() <= 0, which can lead to division by zero.
- The inner ball radius is assumed to be strictly positive; if it is zero or numerically small, the resulting step size becomes ineffective.
- Invalid values of a (e.g., non-positive) are silently clamped via std::max(NT(1), a), hiding invalid input rather than reporting it.
2. User-provided step size is not validated
_delta = params.set_delta ? params.m_L : compute_delta(P, a);- params.m_L is not checked for positivity.
- A zero or negative value can lead to a frozen or undefined walk without any warning.
3. update_delta() allows invalid runtime state
inline void update_delta(NT delta)
{
_delta = delta;
}- There is no validation that the updated step size is strictly positive.
- This allows silent corruption of the walk state during execution.
Impact on users
- Sampling may appear to succeed while producing highly correlated or non-exploratory chains.
- Users may unknowingly obtain misleading samples from Gaussian Ball Walk.
- Low-dimensional and nearly degenerate polytopes are particularly affected.
- The behavior is difficult to diagnose, as no error or warning is raised.
Possible directions for improvement
- Add explicit validation for:
- positive polytope dimension,
- non-zero inner ball radius,
- valid Gaussian scale parameter a. - Reject non-positive user-provided step sizes.
- Document the assumptions under which Gaussian Ball Walk is expected to behave correctly.