Skip to content

Handle degenerate negative-binomial parameters before sampling - #169

Open
trestoncuzzort wants to merge 2 commits into
boostorg:developfrom
trestoncuzzort:negative-binomial-boundary-cases
Open

trestoncuzzort wants to merge 2 commits into
boostorg:developfrom
trestoncuzzort:negative-binomial-boundary-cases

Conversation

@trestoncuzzort

Copy link
Copy Markdown

Fixes #63.

negative_binomial_distribution(0, 0.5) and (10, 1) satisfy the documented preconditions but abort in the gamma constructor. (0, 0) divides by zero before reaching that constructor.

Return zero directly when the target count is zero or the success probability is one. These cases need no random draws. For a positive target, clarify that the success probability must be strictly positive, and assert that generation precondition before dividing. The interior gamma-Poisson path is unchanged.

Extend the existing tests across signed/unsigned integer and floating representations, direct/parameter construction, parameter setters, explicit-parameter generation and stream round trips. Check both the zero results and unchanged engine state.

Validation on Linux:

  • GCC 16, C++17: the complete B2 test target passes all 123 executables before and after, including its Release multiprecision-float target. Every runtime status/output was checked.
  • GCC C++11 and Clang 23 C++17 ASan/UBSan with float-division/cast checks and leak detection: the affected executable passes all nine cases and 1,607 assertions. Each new boundary case fails against the original header.
  • 387,072 interior samples and 1,512 complete engine states match the frozen original implementation byte for byte under UBSan. This checks sequence preservation, not an independent statistical proof.
  • A separate assertion-handler probe confirms that a positive target with zero probability is caught before division.
  • git diff --check passes. Native Windows/32-bit builds and generated documentation were not tested.

OpenAI Codex assisted with investigation, implementation and tests. dawnr checked the boundary guard's gamma-parameter conditions using an exact rational abstraction and seven provers; that does not prove floating-point sampling or the generic/compiled C++ implementation.

@mborland mborland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Once the CI runs green I will merge. Thank you!

@codecov

codecov Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.21%. Comparing base (15a0968) to head (117e344).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #169      +/-   ##
===========================================
+ Coverage    96.18%   96.21%   +0.02%     
===========================================
  Files          129      130       +1     
  Lines         8446     8504      +58     
===========================================
+ Hits          8124     8182      +58     
  Misses         322      322              
Files with missing lines Coverage Δ
...de/boost/random/negative_binomial_distribution.hpp 100.00% <100.00%> (ø)
test/test_negative_binomial_distribution.cpp 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15a0968...117e344. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Treston Cuzzort and others added 2 commits November 3, 2026 13:44
Return zero without using the engine when k is zero or p is one.
Clarify that positive k requires p > 0 and assert that precondition
before computing the gamma scale. Preserve the interior sampling path.

Cover the boundary cases through construction, parameter changes,
explicit parameters and streaming for several integer and real types.

Generated-by: OpenAI Codex
@mborland
mborland force-pushed the negative-binomial-boundary-cases branch from b1241dd to 117e344 Compare November 3, 2026 18:52
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.

negative_binomial_distribution operator() doesn't properly handle special cases k == 0, p == 0, p == 1

2 participants