stan-dev
Fix weibull lcdf
Fix for #2810.
Release notes
Makes the Weibull cdf/lcdf more stable by using log1m_exp function.
Checklist
-
[x] Math issue #2810
-
[x] Copyright holder: Sean Pinkney
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/)
-
[x] 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)
- unit tests pass (to run, use:
-
[x] the code is written in idiomatic C++ and changes are documented in the doxygen
-
[x] the new changes are tested
@rok-cesnovar I don't know how to fix the expression tests for changes up to commit 9c1f7ace7ba39b9f41c3832c84e75565f253173a (2nd to last commit). Though the changes are minimal. For the final commit, I ended up following the neg_binomial_2_l/cdf files which meant overhauling the files. In my opinion, the file is more readable but would like you or others to take a look. There aren't great tests for these functions so my plan is to add them as well.
@nhuurre for y values of 0 I have the function returning 0 for the cdf and negative infinity for the lcdf. I believe the cdf is right. I'm wondering if the lcdf is correct or if it should also return 0.
Negative infinity for the lcdf is correct. The derivatives should be well-defined (and zero) when alpha > 1.
Negative infinity for the lcdf is correct. The derivatives should be well-defined (and zero) when
alpha > 1.
Shoot, I was hoping it was for all alpha because this raises a question about what we do with the pow() function. In the case of the model in the discourse post I get the same answer when I return a 0 for all alpha as compared to the UDF defined in the same post. When I limit the implementation here to alpha > 1 I get divergences and a different answer, unsurprisingly. The question then is, do we update pow or do I leave it for all alpha here?
CDF derivative with respect to alpha is always zero. The derivatives with respect y and sigma are zero only if alpha > 1.
The derivative of LCDF is
$$ \frac{1}{e^{\left(\frac{y}{\sigma}\right)^{\alpha}}-1}\left(\frac{y}{\sigma}\right)^{\alpha}\log\left(\frac{y}{\sigma}\right)\approx \log\left(\frac{y}{\sigma}\right) $$
which goes to negative infinity at y=0—but realistically the only way to get a finite log-prob at the end is to put the LCDF value through a log_sum_exp/log_diff_exp or the equivalent and those are going to squash any finite derivative to zero. But unfortunately infinite derivatives probably still break even when they shouldn't. We could either just say the derivative here is zero (and trust that any situation where that leads to incorrect result is going to be rejected anyway due to a target=-inf issue or adjust log_sum_exp to ignore the derivatives of -inf terms.
@rok-cesnovar I don't know how to update the opencl version of weibull cdf. It needs to return 0 for the gradients when y is 0 and I'm getting header check errors when I try to update.
