[Rcpp-devel] Code review for xtensor R bindings

Wolf Vollprecht w.vollprecht at gmail.com
Thu Jun 8 01:06:08 CEST 2017


The idea with IntegerVector for shape is great. I had to fix some minor
stuff in xtensor but it appears to be working.

Ok, good to know about the shape copy!

Yeah, this is literally a weekend project for now (and I haven't used R
before) so this was just a quick-n-dirty test script.

Cheers,

Wolf


2017-06-07 3:59 GMT-07:00 Dirk Eddelbuettel <edd at debian.org>:

>
> 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/d60c742aa8acc764874db87e3c748e
> 27986e1134/
> | 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.r-forge.r-project.org/pipermail/rcpp-devel/attachments/20170607/5c1d07c2/attachment-0001.html>


More information about the Rcpp-devel mailing list