Repository navigation
Conversation
…vector parameters
avehtari
marked this pull request as draft
November 1, 2026 15:47
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #56. #3408 fixes the remaining part,
poisson_lccdf.Merge order: please merge this PR after #3408, #3409, #3489, #3490, #3433, #3434, #3437, #3441, #3445 and #3470. Seven of them change 16 of the 18 headers in this PR. #3490 changes the four
neg_binomialcdf headers, and #3433 changesbinomial_lccdfandneg_binomial_lccdf. #3434 changesbeta_binomial_lccdfand thebeta_neg_binomialandyule_simoncdf headers. #3437 and #3445 also changebeta_binomial_lccdf, and #3441 also changes the threebeta_neg_binomialheaders. #3408, #3409 and #3489 change none of these headers. #3490 and #3433 are stacked on #3409 and #3489. #3470 removes unused#includelines from 14 of the 18 headers, so its conflicts are in the include blocks. One of these includes must stay: #3470 removesselect.hppfrombernoulli_lccdf.hpp, becausedevelopdoes not useselectthere, but this PR does. I keep this PR as a draft until those PRs are merged. Then I mergedevelopinto this branch, resolve the conflicts and run the tests again.This PR was Claude-assisted.
The scripts behind the numbers in this text are in https://github.com/avehtari/stan-pr-checks/tree/6abd778ab2f5ab0f55f3a68b34015af34f1da285/pr3496. Its
README.mdquotes the claims of this text in sections 1 to 8, with the commands and the expected output. The quoted values come from probes built with g++ 11.4.0.Summary
For a vector argument, a
cdfreturns the product of the element probabilities, and anlcdforlccdfreturns the sum of their logs. The functions in the table below had one or both of two defects. One element with probability 1 made the whole result 1 (cdf) or 0 (lcdf,lccdf), and the function ignored the other elements. Example:neg_binomial_lccdf({3, -1} | 2.5, 1.5)returned 0, but the correct value isneg_binomial_lccdf(3 | 2.5, 1.5) = -2.04. An element with probability 0 gave the correct-inf. The gradient was not 0, because the function returned after it had set the partials of earlier elements. In the table,INT_MAXisstd::numeric_limits<int>::max(). Ondevelop, the discrete cdf functions already treat a countn = INT_MAXas an infinite count. For such an element, thecdfis 1, thelcdfis 0 and thelccdfis-inf. This PR keeps that convention.bernoulli_lccdf(CPU and OpenCL)n < 0binomial_lccdfn < 0binomial_lccdfn >= N-inf, gradient not 0-inf, gradient 0beta_binomial_lccdfn < 0beta_binomial_lccdfn >= N-inf, gradient not 0-inf, gradient 0neg_binomial_cdf,neg_binomial_2_cdfn = INT_MAXneg_binomial_lcdfn = INT_MAXneg_binomial_2_lcdfn = INT_MAXneg_binomial_lccdf(also used byneg_binomial_2_lccdf)n < 0neg_binomial_lccdfn = INT_MAX-inf, gradient not 0-inf, gradient 0beta_neg_binomial_cdf,beta_neg_binomial_lcdfn = INT_MAXbeta_neg_binomial_lccdfn < 0beta_neg_binomial_lccdfn = INT_MAX-inf, gradient not 0-inf, gradient 0yule_simon_cdf,yule_simon_lcdfn = INT_MAXyule_simon_lccdfn < 1poisson_binomial_lccdfy = N-infwith gradient not 0-inf, gradient 0poisson_binomial_cdfandpoisson_binomial_lcdfnow also skip an element withy = N. Before, such an element gave a factor 1 (cdf) or a term 0 (lcdf), with a rounding error of about 1e-16.All functions in the table now use one rule:
-infbefore it sets any partial.Three functions needed more than the rule:
neg_binomial_2_lcdfcallsbeta_lcdfwith the second shapen + 1. Forn = INT_MAXthat shape is2^31, and the gradient throughbeta_lcdfis not accurate there. Forneg_binomial_2_lcdf({3, INT_MAX} | 4, 2)ondevelop, thephiderivative is 19.41; the correct value, thephiderivative ofneg_binomial_2_lcdf(3 | 4, 2), is -0.0604. The function now removes the elements withn = INT_MAXbefore the call. Large finite counts still go throughbeta_lcdf, and this PR does not change that path. There thephiderivative is also wrong. Atmu = 4,phi = 2, it is 11.8 forn = 10^6and 18.7 forn = 10^9. The correct derivative is below1e-176000. At the head of Evaluate the incomplete beta complement without cancellation in the lccdf and Student t cdf functions #3433, which is stacked on More stable incomplete beta gradient roots #3409, Reverse-mode grad_reg_inc_beta (part 2 of #3409) #3489 and Move the beta, neg_binomial and student_t cdfs to the new grad_reg_inc_beta (part 3 of #3409) #3490, the derivative becomes 0.poisson_binomial_lccdfaty = Nreturnedlog1m_expof a rounded sum near 0. Fortheta = (0.5, 0.2, 0.7)andy = 3it returned -36.74, which islog(2^-53). The existing testprim/prob/poisson_binomial_ccdf_log_testexpected that value. This PR changes the expectation to-inf, as the othery = Ncases in that file expect.neg_binomial_lcdfcomputesdigamma(n + 1)for each element before the main loop. Withn = INT_MAX,n + 1overflowsint(UBSan reports it ondevelop); this PR usesn + 1.0.The PR does not change the discrete cdf functions that already followed the rule:
bernoulli_cdfand_lcdf,binomial_cdfand_lcdf,beta_binomial_cdfand_lcdf, anddiscrete_range.Testing
test/unit/math/mix/prob/discrete_cdfs_edge_elements_test.cppcovers the cdf, lcdf and lccdf functions of the 8 families. An edge element is an element with probability 0 or 1. For an edge element with probability 1, the test comparesf({ordinary, edge})andf({edge, ordinary})withf({ordinary}). It compares value and gradient, in reverse and forward mode. For an edge element with probability 0, it checks the value 0 or-infand a zero gradient. Other cases use vector parameters, a scalar edge count with vector parameters, or (forpoisson_binomial) a separatethetavector per element. With vector parameters, the gradient with respect to the parameters of the edge element must be exactly 0.expect_adchecks the newneg_binomial_2_lcdfcode with scalar and vectormuandphi. All 11 test cases pass with the change and fail with thedevelopheaders (503 failed expectations). For this test, I replaced all 17 changedprimheaders with theirdevelopversions at once.opencl_matches_cpu_small_n_negativeofbernoulli_lccdf(counts{0, 0, -5}) compares OpenCL with the fixed CPU function. It passes on a V100. With thedevelopOpenCL file and the fixed CPU file, it fails. Two new OpenCL tests check broadcasting inbernoulli_lccdf: one with a scalarn = -1, one with a scalarthetaandn = {0, -1, 0}. They compare OpenCL with OpenCL and also pass ondevelop.The new test and 35 existing unit suites of the 8 families (
prim,rev,mix) pass. The generatedtest/probtests of the 7 families that have them (all exceptpoisson_binomial) pass. cpplint,make test-math-dependencies, a compile check of each changed header,make test-headersand clang-format 10 pass.make doxygenwith doxygen 1.9.7 gives no warnings. I ran the tests with the change ondevelopatdc66941969, with g++ 14.3.0 (CPU) and a V100 (OpenCL).Release notes
Discrete
cdf,lcdfandlccdffunctions now skip a vector element with probability 1; before, such an element made the result 1 or 0. An element with probability 0 now gives 0 or-infwith a zero gradient.Checklist
Copyright holder: Aalto University
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