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

Allowing ARMA_64BIT_WORD if required #88

Closed
wants to merge 6 commits into from
Closed

Allowing ARMA_64BIT_WORD if required #88

wants to merge 6 commits into from

Conversation

gvegayon
Copy link
Contributor

@gvegayon gvegayon commented May 3, 2016

Following this discussion http://lists.r-forge.r-project.org/pipermail/rcpp-devel/2016-April/009225.html developers using RcppArmadillo can now pass -DARMA_64BIT_WORD to change the default ARMA_32BIT_WORD. This is useful in case that the user needs to work with objects (matrices/vectors) holding more than 4 billion objects (e.g. Sparse Matrix used as an adjacency matrix for a social network). This will be tested in https://github.com/gvegayon/arma64bit.

@eddelbuettel
Copy link
Member

Hi George -- I generally discourage PRs without prior issue tickets. Here I would have been careful -- I seem to recall that 64BIT_WORD was actually the default per Conrad, but then broke some user code.

Now what you have here in the short commit looks good as it makes the change conditinal, but we will need a full reverse-depends check.

@gvegayon
Copy link
Contributor Author

gvegayon commented May 3, 2016

Makes complete sense. Do you have any url where I can read about 64BIT_WORD breaking some user code? I searched in the issues but I wasn't able to find any.

@eddelbuettel
Copy link
Member

I just looked in the rcpp-logs repo but didn't see anything either.

@gvegayon
Copy link
Contributor Author

gvegayon commented May 4, 2016

Sorry, I've just introduced a small change that I think fixes this issue #89 (first time I fork and PR a repo, so I wasn't aware that it would be introduced right away!). I used this as a reference (batch insertion constructor of SpMat). Further, I think that this patch fixes the issue described in #72 and in this entry of [Rcpp-devel] CRAN submission growfunctions 0.12 as I was experiencing the same problem. To see this a short example

With the previous code

==14527== Memcheck, a memory error detector
==14527== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==14527== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==14527== Command: /usr/lib/R/bin/exec/R --vanilla
==14527== 

R version 3.2.5 (2016-04-14) -- "Very, Very Secure Dishes"
Copyright (C) 2016 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> library(Matrix)
> library(Rcpp)
> 
> code <- '
+ // [[Rcpp::depends(RcppArmadillo)]]
+ #include <RcppArmadillo.h>
+ using namespace Rcpp;
+ 
+ // [[Rcpp::export]]
+ arma::sp_mat iter_sum(const arma::sp_mat & mat, double s) {
+   arma::sp_mat::iterator begin = mat.begin();
+   arma::sp_mat::iterator end   = mat.end();
+ 
+   for (arma::sp_mat::iterator it = begin; it != end; ++it)
+     (*it) += s;
+ 
+   return mat;
+ }
+ '
> 
> sourceCpp(code=code)
> 
> x <- matrix(0, 4, 4)
> diag(x) <- 1:4
> x <- methods::as(x, "dgCMatrix")
> 
> iter_sum(x, -1.5)
==14527== Conditional jump or move depends on uninitialised value(s)
==14527==    at 0x12A2FCD3: operator!= (SpMat_iterators_meat.hpp:244)
==14527==    by 0x12A2FCD3: iter_sum(arma::SpMat<double> const&, double) (file38bf168f33ce.cpp:11)
==14527==    by 0x12A312FB: sourceCpp_0_iter_sum (file38bf168f33ce.cpp:28)
==14527==    by 0x4F0BA37: ??? (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4ACCA: Rf_eval (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4BE56: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4A8AE: Rf_eval (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F71D61: Rf_ReplIteration (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F720B0: ??? (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F72163: run_Rmainloop (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4007EA: main (in /usr/lib/R/bin/exec/R)
==14527==  Uninitialised value was created by a heap allocation
==14527==    at 0x4C2D110: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==14527==    by 0x4C2D227: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==14527==    by 0x12A34473: acquire<unsigned int> (memory.hpp:75)
==14527==    by 0x12A34473: acquire_chunked<unsigned int> (memory.hpp:112)
==14527==    by 0x12A34473: Rcpp::traits::Exporter<arma::SpMat<double> >::get() (RcppArmadilloAs.h:102)
==14527==    by 0x12A312D4: as<arma::SpMat<double> > (as.h:81)
==14527==    by 0x12A312D4: as<arma::SpMat<double> > (as.h:144)
==14527==    by 0x12A312D4: ConstReferenceInputParameter (InputParameter.h:72)
==14527==    by 0x12A312D4: sourceCpp_0_iter_sum (file38bf168f33ce.cpp:26)
==14527==    by 0x4F0BA37: ??? (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4ACCA: Rf_eval (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4BE56: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4A8AE: Rf_eval (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F71D61: Rf_ReplIteration (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F720B0: ??? (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F72163: run_Rmainloop (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4007EA: main (in /usr/lib/R/bin/exec/R)
==14527== 
4 x 4 sparse Matrix of class "dgCMatrix"

[1,] -0.5 .   .   .  
[2,]  .   0.5 .   .  
[3,]  .   .   1.5 .  
[4,]  .   .   .   2.5
> 
> devtools::session_info()
Session info -------------------------------------------------------------------
 setting  value                       
 version  R version 3.2.5 (2016-04-14)
 system   x86_64, linux-gnu           
 ui       X11                         
 language en_US                       
 collate  en_US.UTF-8                 
 tz       <NA>                        
 date     2016-05-03                  

Packages -----------------------------------------------------------------------
 package       * version     date       source        
 devtools        1.11.1      2016-04-21 CRAN (R 3.2.5)
 digest          0.6.9       2016-01-08 CRAN (R 3.2.3)
 lattice         0.20-33     2015-07-14 CRAN (R 3.2.1)
 Matrix        * 1.2-5       2016-04-17 CRAN (R 3.2.5)
 memoise         1.0.0       2016-01-29 CRAN (R 3.2.3)
 Rcpp          * 0.12.4      2016-03-26 CRAN (R 3.2.4)
 RcppArmadillo   0.6.700.3.0 2016-04-06 CRAN (R 3.2.5)
 withr           1.0.1       2016-02-04 CRAN (R 3.2.3)
> 
==14527== 
==14527== HEAP SUMMARY:
==14527==     in use at exit: 102,313,696 bytes in 54,649 blocks
==14527==   total heap usage: 107,986 allocs, 53,337 frees, 207,713,096 bytes allocated
==14527== 
==14527== 1,024 bytes in 1 blocks are definitely lost in loss record 171 of 2,959
==14527==    at 0x4C2D110: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==14527==    by 0x4C2D227: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==14527==    by 0x12A3425D: acquire<unsigned int> (memory.hpp:75)
==14527==    by 0x12A3425D: acquire_chunked<unsigned int> (memory.hpp:112)
==14527==    by 0x12A3425D: init (SpMat_meat.hpp:3964)
==14527==    by 0x12A3425D: SpMat (SpMat_meat.hpp:76)
==14527==    by 0x12A3425D: Rcpp::traits::Exporter<arma::SpMat<double> >::get() (RcppArmadilloAs.h:94)
==14527==    by 0x12A312D4: as<arma::SpMat<double> > (as.h:81)
==14527==    by 0x12A312D4: as<arma::SpMat<double> > (as.h:144)
==14527==    by 0x12A312D4: ConstReferenceInputParameter (InputParameter.h:72)
==14527==    by 0x12A312D4: sourceCpp_0_iter_sum (file38bf168f33ce.cpp:26)
==14527==    by 0x4F0BA37: ??? (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4ACCA: Rf_eval (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4BE56: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4A8AE: Rf_eval (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F71D61: Rf_ReplIteration (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F720B0: ??? (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F72163: run_Rmainloop (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4007EA: main (in /usr/lib/R/bin/exec/R)
==14527== 
==14527== 2,048 bytes in 1 blocks are definitely lost in loss record 1,213 of 2,959
==14527==    at 0x4C2D110: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==14527==    by 0x4C2D227: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==14527==    by 0x12A34231: acquire<double> (memory.hpp:75)
==14527==    by 0x12A34231: acquire_chunked<double> (memory.hpp:112)
==14527==    by 0x12A34231: init (SpMat_meat.hpp:3963)
==14527==    by 0x12A34231: SpMat (SpMat_meat.hpp:76)
==14527==    by 0x12A34231: Rcpp::traits::Exporter<arma::SpMat<double> >::get() (RcppArmadilloAs.h:94)
==14527==    by 0x12A312D4: as<arma::SpMat<double> > (as.h:81)
==14527==    by 0x12A312D4: as<arma::SpMat<double> > (as.h:144)
==14527==    by 0x12A312D4: ConstReferenceInputParameter (InputParameter.h:72)
==14527==    by 0x12A312D4: sourceCpp_0_iter_sum (file38bf168f33ce.cpp:26)
==14527==    by 0x4F0BA37: ??? (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4ACCA: Rf_eval (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4BE56: Rf_applyClosure (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F4A8AE: Rf_eval (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F71D61: Rf_ReplIteration (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F720B0: ??? (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4F72163: run_Rmainloop (in /usr/lib/R/lib/libR.so)
==14527==    by 0x4007EA: main (in /usr/lib/R/bin/exec/R)
==14527== 
==14527== LEAK SUMMARY:
==14527==    definitely lost: 3,072 bytes in 2 blocks
==14527==    indirectly lost: 0 bytes in 0 blocks
==14527==      possibly lost: 0 bytes in 0 blocks
==14527==    still reachable: 102,310,624 bytes in 54,647 blocks
==14527==         suppressed: 0 bytes in 0 blocks
==14527== Reachable blocks (those to which a pointer was found) are not shown.
==14527== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==14527== 
==14527== For counts of detected and suppressed errors, rerun with: -v
==14527== ERROR SUMMARY: 7 errors from 3 contexts (suppressed: 0 from 0)

With the new code

==13653== Memcheck, a memory error detector
==13653== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==13653== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==13653== Command: /usr/lib/R/bin/exec/R --vanilla
==13653== 

R version 3.2.5 (2016-04-14) -- "Very, Very Secure Dishes"
Copyright (C) 2016 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> library(Matrix)
> library(Rcpp)
> 
> code <- '
+ // [[Rcpp::depends(RcppArmadillo)]]
+ #include <RcppArmadillo.h>
+ using namespace Rcpp;
+ 
+ // [[Rcpp::export]]
+ arma::sp_mat iter_sum(const arma::sp_mat & mat, double s) {
+   arma::sp_mat::iterator begin = mat.begin();
+   arma::sp_mat::iterator end   = mat.end();
+ 
+   for (arma::sp_mat::iterator it = begin; it != end; ++it)
+     (*it) += s;
+ 
+   return mat;
+ }
+ '
> 
> sourceCpp(code=code)
> 
> x <- matrix(0, 4, 4)
> diag(x) <- 1:4
> x <- methods::as(x, "dgCMatrix")
> 
> iter_sum(x, -1.5)
4 x 4 sparse Matrix of class "dgCMatrix"

[1,] -0.5 .   .   .  
[2,]  .   0.5 .   .  
[3,]  .   .   1.5 .  
[4,]  .   .   .   2.5
> 
> devtools::session_info()
Session info -------------------------------------------------------------------
 setting  value                       
 version  R version 3.2.5 (2016-04-14)
 system   x86_64, linux-gnu           
 ui       X11                         
 language en_US                       
 collate  en_US.UTF-8                 
 tz       <NA>                        
 date     2016-05-03                  

Packages -----------------------------------------------------------------------
 package       * version     date       source        
 devtools        1.11.1      2016-04-21 CRAN (R 3.2.5)
 digest          0.6.9       2016-01-08 CRAN (R 3.2.3)
 lattice         0.20-33     2015-07-14 CRAN (R 3.2.1)
 Matrix        * 1.2-5       2016-04-17 CRAN (R 3.2.5)
 memoise         1.0.0       2016-01-29 CRAN (R 3.2.3)
 Rcpp          * 0.12.4      2016-03-26 CRAN (R 3.2.4)
 RcppArmadillo   0.6.700.3.0 2016-05-03 local         
 withr           1.0.1       2016-02-04 CRAN (R 3.2.3)
> 
==13653== 
==13653== HEAP SUMMARY:
==13653==     in use at exit: 102,371,593 bytes in 54,645 blocks
==13653==   total heap usage: 107,953 allocs, 53,308 frees, 207,809,518 bytes allocated
==13653== 
==13653== LEAK SUMMARY:
==13653==    definitely lost: 0 bytes in 0 blocks
==13653==    indirectly lost: 0 bytes in 0 blocks
==13653==      possibly lost: 0 bytes in 0 blocks
==13653==    still reachable: 102,371,593 bytes in 54,645 blocks
==13653==         suppressed: 0 bytes in 0 blocks
==13653== Reachable blocks (those to which a pointer was found) are not shown.
==13653== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==13653== 
==13653== For counts of detected and suppressed errors, rerun with: -v
==13653== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

So now there is no memory leak and the Conditional jump... error no longer shows. I've also tested this with the package I'm working on (https://github.com/USCCANA/netdiffuseR) and, after running R CMD check with Valgrind neither the examples nor the tests show memory leak or Conditional jump.... Let me know if you need anything else.

Best,

@eddelbuettel
Copy link
Member

Nice work! Looks like you got the hang of it in no time!

Can you wrap it up as a (small, self-contained, ideally documented in ChangeLog and inst/NEWS.Rd too, maybe with a new unit test) PR?

@gvegayon
Copy link
Contributor Author

gvegayon commented May 4, 2016

Working on it

@gvegayon gvegayon mentioned this pull request May 4, 2016
@gvegayon
Copy link
Contributor Author

gvegayon commented May 4, 2016

This has been solve in #90 and #91

@gvegayon gvegayon closed this May 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants