Skip to content

[Rcpp-devel] Broken conversion from R-type integer to uvec after update to newer Rcpp version

12 messages · Venelin Mitov, Dirk Eddelbuettel, Kevin Ushey

#
Hello,

I have a dynamic library with C++ functions built using Rcpp. I've recently
updated to the newest version of R/Rcpp/RcppArmadillo and after rebuilding
my library it lost its expected behavior. After some debugging I found that
one of possibly several problems is the following:

I have an exported cpp function with the following signature:
uvec shift(const uvec& idx, int dshift, int n, int i=1);

In my R-code I call this function as follows:
shifted_idx <- shift(1:10, 5, 10, 1).
This is a detail, but let me mension that all this function does is to
rotate the vector of indices idx left by 5 positions, so the result should
be the vector: c(6,7,8,9,10,1,2,3,4,5).

This is the wrapper for the same function generated with the older
Rcpp-version:
RcppExport SEXP sourceCpp_28368_shift(SEXP idxSEXP, SEXP dshiftSEXP, SEXP
nSEXP, SEXP iSEXP) {
BEGIN_RCPP
    SEXP __sexp_result;
    {
        Rcpp::RNGScope __rngScope;
        *uvec idx = Rcpp::as<uvec >(idxSEXP);*
        int dshift = Rcpp::as<int >(dshiftSEXP);
        int n = Rcpp::as<int >(nSEXP);
        int i = Rcpp::as<int >(iSEXP);
        uvec __result = shift(idx, dshift, n, i);
        PROTECT(__sexp_result = Rcpp::wrap(__result));
    }
    UNPROTECT(1);
    return __sexp_result;
END_RCPP
}

This is the wrapper for this function generated with the newer Rcpp-version:
RcppExport SEXP sourceCpp_85322_shift(SEXP idxSEXP, SEXP dshiftSEXP, SEXP
nSEXP, SEXP iSEXP) {
BEGIN_RCPP
    SEXP __sexp_result;
    {
        Rcpp::RNGScope __rngScope;
        *Rcpp::traits::input_parameter< const uvec& >::type idx(idxSEXP );*
        Rcpp::traits::input_parameter< int >::type dshift(dshiftSEXP );
        Rcpp::traits::input_parameter< int >::type n(nSEXP );
        Rcpp::traits::input_parameter< int >::type i(iSEXP );
        uvec __result = shift(idx, dshift, n, i);
        PROTECT(__sexp_result = Rcpp::wrap(__result));
    }
    UNPROTECT(1);
    return __sexp_result;
END_RCPP
}

The result is that the converted array idx in the newer version has
apparently different values than the the passed argument which is simply
1:10. If, for debugging purpose, I modify the function to simply return its
first argument this is the value I'm getting:
[1,]          0
 [2,] 1072693248
 [3,]          0
 [4,] 1073741824
 [5,]          0
 [6,] 1074266112
 [7,]          0
 [8,] 1074790400
 [9,]          0
[10,] 1075052544

I'm running this on a 64 bit computer and I could reproduce it on
Ubuntu as well.

I guess that this behaviour is wrong.

To check versions, here is the output of sessionInfo():

R version 3.0.1 (2013-05-16)
Platform: x86_64-w64-mingw32/x64 (64-bit)

locale:
[1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United
States.1252
[3] LC_MONETARY=English_United States.1252 LC_NUMERIC=C
[5] LC_TIME=English_United States.1252

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] Rcpp_0.11.1

loaded via a namespace (and not attached):
[1] RcppArmadillo_0.4.200.0 tools_3.0.1


Best,

Venelin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.r-forge.r-project.org/pipermail/rcpp-devel/attachments/20140420/7d3c2870/attachment.html>
#
Hi Venelin,
On 20 April 2014 at 15:18, Venelin Mitov wrote:
| Hello,
| 
| I have a dynamic library with C++ functions built using Rcpp. I've recently
| updated to the newest version of R/Rcpp/RcppArmadillo and after rebuilding my
| library it lost its expected behavior. After some debugging I found that one of
| possibly several problems is the following:
| 
| I have an exported cpp function with the following signature:
| uvec shift(const uvec& idx, int dshift, int n, int i=1);
| 
| In my R-code I call this function as follows:
| shifted_idx <- shift(1:10, 5, 10, 1).
| This is a detail, but let me mension that all this function does is to rotate
| the vector of indices idx left by 5 positions, so the result should be the
| vector: c(6,7,8,9,10,1,2,3,4,5).
| 
| This is the wrapper for the same function generated with the older
| Rcpp-version:
| RcppExport SEXP sourceCpp_28368_shift(SEXP idxSEXP, SEXP dshiftSEXP, SEXP
| nSEXP, SEXP iSEXP) {
| BEGIN_RCPP
| ??? SEXP __sexp_result;
| ??? {
| ??????? Rcpp::RNGScope __rngScope;
| ??????? uvec idx = Rcpp::as<uvec >(idxSEXP);
| ??????? int dshift = Rcpp::as<int >(dshiftSEXP);
| ??????? int n = Rcpp::as<int >(nSEXP);
| ??????? int i = Rcpp::as<int >(iSEXP);
| ??????? uvec __result = shift(idx, dshift, n, i);
| ??????? PROTECT(__sexp_result = Rcpp::wrap(__result));
| ??? }
| ??? UNPROTECT(1);
| ??? return __sexp_result;
| END_RCPP
| }
| 
| This is the wrapper for this function generated with the newer Rcpp-version:
| RcppExport SEXP sourceCpp_85322_shift(SEXP idxSEXP, SEXP dshiftSEXP, SEXP
| nSEXP, SEXP iSEXP) {
| BEGIN_RCPP
| ??? SEXP __sexp_result;
| ??? {
| ??????? Rcpp::RNGScope __rngScope;
| ??????? Rcpp::traits::input_parameter< const uvec& >::type idx(idxSEXP );
| ??????? Rcpp::traits::input_parameter< int >::type dshift(dshiftSEXP );
| ??????? Rcpp::traits::input_parameter< int >::type n(nSEXP );
| ??????? Rcpp::traits::input_parameter< int >::type i(iSEXP );
| ??????? uvec __result = shift(idx, dshift, n, i);
| ??????? PROTECT(__sexp_result = Rcpp::wrap(__result));
| ??? }
| ??? UNPROTECT(1);
| ??? return __sexp_result;
| END_RCPP
| }
| 
| The result is that the converted array idx in the newer version has apparently
| different values than the the passed argument which is simply 1:10. If, for
| debugging purpose, I modify the function to simply return its first argument
| this is the value I'm getting:
| 
| 
| > shift(1:10,5,10,1)
|             [,1]
|  [1,]          0
|  [2,] 1072693248
|  [3,]          0
|  [4,] 1073741824
|  [5,]          0
|  [6,] 1074266112
|  [7,]          0
|  [8,] 1074790400
|  [9,]          0
| [10,] 1075052544
| 
| 
| I'm running this on a 64 bit computer and I could reproduce it on Ubuntu as well.
| 
| I guess that this behaviour is wrong.
| 
| To check versions, here is the output of sessionInfo():
| 
| R version 3.0.1 (2013-05-16)
| Platform: x86_64-w64-mingw32/x64 (64-bit)
| 
| locale:
| [1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252
| [3] LC_MONETARY=English_United States.1252 LC_NUMERIC=C
| [5] LC_TIME=English_United States.1252
| 
| attached base packages:
| [1] stats     graphics  grDevices utils     datasets  methods   base
| 
| other attached packages:
| [1] Rcpp_0.11.1
| 
| loaded via a namespace (and not attached):
| [1] RcppArmadillo_0.4.200.0 tools_3.0.1

It is entirely possible that something is wrong, and while you provided an
appreciable level of detail, you have not provided a small, self-contained,
reproducible piece of code.  

That would help.

Below are two conversions from Armadillo 'uvec' and 'ivec'.  They work for me.


R> cppFunction("SEXP uvec(int n) { arma::uvec v = arma::conv_to<arma::uvec>::from(arma::ones(n)); return(wrap(v)); }", depends="RcppArmadillo")
R> uvec(3)
     [,1]
[1,]    1
[2,]    1
[3,]    1
R> cppFunction("SEXP ivec(int n) { arma::ivec v = arma::conv_to<arma::ivec>::from(arma::ones(n)); return(wrap(v)); }", depends="RcppArmadillo")
R> ivec(3)
     [,1]
[1,]    1
[2,]    1
[3,]    1
R> 

In both cases I use ones() to generate (double-valued) vector which I cast
using Armadillo's converter function.  The resulting integer, and unsigned
integer, vectors are then converted via wrap().  In both cases do the correct
values appear.

I probably missed a key aspect of your bug report.  Can you provide a simple example?

Thanks!

Dirk

 
| 
| 
| Best,
| 
| Venelin
| 
| 
| ----------------------------------------------------------------------
| _______________________________________________
| 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
#
Hi again,

Here is an example:

my c++ file:
// [[Rcpp::depends(RcppArmadillo)]]
#include <RcppArmadillo.h>
#include <Rcpp.h>

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

// [[Rcpp::export]]
uvec touvec(const uvec& v) {
  return v;
}

Save to "example.cpp" and call the following in R-console:
[1,]          0
 [2,] 1072693248
 [3,]          0
 [4,] 1073741824
 [5,]          0
 [6,] 1074266112
 [7,]          0
 [8,] 1074790400
 [9,]          0
[10,] 1075052544

Best,

Venelin







2014-04-20 15:34 GMT+02:00 Dirk Eddelbuettel <edd at debian.org>:

  
    
#
Venelin,

For completeness, things also work for me on conversion vectors 'in':

R> cppFunction("SEXP uvec2(arma::uvec v) { arma::uvec w = 2*v; return(wrap(w)); }", depends="RcppArmadillo")
R> uvec2(seq(1L, 3L))
     [,1]
[1,]    2
[2,]    4
[3,]    6
R> cppFunction("SEXP ivec2(arma::ivec v) { arma::ivec w = 2*v; return(wrap(w)); }", depends="RcppArmadillo")
R> ivec2(seq(1L, 3L))
     [,1]
[1,]    2
[2,]    4
[3,]    6
R> 

A reproducible example would be good.

Dirk
#
Got it:

R> cppFunction("arma::uvec venelin(const arma::uvec v) { return(v); }", depends="RcppArmadillo")
R> venelin(1:3)
           [,1]
[1,]          0
[2,] 1072693248
[3,]          0
R> cppFunction("arma::uvec venelin2(arma::uvec v) { return(v); }", depends="RcppArmadillo")
R> venelin2(1:3)
     [,1]
[1,]    1
[2,]    2
[3,]    3
R> 

The const seems to throw it off.  We will try to take a look!

Dirk
#
Hi again,

The following code worked:
// [[Rcpp::depends(RcppArmadillo)]]
#include <RcppArmadillo.h>
#include <Rcpp.h>

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

// [[Rcpp::export]]
uvec touvec(*uvec v*) {
  return v;
}

So maybe something goes wrong if i pass const & as an argument :(.

Best,
Venelin


2014-04-20 15:46 GMT+02:00 Venelin Mitov <vmitov at gmail.com>:

  
    
#
On 20 April 2014 at 15:50, Venelin Mitov wrote:
| So maybe something goes wrong if i pass const & as an argument :(.

Yes. For now, please remove the 'const' qualifier. 

I had a first look but don't yet see where this leads the compiler astray. 

Dirk
#
Hi Dirk, 

Had a look at your example (function named venelin). It seems to me that the problem is not only the const qualifier but also the passing of an argument by reference. 

uvec touvec(uvec& v) {    //fails!
   return v;
}

uvec touvec(uvec v) {     // works!
  return v;
}

Don?t know if this helps?.
Best,
Venelin
On 20 Apr 2014, at 16:10, Dirk Eddelbuettel <edd at debian.org> wrote:

            
#
On 20 April 2014 at 20:18, Mitov Venelin wrote:
| Hi Dirk, 
| 
| Had a look at your example (function named venelin). It seems to me that the problem is not only the const qualifier but also the passing of an argument by reference. 
| 
| uvec touvec(uvec& v) {    //fails!
|    return v;
| }
| 
| uvec touvec(uvec v) {     // works!
|   return v;
| }
| 
| Don?t know if this helps?.

It seems the combination of either/or/both const and & AND use of uvec
triggers this.  A base uvec vector seems to pass in fine, as does const ref
use of a normal int vector.

I think we are being tripped up by a partial conversion. We will try to get
to it.  In the meantime, maybe just avoid 'uvec' which has no direct R
counterpart.

Dirk
#
Hi guys,

Here's what I think is going on. RcppArmadillo has the following
'InputParameter' class to handle 'as':

template <typename T, typename VEC, typename REF>
    class ArmaVec_InputParameter {
    public:
        ArmaVec_InputParameter( SEXP x_ ) : v(x_), vec(
reinterpret_cast<T*>( v.begin() ), v.size(), false ){}

        inline operator REF(){
            return vec ;
        }

    private:
        Rcpp::Vector< Rcpp::traits::r_sexptype_traits<T>::rtype > v ;
        VEC vec ;
    } ;

We are trying to re-use memory from an R vector, and here we are just
reinterpreting the 'int' contents of the R vector as 'unsigned int' without
any coercion. This won't work.

(Interestingly, in the 'base' case (ie, arma::uvec vs. const or
ref-qualified arma::uvec), we are just dispatching to an internal Rcpp as,
not any of the RcppArmadillo 'as'es. That's why we don't see it here --
that dispatch explicitly uses 'as')

To fix this, we need to coerce when necessary. I believe the
r_sexptype_traits class has a bit of 'does this fit in an R vector, or does
it need coercion?' in it, so maybe we can leverage that.

I have a fix that I will submit to RcppArmadillo as a pull request in a few
moments.

Thanks for the bug report!

Kevin
On Sun, Apr 20, 2014 at 11:36 AM, Dirk Eddelbuettel <edd at debian.org> wrote:

            
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.r-forge.r-project.org/pipermail/rcpp-devel/attachments/20140420/2268e90d/attachment.html>
#
Hi Mitov,

I've pushed some fixes to RcppArmadillo:
https://github.com/RcppCore/RcppArmadillo

Can you try installing the development version
(devtools::install_github("RcppCore/RcppArmadillo")) and confirm everything
works as expected?

Thanks,
Kevin
On Sun, Apr 20, 2014 at 12:24 PM, Kevin Ushey <kevinushey at gmail.com> wrote:

            
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.r-forge.r-project.org/pipermail/rcpp-devel/attachments/20140420/90eecc00/attachment.html>
#
Hi Kevin,

I gave it a try and my program works as expected now. Thanks a lot! I had
also to update my R to version 3.1.0 and RTools to 3.1.
Best,
Venelin


2014-04-20 22:35 GMT+02:00 Kevin Ushey <kevinushey at gmail.com>: