search

rcannood / princurve

Fits a Principal Curve in Arbitrary Dimension ⤵
41 stars 6 forks source link

Should avoid changing variable `s` in the R global environment via `Rcpp::clone()` ? #33

Closed szcf-weiya closed 4 years ago

szcf-weiya commented 4 years ago

After fitting the principal curve,

x <- runif(100,-1,1)
x <- cbind(x, x ^ 2 + rnorm(100, sd = 0.1))
fit <- principal_curve(x)

the s would look like

> fit$s[fit$ord,]
                 x           
  [1,] -1.01316597 1.06750623
  [2,] -0.99012699 1.01817974
...
 [99,]  0.87941666 0.76858998
[100,]  0.95491256 0.86461089

then I want to get the location for new data point [0, 1] via

project_to_curve(matrix(c(2, 1),1, 2), s)

but then I found s changed

> s
                 x           
  [1,] -1.05924393 1.16615921
  [2,] -0.99012699 1.01817974
...
 [99,]  0.87941666 0.76858998
[100,]  1.10590436 1.05665272

I checked your program, it should be due to https://github.com/rcannood/princurve/blob/1dd2979c5d8f675a11fde9efbe001294f6772841/src/project_to_curve.cpp#L59-L60 and I also try to find if this is a feature (or say requirement) for your other code, such as https://github.com/rcannood/princurve/blob/1dd2979c5d8f675a11fde9efbe001294f6772841/R/principal_curve.R#L195 but it seems that you do not access the previous s.

I am wondering if it is necessary to avoid such a phenomenon if you do not rely on it. At first, I am quite confused. Based on my experience, R cannot modify the elements of a vector/matrix after reassignment, I mean, for the following toy example,

a = c(0, 1)
b = a
b[1] = 1

a is still c(0, 1), so it does not need copy function like in other programming languages, and hence it does not have such functions (right?). For the changed s, I guess it is due to Rcpp, and indeed I found an explanation http://lists.r-forge.r-project.org/pipermail/rcpp-devel/2014-February/007081.html, so if possible, Rcpp::clone(s) can avoid this issue.

Can we change it? Or am I missing something?

rcannood commented 4 years ago

Hello szcf-weiya! This was indeed not intentional. I fixed it in the commit above :+1: Thanks a lot!

rcannood commented 4 years ago

I won't be able to publish the changes on CRAN until Aug 24th:

Error: Submission failed: The submission team is on vacation from Aug 14, 2020 to Aug 24, 2020. During this time the submission of packages is not possible. Sorry for any inconvenience.

rcannood commented 4 years ago

Thanks again, @szcf-weiya! princurve 2.1.5 is on its way to CRAN.