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

RcppArmadillo sample with different probabilities #275

Closed
emanuelegdepaoli opened this issue Nov 13, 2019 · 3 comments
Closed

RcppArmadillo sample with different probabilities #275

emanuelegdepaoli opened this issue Nov 13, 2019 · 3 comments

Comments

@emanuelegdepaoli
Copy link

@emanuelegdepaoli emanuelegdepaoli commented Nov 13, 2019

I've found a problem with RcppArmadillo sample function when the sampling probabilities are not constant.
The code is:

#include <RcppArmadillo.h>
#include <RcppArmadilloExtensions/sample.h>
// [[Rcpp::depends(RcppArmadillo)]]

using namespace Rcpp;
using namespace arma;
using namespace std;

// [[Rcpp::export]]
uvec arma_sample(int n, int k, vec prob){
  uvec out(n, fill::zeros);
  vec comp = regspace(1,  k);
  prob.print();
  for(int i = 0; i < n; ++i){
    out(i) = Rcpp::RcppArmadillo::sample(comp, 1, false, prob)[0];
  }
  prob.print();
  return out;
}

and the output when the function is called in R:

> res = arma_sample(10^4, 2, c(0.1,0.9))
   0.1000
   0.9000
   0.5000
   0.5000
> prop.table(table(res))
res
     1      2 
0.4934 0.5066 `

Apparently the function is sampling from a discrete distribution with uniform probabilities.

@eddelbuettel

This comment has been minimized.

Copy link
Member

@eddelbuettel eddelbuettel commented Nov 13, 2019

@helmingstay Are you around and able to take a look?

helmingstay added a commit to helmingstay/RcppArmadillo that referenced this issue Nov 14, 2019
* Fix issue RcppCore#275: const input prob (workers modify copy)
@helmingstay

This comment has been minimized.

Copy link
Contributor

@helmingstay helmingstay commented Nov 14, 2019

Ouch. Thanks for @emanuelegdepaoli for the lovely bug report.
To clarify, the first call to sample() works as intended, but the input arma::vector prob was clobbered by the worker functions called by sample_main, e.g., ProbSampleReplace.

The input prob vector is now passed as const and copied inside sample_main.

@emanuelegdepaoli

This comment has been minimized.

Copy link
Author

@emanuelegdepaoli emanuelegdepaoli commented Nov 14, 2019

Thank you @helmingstay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.