Improve efficiency of beta_distribution::operator()

[mborland]

@Hailios since gamma_distribution::operator() can be marked const we can in fact avoid re-initializing at least 2 gamma distributions for each call to beta_distribution::operator().

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.85%. Comparing base (3289126) to head (093554f).
Report is 3 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop     #135   +/-   ##
========================================
  Coverage    95.85%   95.85%           
========================================
  Files          115      115           
  Lines         8126     8128    +2     
========================================
+ Hits          7789     7791    +2     
  Misses         337      337           

Files with missing lines	Coverage Δ
include/boost/random/beta_distribution.hpp	100.00% <100.00%> (ø)
include/boost/random/gamma_distribution.hpp	100.00% <100.00%> (ø)

---

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 3289126...093554f. Read the comment docs.

🚀 New features to boost your workflow:

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

[Hailios]

Thanks for looking in to this. What does static bring here?

[mborland]

Thanks for looking in to this. What does static bring here?

Since the alpha_dist and beta_dist are both constants we can mark them as static const so they're both only initialized once rather than at every function call.

[Hailios]

what I mean is that static makes it initialize the gamma distributions truly once, not once per call to the beta dist, or once per beta dist parameters. that is, when the next beta dist arrives, with new parameters, it will get the gamma distributions with the wrong parameters.

here is a small reproducer that showcase the issue. I added prints in the beta dist, see below.
the expected prints would be
1 time "alpha 0.500000,beta 1.000000"
followed by 100 times "alpha 0.100000,beta 1.000000"

but instead all lines become "alpha 0.500000,beta 1.000000"

TEST(test, reprod2) {
  boost::random::beta_distribution<double> dist(0.1, 0.1);
  boost::random::beta_distribution<double> dist2(0.5, 0.5);
  std::mt19937_64 gen(12345);

  {
    double Z = dist2(gen);
    EXPECT_FALSE(std::isnan(Z));
  }

  for (int i = 0; i < 100; ++i) {
    double Z = dist(gen);
    EXPECT_FALSE(std::isnan(Z));
  }
}

and in the beta_distribution.hpp, I added prints just to show that the params are wrong

    /**
     * Returns a random variate distributed according to the
     * beta distribution.
     */
    template<class URNG>
    RealType operator()(URNG& urng) const
    {
        static const auto alpha_dist = gamma_distribution<RealType>(_alpha, RealType(1.0));
        static const auto beta_dist = gamma_distribution<RealType>(_beta, RealType(1.0));

        RealType a = 0;
        RealType b = 0;

        printf("alpha %f,", alpha_dist.alpha());
        printf("beta %f\n", beta_dist.beta());

        do
        {
            a = alpha_dist(urng);
            b = beta_dist(urng);
        } while (a + b == RealType(0));

        return a / (a + b);
    }

removing static makes it work again, and avoids calling .init() on each loop iteration.

thanks for looking into this.

[mborland]

Oh you're right. I only thought about and plotted the single beta_distribution usage. Will fix promptly.

[mborland]

The static has been removed, and all diffs are on master so you'll see them in the 1.89 release in the next few weeks. Thanks again.