New Distribution Testing Infastructure

[JacobHass8]

Currently, the new helper function, test_invalid_support, only ensures that a domain error is thrown if the cdf, pdf, logcdf, logpdf or quantile is sampled outside the support of the distribution. I think there are still edge cases that need to be addressed. Here are my following thoughts

• How should the cdf/pdf behave when sampled at +/- infinity? The current testing only checks for distributions with finite support. If the support is infinite should I check that the pdf and cdf evaluate properly at +/- infinity?
• The current function relies on distributions having default arguments. Hopefully this is the case and doesn't get me in trouble.
• I'm unsure how to test incorrect construction of a distribution. I think that the best way would be to define something similar to the support function but here we define a vector of pairs that list each parameter's space.

I've deleted some tests that I previously added for the arcsine distribution just to see if I am getting the same code coverage as before (100%).

Here's the code coverage report.

Here's an ongoing list of distributions that I've added generic testing for

• arcsine (100%)
• bernoulli (100%)
• beta (100%)
• binomial (98.51% - don't know how to hit last 3 lines)
• cauchy (100%)
• exponential (100%)
• fisher (95% - missing 8 lines but looks like a bug in codecov)

[jzmaddock]

Thanks for this,

How should the cdf/pdf behave when sampled at +/- infinity? The current testing only checks for distributions with finite support. If the support is infinite should I check that the pdf and cdf evaluate properly at +/- infinity?

I guess so, and actually we can check the values are correct too I think? PDF should be zero at +-INF and CDF should be zero at -INF and 1 at +INF.

[codecov]

Codecov Report

❌ Patch coverage is 97.53086% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.60%. Comparing base (7a0975e) to head (4ab9190).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
test/test_dist_helpers.hpp	96.87%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1400      +/-   ##
===========================================
- Coverage    95.61%   95.60%   -0.01%     
===========================================
  Files          825      826       +1     
  Lines        68487    68501      +14     
===========================================
+ Hits         65481    65489       +8     
- Misses        3006     3012       +6     

Files with missing lines	Coverage Δ
include/boost/math/distributions/arcsine.hpp	100.00% <100.00%> (ø)
test/test_arcsine.cpp	99.10% <100.00%> (-0.15%)	⬇️
test/test_bernoulli.cpp	100.00% <100.00%> (ø)
test/test_cauchy.cpp	100.00% <100.00%> (ø)
test/test_dist_helpers.hpp	96.87% <96.87%> (ø)

... and 4 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JacobHass8]

I'm trying to write a function to test bad constructors. My idea was to pass in a vector of vectors where each element is a vector of invalid parameters. However, it's not working as I intended and I have no clue why.

I'm first trying to do this for the arcsine distribution. I began by defining a new constructor from a vector

BOOST_MATH_GPU_ENABLED arcsine_distribution(std::vector<RealType> params) : arcsine_distribution(params[0], params[1]) {}

which just unpacks the vector and calls the original constructor. Then in test_arcsine.hpp, I have

std::vector<std::vector<RealType> > invalid_params = {{1, 0}, // x_min > x_max
                                                      {1, -1}}; 
test_invalid_parameters<arcsine_distribution, RealType>(invalid_params);

The function test_invalid_parameters is defined as

template <template <typename...> typename Distribution, class Real>
void test_invalid_parameters(std::vector<std::vector<Real> > invalid_parameters)
{
    typedef Distribution<Real> dist;

    std::vector<Real> params;
    for (unsigned i=0; i<invalid_parameters.size(); i++)
    {
        params = invalid_parameters[i];
        BOOST_CHECK_THROW(dist(params), std::domain_error); // This should throw an error, but doesn't! 
        BOOST_CHECK_THROW(dist x(params), std::domain_error); // This throws a domain_error
        BOOST_CHECK_THROW(boost::math::pdf(dist(params), 0.5), std::domain_error); // This also throws a domain_error
    }
}

I don't understand why dist(params) doesn't throw an error but dist x(params) does. @jzmaddock do you have any thoughts? I'm still new to c++ and this is getting beyond my understanding.

[jzmaddock]

I don't understand why dist(params) doesn't throw an error but dist x(params) does. @jzmaddock do you have any thoughts? I'm still new to c++ and this is getting beyond my understanding.

That's not clear to me either, I always step through the code in situations like this.

BTW I'd rather not start adding lots of new constructors just for testing purposes, although we could conceivably add a std::initializer_list constructor. Even that gets complicated, because a new public constructor will need careful bounds checking and error handling, which would preclude it being a simple forwarding constructor, which sort of defeats the object of the exercise!

OK how about:

template <class Dist, class Container>
Dist make_distribution(const Container& c)
{
    using value_type = typename Dist::value_type;
    if constexpr (std::is_constructible_v<Dist, value_type, value_type, value_type>)
    {
        if (c.size() >= 3)
            return Dist(c.data()[0], c.data()[1], c.data()[2]);
    }
    if constexpr (std::is_constructible_v<Dist, value_type, value_type>)
    {
        if (c.size() >= 2)
            return Dist(c.data()[0], c.data()[1]);
    }
    if constexpr (std::is_constructible_v<Dist, value_type>)
    {
        if (c.size() >= 1)
            return Dist(c.data()[0]);
    }
}

template <class Dist, class V>
Dist make_distribution(const std::initializer_list<V>& c)
{
    return make_distribution<Dist, std::initializer_list<V>>(c);
}
``

You can call it with a vector, but also an initializer_list, so:

make_distribution< binomial_distribution>({ 1.0, 2.0 })

Throws the expected exception.

[JacobHass8]

I don't understand why dist(params) doesn't throw an error but dist x(params) does. @jzmaddock do you have any thoughts? I'm still new to c++ and this is getting beyond my understanding.

That's not clear to me either, I always step through the code in situations like this.

BTW I'd rather not start adding lots of new constructors just for testing purposes, although we could conceivably add a std::initializer_list constructor. Even that gets complicated, because a new public constructor will need careful bounds checking and error handling, which would preclude it being a simple forwarding constructor, which sort of defeats the object of the exercise!

This is exactly what I was looking for! Thanks for implementing this. I was over complicating things.

[JacobHass8]

Is there a way to only run the codecov report when I push a commit? I'm running all the tests and they don't really need to be run on every commit.

[jzmaddock]

Is there a way to only run the codecov report when I push a commit? I'm running all the tests and they don't really need to be run on every commit.

No not really, not unless you temporarily disable the other runs in the github actions scripts: If you do that, make that one commit in it's own right, and then make sure you revert that commit before you're ready to merge.