Skip to content

bugfix for temporary falling out of scope in poisson_ccdf_log #3203

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

SteveBronder
Copy link
Collaborator

@SteveBronder SteveBronder commented Jun 11, 2025

Summary

I'm still investigating this as it only happens on my work desktop and not my mac.

An error shows up on my linux desktop for several tests

./runTests.py -j4 ./test/prob/poisson/poisson_ccdf_log_00000_generated_v_test
[----------] Global test environment tear-down
[==========] 180 tests from 18 test suites ran. (17 ms total)
[  PASSED  ] 177 tests.
[  FAILED  ] 3 tests, listed below:
[  FAILED  ] AgradCcdfLogPoisson_v_2/AgradCcdfLogTestFixture/0.RepeatAsVector, where TypeParam = std::tuple<AgradCcdfLogPoisson, std::tuple<int, Eigen::Matrix<double, -1, 1, 0, -1, 1>, empty, empty, empty, empty> >
[  FAILED  ] AgradCcdfLogPoisson_v_2/AgradCcdfLogTestFixture/0.UpperBound, where TypeParam = std::tuple<AgradCcdfLogPoisson, std::tuple<int, Eigen::Matrix<double, -1, 1, 0, -1, 1>, empty, empty, empty, empty> >
[  FAILED  ] AgradCcdfLogPoisson_v_2/AgradCcdfLogTestFixture/0.AsScalarsVsAsVector, where TypeParam = std::tuple<AgradCcdfLogPoisson, std::tuple<int, Eigen::Matrix<double, -1, 1, 0, -1, 1>, empty, empty, empty, empty> >

I tracked it down to this line in poisson_ccdf

  const auto& log_Pi = to_ref_if<!is_constant_all<T_rate>::value>(
      log(gamma_p(n_val + 1.0, lambda_val)));
  T_partials_return P = sum(log_Pi);

Removing the to_ref_if there causes the tests to pass. I need to make an MIR showing the effect here, but I'm pretty sure that n_val + 1.0 is falling out of scope after we build up all of our expressions. the to_ref_if's template parameter evaluates to false here so it is returning back an expression on the right hand side. But once the expression is made n_val + 1.0 falls out of scope and so we get a wrong answer.

@t4c1 if you are around to have a look at this it would be great.

Tests

Side Effects

Are there any side effects that we should be aware of?

Release notes

Replace this text with a short note on what will change if this pull request is merged in which case this will be included in the release notes.

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

    • 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

@SteveBronder
Copy link
Collaborator Author

Also odd this was not caught by jenkins when it runs the distribution tests?

@SteveBronder
Copy link
Collaborator Author

Closing this for now

@WardBrian
Copy link
Member

I get the same failures locally (gcc 13.3)

@WardBrian
Copy link
Member

I'm getting another very similar failure with
CXXFLAGS="-fsanitize=address -ggdb3" python ./runTests.py -j4 ./test/prob/loglogistic/loglogistic_cdf_00001_generated_ffv_test

@WardBrian
Copy link
Member

These have been failing since c71dd3e (#2642). The commit before, f40a2d1, is passing

@WardBrian
Copy link
Member

Discussion from #2414 (comment) on to the end of the issue looks very relevant

@WardBrian
Copy link
Member

There are shockingly few places where we use to_ref_if on an expression which is the result of an apply_scalar_binary call, I believe it is only in:

stan/math/prim/prob/loglogistic_cdf.hpp
stan/math/prim/prob/loglogistic_lpdf.hpp
stan/math/prim/prob/pareto_type_2_lcdf.hpp
stan/math/prim/prob/poisson_cdf.hpp
stan/math/prim/prob/poisson_lccdf.hpp
stan/math/prim/prob/poisson_lcdf.hpp
stan/math/prim/prob/weibull_lpdf.hpp

Two of these 7 are failing, it's probably worth checking the others

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.

4 participants