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

pbnbinom and pgpois are slow for some combinations of parameters #6

Open
twolodzko opened this issue May 5, 2017 · 1 comment
Open

Comments

@twolodzko
Copy link
Owner

pbnbinom and pgpois (and potentially bbbinom) are slow for some combinations of parameters. This is caused by the fact that they are computed as F(x) = sum(f(1:x)) and it is inefficient (no matter of optimizations) for large x or for some combinations of parameters (e.g. for beta negative binomial small alpha and large beta).

@twolodzko twolodzko added this to the ver 1.8.6 milestone May 5, 2017
@twolodzko twolodzko modified the milestone: ver 1.8.6 Jun 19, 2017
@twolodzko
Copy link
Owner Author

twolodzko commented Sep 6, 2017

One solution would be to use different mx values for different combinations of x's and parameters, e.g.

  double mx;
  std::map<std::tuple<int, int, int>, double> maxx;
  
  for (int i = 0; i < Nmax; i++) {
    
    mx = std::min(GETV(x, i), GETV(size, i));
    
    if ( // if not exists
        (maxx.find(std::make_tuple(
            static_cast<int>(i % size.length()),
            static_cast<int>(i % alpha.length()),
            static_cast<int>(i % beta.length()))) == maxx.end())
      || // or is smaller than current max
        (maxx[std::make_tuple(
            static_cast<int>(i % size.length()),
            static_cast<int>(i % alpha.length()),
            static_cast<int>(i % beta.length()))] < mx)
    ) {
      
      maxx[std::make_tuple(
          static_cast<int>(i % size.length()),
          static_cast<int>(i % alpha.length()),
          static_cast<int>(i % beta.length()))] = mx;
      
    }
    
  }

and then

   if (!tmp.size()) {
        mx = maxx.at(std::make_tuple(
          static_cast<int>(i % size.length()),
          static_cast<int>(i % alpha.length()),
          static_cast<int>(i % beta.length())
        ));
        tmp = cdf_bbinom_table(mx, GETV(size, i), GETV(alpha, i), GETV(beta, i));
      }

But on benchmarks this makes the code approx 1.35x slower unless when using vectorization heavily (many different combinations of parameters relatively to sample size).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant