Skip to content

[Rcpp-devel] api/meat/is.h missing CharacterVector and CharacterMatrix

7 messages · Romain Francois, Thomas Tse, Dirk Eddelbuettel

Original
Thomas Tse
# 
hi,

seems like?api/meat/is.h missed the implementation for CharacterVector and CharacterMatrix, i.e. we should add:

? ? template <> inline bool is__simple<CharacterVector>( SEXP x ){
? ? ? ? return TYPEOF(x) == STRSXP ;
? ? }
? ? template <> inline bool is__simple<CharacterMatrix>( SEXP x ){
? ? ? ? return TYPEOF(x) == STRSXP && is_matrix(x) ;
? ? }

thanks,
Thomas
Dirk Eddelbuettel
# 
Thomas,
On 2 October 2013 at 10:12, Thomas Tse wrote:
| hi,
| 
| seems like?api/meat/is.h missed the implementation for CharacterVector and CharacterMatrix, i.e. we should add:
| 
| ? ? template <> inline bool is__simple<CharacterVector>( SEXP x ){
| ? ? ? ? return TYPEOF(x) == STRSXP ;
| ? ? }
| ? ? template <> inline bool is__simple<CharacterMatrix>( SEXP x ){
| ? ? ? ? return TYPEOF(x) == STRSXP && is_matrix(x) ;
| ? ? }

Looks like a good catch, and nice work.  If I add this, I can in fact do

R> cppFunction('bool isCharVec(SEXP x) {return(is<CharacterVector>(x));}')
R> isCharVec(LETTERS)
[1] TRUE
R> 

whereas I get a linker error without your suggested change. Unit tests all
pass, so no colleteral damage -- hence committed as rev4553, and added you to
the THANKS file.

Thanks -- keep the fixes coming!  ;-)

Dirk
Romain Francois
# 
Le 02/10/13 05:05, Dirk Eddelbuettel a ?crit :
Just to let you know. I've implemented is slightly differently in 
Rcpp11. The template is function delegates to a Is template class with a 
test method. Details on this commit: 
https://github.com/romainfrancois/Rcpp11/commit/398a58cb5d7bd3098efad6b6e98c851c9c1407bb

The reason it is relevant is when we start to want to make partial 
specializations, which we cannot do with function templates. So for 
example, we could want to implement is for armadillo matrices. We would 
just have to do :

template <typename T> struct Is< arma::Mat<T> > :
             Is< Matrix<Rcpp::traits::r_sexptype_traits<T>::rtype> >{} ;

whereas with the current implementation in Rcpp, we would have to fully 
specify for arma::mat, arma::cx_mat, ...

The only c++11 thing here is the use of std::conditional, but Rcpp has 
traits::if_ do do the same.

Romain
Thomas Tse
# 
Hi,

is_na() does not handles R's NaN correctly, for the below test.cpp:

#include <Rcpp.h>
using namespace Rcpp;
using namespace std;

// [[Rcpp::export]]
void test (NumericVector x)
{
? LogicalVector y1 = is_na(x);
? LogicalVector y2 = is_finite(x);
? LogicalVector y3 = is_infinite(x);
? LogicalVector y4 = is_nan(x);

? Rcpp::Rcout << "x= ";
? for(int i = 0; i < x.size(); i++) Rcpp::Rcout << x[i] << " ";
? Rcpp::Rcout << std::endl;

? Rcpp::Rcout << "is_na(x)= ";
? for(int i = 0; i < y1.size(); i++) Rcpp::Rcout << y1[i] << " ";
? Rcpp::Rcout << std::endl;

? Rcpp::Rcout << "is_finite(x)= ";
? for(int i = 0; i < y2.size(); i++) Rcpp::Rcout << y2[i] << " ";
? Rcpp::Rcout << std::endl;

? Rcpp::Rcout << "is_infinite(x)= ";
? for(int i = 0; i < y3.size(); i++) Rcpp::Rcout << y3[i] << " ";
? Rcpp::Rcout << std::endl;

? Rcpp::Rcout << "is_nan(x)= ";
? for(int i = 0; i < y4.size(); i++) Rcpp::Rcout << y4[i] << " ";
? Rcpp::Rcout << std::endl;
}



and the below codes in R, we see that is_na() in C++ does NOT match?is.na() in R (in NaN case):
x= 1 2 3 1.#QNAN 4 1.#INF -1.#INF -1.#IND
is_na(x)= 0 0 0 1 0 0 0 0
is_finite(x)= 1 1 1 0 1 0 0 0
is_infinite(x)= 0 0 0 0 0 1 1 0
is_nan(x)= 0 0 0 0 0 0 0 1
[1] FALSE FALSE FALSE ?TRUE FALSE FALSE FALSE ?TRUE
[1] ?TRUE ?TRUE ?TRUE FALSE ?TRUE FALSE FALSE FALSE
[1] FALSE FALSE FALSE FALSE FALSE ?TRUE ?TRUE FALSE
[1] FALSE FALSE FALSE FALSE FALSE FALSE FALSE ?TRUE


so, in .....\include\Rcpp\traits\is_na.h, we should modify like this:
template <>
inline bool is_na<REALSXP>( double x ){
//return R_IsNA(x) ;
return R_IsNA(x) || R_IsNaN(x);
}

template <>
inline bool is_na<CPLXSXP>( Rcomplex x ){
//return R_IsNA(x.r) || R_IsNA(x.i) ;
return R_IsNA(x.r) || R_IsNA(x.i) || R_IsNaN(x.r) || R_IsNaN(x.i);
}



is it possible to apply this change in next release of Rcpp ?

many thanks,
Thomas
Romain Francois
# 
Hi,

The question is that do we want to be consistent with R or do we want 
something that makes sense.

I believe that is_na should test if its argument is NA, and is_nan 
should check if is NaN

Romain

Le 02/10/13 13:31, Thomas Tse a ?crit :

  
    

      
      
    

  


  


      

    

    

      
      

        


  

        

      Thomas Tse
    

    

      
      #
    

  


  

      
Hi,

in R, is.finite() and is.infinite() always return FALSE for character():

            

      
      
      
    

        
[1] FALSE FALSE FALSE

            

      
      
      
    

        
[1] FALSE FALSE FALSE

so, we may need to change the template?is_finite<STRSXP> from:

template <>

inline bool is_finite<STRSXP>( SEXP x ){
? ? return x != NA_STRING ;
}



to:

template <>

inline bool is_finite<STRSXP>( SEXP x ){
? ? return false;

}


thanks,
Thomas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.r-forge.r-project.org/pipermail/rcpp-devel/attachments/20131002/57d8f52d/attachment.html>

  
  


  


      

    

    

      
      

        


  

        

      Dirk Eddelbuettel
    

    

      
      #
    

  


  

      
Thomas,

        
On 2 October 2013 at 19:31, Thomas Tse wrote:

        
| is_na() does not handles R's NaN correctly, for the below test.cpp:
[...]
| is it possible to apply this change in next release of Rcpp ?

Certainly.  Though Romain has a point:

        
On 2 October 2013 at 13:36, Romain Francois wrote:

        
| The question is that do we want to be consistent with R or do we want 
| something that makes sense.
| 
| I believe that is_na should test if its argument is NA, and is_nan 
| should check if is NaN

We have to make sure that we're doing is "consistent" for various definitions
of consistent. IEEE754 only knows NaN and Inf; NA is an R extension.

But from the top of my head I'd have to say I would want Rcpp to behave like
R over behaving like C++.  But I could be swayed.  Other views?

Dirk