pacificclimate / climdex.pcic

Routines to compute ETCCDI Climdex indices
GNU General Public License v3.0
23 stars 13 forks source link

make it resistant to potential const correctness fix in Rcpp. #2

Closed romainfrancois closed 9 years ago

romainfrancois commented 9 years ago

This is related to RcppCore/Rcpp#211

The fix is minimal and just makes sure the code works with both released Rcpp and a version of Rcpp that is more correct regarding const ness of Vector iterators.

I guess a better fix would be to use this signature in c_quantile:

double c_quantile(NumericVector::const_iterator data, const int n, const double quantile, const bool sorted=false)

since you are sending it an iterator to the beginning of a const vector. However you are then using nth_element on the pointer, so you won't respect constness. Then not sure why you use const NumericVector in the first place in this:

RcppExport SEXP c_quantile2(SEXP data_, SEXP quantile_) {
  const NumericVector q(quantile_);
  const NumericVector data(data_);
  const int n = data.size();
  const int nq = q.size();
  NumericVector res(nq);

  for(int i = 0; i < nq; ++i)
    res[i] = c_quantile(const_cast<NumericVector&>(data).begin(), n, q[i]);

  return res;
}

The call res[i] = c_quantile(data.begin(), n, q[i]); works with current Rcpp because it does not respect const ness and gives you a non const pointer where it should really give you a const one.

jameshiebert commented 9 years ago

Cool, that works for me. Thanks for the fix and good luck with https://github.com/RcppCore/Rcpp/pull/211