[Rcpp-devel] Code review for xtensor R bindings

[Dirk Eddelbuettel]

On 7 June 2017 at 09:29, Romain Francois wrote:
| Hi, 
| 
| Instead of doing that: 
| 
| const int vtype = traits::r_sexptype_traits<int>::rtype;
| SEXP shape_sxp = Rf_allocVector(vtype, shape.size());
| 
| Perhaps your rxarray object could hold an IntegerVector. Also not quite sure
| why you use the first line at all, which does not depend on T, so vtype is
| always going to be INTSXP. 
| 
| Also, you should avoid using std::cout in https://github.com/QuantStack/
| xtensor-r/blob/master/src/rcpp_hello_xtensor.cpp#L14
| 
| You can use Rcpp::Rcout instead, or Rprintf. 

Yep. And 'install_and_test.R' builds via build() and installs the package
which is muddled; the demo code is better in inst/demo/ or inst/scripts.

For what it is worth I looked into taking the xtensor .deb package and
creating an Ubuntu 16.04 package via the launchpad.net service. I have that
in 'my' PPA (https://launchpad.net/~edd/+archive/ubuntu/misc/+packages) and
will try the same for a 14.04 package at which point we could use Travis
easily. I may create a simple 'more standard R package' PR.

Dirk

|     And the memory is going to be freed again, at some point. And calling
|     Rcpp_ReleaseObject(m_sexp) would also clear the shape_sxp "attribute",
|     right?
| 
| The shape_sxp object you create here (and don’t protect)
| https://github.com/QuantStack/xtensor-r/blob/master/inst/include/xtensor-r/
| rxarray.hpp#L152
| 
| Is probably duplicated here: 
| https://github.com/QuantStack/xtensor-r/blob/master/inst/include/xtensor-r/
| rxarray.hpp#L160
| 
| As part of allocArray’s business: 
| https://github.com/wch/r-source/blob/d60c742aa8acc764874db87e3c748e27986e1134/
| src/main/array.c#L259
| 
| Romain
| 
| 
| 
| 
|     Le 7 juin 2017 à 03:01, Wolf Vollprecht <w.vollprecht at gmail.com> a écrit :
| 
|     Ok, I think I got the idea. 
| 
|     In the R->xtensor bindings, we always work directly on memory allocated
|     through R (either passed in from R or allocated from C++ code).
| 
|     The review-relevant lines are really only ~10 lines: https://github.com/
|     QuantStack/xtensor-r/blob/master/inst/include/xtensor-r/rxarray.hpp#L145
| 
|         const int vtype = traits::r_sexptype_traits<int>::rtype;
|         SEXP shape_sxp = Rf_allocVector(vtype, shape.size());
| 
|         int* r_shape = INTEGER(shape_sxp);
|             // set shape values ... 
|         m_sexp = Rf_allocArray(SXP, shape_sxp);
| 
|     So afterwards, the idea would be to make a call to 
| 
|     Rcpp_PreserveObject(m_sexp);
| 
|     right? And if the destructor of the object is called (and the object is
|     owned from C++) we would just call 
| 
|     Rcpp_ReleaseObject(m_sexp);
| 
|     And the memory is going to be freed again, at some point. And calling
|     Rcpp_ReleaseObject(m_sexp) would also clear the shape_sxp "attribute",
|     right?
| 
|     Thanks for your reply! 
|     [cleardot]
|     Wolf
|    
| 
|     2017-06-06 16:27 GMT-07:00 Dirk Eddelbuettel <edd at debian.org>:
| 
|        
|         On 6 June 2017 at 17:06, Dirk Eddelbuettel wrote:
|         | The general idea when working with R objects is that
|         |
|         |   -- on the way in from R to the C++ functions we construct object
|         such that
|         |      the existing memory (from R) is used; one example is how
|         RcppArmadillo
|         |      uses the 'advanced' constructors of Armadillo allowing to
|         operate
|         |      without copies; hence R managed memory is used while C++
|         functions are
|         |      called; in general this side of the conversion is the templates
|         as<>()
|         |      converters;
|         |
|         |   -- on the way back we use the *alloc functions from the C-level API
|         of R to
|         |      construct objects that use memory from R's pool, once we return
|         these
|         |      objects to R as SEXP they are indistinguishable to R from its
|         own; these
|         |      are the wrap() functions (and I think we may make on occassion
|         make "one
|         |      final copy" at his level, but I'd have to double-check).
| 
|         Actually, let me rephrase: "in most cases not involving native R types"
|         do we
|         make one copy at the return.
| 
|         The general approach is iterator based, see internal/wrap.h. But there
|         is a
|         lot of template magic...
| 
|         Dirk
|        
|         | I haven't had a chance to look at what Wes is doing with Apache
|         Arrow, and
|         | what is happening with xtensor -- so thanks for getting the ball
|         rolling.
|         |
|         | Dirk
|         |
|         | --
|         | http://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org
|         | _______________________________________________
|         | Rcpp-devel mailing list
|         | Rcpp-devel at lists.r-forge.r-project.org
|         | https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/
|         rcpp-devel
| 
|         --
|         http://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org
| 
| 
|     _______________________________________________
|     Rcpp-devel mailing list
|     Rcpp-devel at lists.r-forge.r-project.org
|     https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
| 
| 
| _______________________________________________
| Rcpp-devel mailing list
| Rcpp-devel at lists.r-forge.r-project.org
| https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel

-- 
http://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org