Skip to content
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

Unexpected exception thrown by ibeta_inv with extremely small x. #961

Closed
WarrenWeckesser opened this issue Mar 3, 2023 · 3 comments · Fixed by #962
Closed

Unexpected exception thrown by ibeta_inv with extremely small x. #961

WarrenWeckesser opened this issue Mar 3, 2023 · 3 comments · Fixed by #962

Comments

@WarrenWeckesser
Copy link
Contributor

I'm using the develop branch, commit 82ccb85. The compiler:

$ g++ --version
g++ (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0

The code below throws the exception

ibeta_inv_tiny_x: /home/warren/repos/git/forks/boost/math/include/boost/math/special_functions/
detail/ibeta_inverse.hpp:122: T boost::math::detail::temme_method_1_ibeta_inverse(T, T, T, const Policy&)
[with T = long double; Policy = 
boost::math::policies::policy<boost::math::policies::promote_float<false>, 
boost::math::policies::promote_double<false>, boost::math::policies::default_policy, 
boost::math::policies::default_policy, boost::math::policies::default_policy, 
boost::math::policies::default_policy, boost::math::policies::default_policy, 
boost::math::policies::default_policy, boost::math::policies::default_policy, 
boost::math::policies::default_policy, boost::math::policies::default_policy>]: 
Assertion `x >= 0' failed.
Aborted (core dumped)

At line 122 of ibeta_inverse.hpp, the value of x that triggers the exception is -5.42101e-20.

There are comments in the code below that give nearby values of the input x that exhibit different behaviors.

ibeta_inv_tiny_x.cpp:

#include <iostream>
#include <iomanip>
#include <cfenv>
#include <boost/math/special_functions/beta.hpp>

using namespace std;
using namespace boost::math;

void show_fp_exception_flags()
{
    if (std::fetestexcept(FE_DIVBYZERO)) {
        cout << " FE_DIVBYZERO";
    }
    // FE_INEXACT is common and not interesting.
    // if (std::fetestexcept(FE_INEXACT)) {
    //     cout << " FE_INEXACT";
    // }
    if (std::fetestexcept(FE_INVALID)) {
        cout << " FE_INVALID";
    }
    if (std::fetestexcept(FE_OVERFLOW)) {
        cout << " FE_OVERFLOW";
    }
    if (std::fetestexcept(FE_UNDERFLOW)) {
        cout << " FE_UNDERFLOW";
    }
    cout << endl;
}

int main(int argc, char *argv[])
{
    double a = 14.208308325339239;
    double b = a;

    double p = 7.703145458496392e-307;  // Throws exception: Assertion `x >= 0' failed.
    // double p = 7.8e-307;  // No flags set, returns 8.57354094063444939e-23
    // double p = 7.7e-307;  // FE_UNDERFLOW set, returns 0.0

    std::feclearexcept(FE_ALL_EXCEPT);

    double x = ibeta_inv(a, b, p);

    show_fp_exception_flags();

    std::cout << std::scientific << std::setw(24)
              << std::setprecision(17) << x << std::endl;

    return 0;
}

@jzmaddock
Copy link
Collaborator

I don't reproduce with MSVC, but I think I see the issue anyway.

jzmaddock added a commit that referenced this issue Mar 4, 2023
Change assert's in temme_method_1_ibeta_inverse to corrections when guess goes out of range.
Change handling of non-convergence in second_order_root_finder to use bracketing when the end points are many orders of magnitude apart.
Fixes: #961.
@jzmaddock
Copy link
Collaborator

Referenced PR improves things quite a bit.... However, the ibeta is potentially so extreme, that most root finders will fail at some point. The function basically resembles a "square wave" in extreme cases switching from 0 to 1 in the blink of an eye and arbitrarily close to either end point. And for every extreme example, you can always create a worse one!

@WarrenWeckesser
Copy link
Contributor Author

WarrenWeckesser commented Mar 4, 2023

Thanks @jzmaddock. I'll add that I wasn't originally intending to test the function with such a small p and those values of a and b. That happened by accident because of a bug in a test script that I was running to check the wrapper of the function in SciPy. If it was just flaky numerical results, I wouldn't have worried, but the exception crashed Python, and that is something we would like to avoid. 😃

jzmaddock added a commit that referenced this issue Mar 5, 2023
* Fix ibeta_inv for very small p.
Change assert's in temme_method_1_ibeta_inverse to corrections when guess goes out of range.
Change handling of non-convergence in second_order_root_finder to use bracketing when the end points are many orders of magnitude apart.
Fixes: #961.

* Add missing copyright.
[CI SKIP]
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 a pull request may close this issue.

2 participants