Is there any optimization function in Rcpp

Viewed 277

The following is my Rcpp code, and I want to minimize the objective function logtpoi(x,theta) respect to theta in R by 'nlminb'. I found it is slow. I have two question:

  1. Anyone can improve my Rcpp code? Thank you very much.
  2. Is there any optimization functions in Rcpp? If yes,maybe I can use them in Rcpp directly. And how to use them? Thank you very much.

My code:

#include <RcppArmadillo.h>
using namespace Rcpp;
using namespace arma;

// [[Rcpp::depends("RcppArmadillo")]] 
// [[Rcpp::export]]
List dtpoi0(const IntegerVector& x, const NumericVector& theta){
//x is 3-dim vector; theta is a 6-dim parameter vector.
//be careful the order of theta1,...,theta6.
  double theta1 = theta[0]; double theta2 = theta[1];
  double theta3 = theta[2]; double theta4 = theta[3];
  double theta5 = theta[4]; double theta6 = theta[5];
  int x1 = x[0]; int x2 = x[1]; int x3 = x[2];
  IntegerVector z1 = IntegerVector::create(x1,x2); 
  IntegerVector z2 = IntegerVector::create(x1,x3);
  IntegerVector z3 = IntegerVector::create(x2,x3); 
  int s1 = min(z1); int s2 = min(z2); int s3 = min(z3);
  arma::imat missy(1,3,fill::zeros); arma::irowvec ijk={0,0,0};
  for (int i = 0; i <= s1; ++i) {
    for (int j = 0; j <= s2; ++j) {
      for (int k = 0; k <= s3; ++k) {
        if ((i+j <= s1) & (i+k <= s2) & ( j+k <= s3))
        { ijk = {i,j,k};
          missy = join_cols(missy,ijk);}
       }
     }
   }
  IntegerMatrix misy = as<IntegerMatrix>(wrap(missy));
  IntegerVector u1 = IntegerVector::create(0);
  IntegerVector u2 = IntegerVector::create(0);
  IntegerVector u3 = IntegerVector::create(0);
  IntegerVector u4 = IntegerVector::create(0);
  IntegerVector u5 = IntegerVector::create(0);
  IntegerVector u6 = IntegerVector::create(0);
  int total = misy.nrow();
  double fvalue = 0; 
  NumericVector part1(1); NumericVector part2(1); 
  NumericVector part3(1); NumericVector part4(1); 
  NumericVector part5(1); NumericVector part6(1);
  for (int l = 1; l < total; ++l) {
    u1 = IntegerVector::create(x1-misy(l,0)-misy(l,1));
    u2 = IntegerVector::create(x2-misy(l,0)-misy(l,2));
    u3 = IntegerVector::create(x3-misy(l,1)-misy(l,2));
    u4 = IntegerVector::create(misy(l,0));
    u5 = IntegerVector::create(misy(l,1));
    u6 = IntegerVector::create(misy(l,2));
    part1 = dpois(u1,theta1);
    part2 = dpois(u2,theta2);
    part3 = dpois(u3,theta3);
    part4 = dpois(u4,theta4);
    part5 = dpois(u5,theta5);
    part6 = dpois(u6,theta6);
    fvalue = fvalue + (part1*part2*part3*part4*part5*part6)[0]; }
   return(List::create(Named("misy") = misy,Named("fvalue") = fvalue)); 
  }
 
 // [[Rcpp::export]]
 NumericVector dtpoi(const IntegerMatrix& x, const NumericVector& theta){ 
//x is n*3 matrix, n is the number of observations.
   int n = x.nrow();
   NumericVector density(n);
   for (int i = 0; i < n; ++i){
     density(i) = dtpoi0(x.row(i),theta)["fvalue"];
   }
   return(density);
 }

// [[Rcpp::export]]
double logtpoi0(const IntegerMatrix& x,const NumericVector theta){
// theta must be a 6-dimiension parameter.
  double nln = -sum(log( dtpoi(x,theta) + 1e-60 ));
  if(arma::is_finite(nln)) {nln = nln;} else {nln = -1e10;} 
  return(nln); 
}

1 Answers

Huge caveat ahead: I don’t really know Armadillo. But I’ve had a stab at it because the code looks interesting.

A few general things:

  1. You don’t need to declare things before you assign them for the first time. In particular, it’s generally not necessary to declare vectors outside a loop if they’re only used inside the loop. This is probably no less efficient than declaring them inside the loop. However, if your code is too slow it makes sense to carefully profile this, and test whether the assumption holds.
  2. Many of your declarations are just aliases for vector elements and don’t seem necessary.
  3. Your z{1…3} vectors aren’t necessary. C++ has a min function to find the minimum of two elements.
  4. dtpoi0 contains two main loops. Both of these have been heavily modified in my code:
    1. The first loop iterates over many ks that can are never used, due to the internal if that tests whether i + j exceeds s2. By pulling this check into the loop condition of j, we perform fewer k loops.
      1. Your if uses & instead of &&. Like in R, using && rather than & causes short-circuiting. While this is probably not more efficient in this case, using && is idiomatic, whereas & causes head-scratching (my code uses and which is an alternative way of spelling && in C++; I prefer its readability).
    2. The second loops effectively performs a matrix operation manually. I feel that there should be a way of expressing this purely with matrix operations — but as mentioned I’m not an Armadillo user. Still, my changes attempt to vectorise as much of this operation as possible (if nothing else this makes the code shorter). The dpois inner product is unfortunately still inside a loop.
  5. The logic of logtpoi0 can be made more idiomatic and (IMHO) more readable by using the conditional operator instead of if.
  6. const-correctness is a big deal in C++, since it weeds out accidental modifications. Use const liberally when declaring variables that are not supposed to change.
  7. In terms of efficiency, the biggest hit when calling dtpoi or logtpoi0 is probably the conversion of missy to misy, which causes allocations and memory copies. Only convert to IntegerMatrix when necessary, i.e. when actually returning that value to R. For that reason, I’ve split dtpoi0 into two parts.
  8. Another inefficiency is the fact that the first loop in dtpoi0 grows a matrix by appending columns. That’s a big no-no. However, rewriting the code to avoid this isn’t trivial.
#include <algorithm>

#include <RcppArmadillo.h>

// [[Rcpp::depends("RcppArmadillo")]]

using namespace Rcpp;
using namespace arma;

imat dtpoi0_mat(const IntegerVector& x) {
    const int s1 = std::min(x[0], x[1]);
    const int s2 = std::min(x[0], x[2]);
    const int s3 = std::min(x[1], x[2]);
    imat missy(1, 3, fill::zeros);

    for (int i = 0; i <= s1; ++i) {
        for (int j = 0; j <= s2 and i + j <= s1; ++j) {
            for (int k = 0; k <= s3 and i + k <= s2 and j + k <= s3; ++k) {
                missy = join_cols(missy, irowvec{i, j, k});
            }
        }
    }

    return missy;
}

double dtpoi0_fvalue(const IntegerVector& x, const NumericVector& theta, imat& missy) {
    double fvalue = 0.0;
    ivec xx = as<ivec>(x);
    missy.each_row([&](irowvec& v) {
        const ivec u(join_cols(xx - v(uvec{0, 0, 1}) - v(uvec{1, 2, 3}), v));
        double prod = 1;
        for (int i = 0; i < u.n_elem; ++i) {
            prod *= R::dpois(u[i], theta[i], 0);
        }
        fvalue += prod;
    });
    return fvalue;
}

double dtpoi0_fvalue(const IntegerVector& x, const NumericVector& theta) {
    imat missy = dtpoi0_mat(x);
    return dtpoi0_fvalue(x, theta, missy);
}

// [[Rcpp::export]]
List dtpoi0(const IntegerVector& x, const NumericVector& theta) {
    imat missy = dtpoi0_mat(x);
    const double fvalue = dtpoi0_fvalue(x, theta, missy);
    return List::create(Named("misy") = as<IntegerMatrix>(wrap(missy)), Named("fvalue") = fvalue);
}

// [[Rcpp::export]]
NumericVector dtpoi(const IntegerMatrix& x, const NumericVector& theta) {
    //x is n*3 matrix, n is the number of observations.
    int n = x.nrow();
    NumericVector density(n);

    for (int i = 0; i < n; ++i){
        density(i) = dtpoi0_fvalue(x.row(i), theta);
    }

    return density;
}

// [[Rcpp::export]]
double logtpoi0(const IntegerMatrix& x, const NumericVector theta) {
    // theta must be a 6-dimension parameter.
    const double nln = -sum(log(dtpoi(x, theta) + 1e-60));
    return is_finite(nln) ? nln : -1e10;
}

Important: This compiles, but I can’t test its correctness. It’s entirely possible (even likely!) that my refactor introduced errors. It should therefore only be viewed as a solution sketch, and should by no means be copied and pasted into an application.

Related