Skip to content

Commit

Permalink
Merge pull request #642 from nathan-russell/bugfix/upper-lower-tri
Browse files Browse the repository at this point in the history 
Fixes for upper_tri and lower_tri with unit tests (closes #641)
  • Loading branch information
@eddelbuettel
eddelbuettel committed Jan 31, 2017
2 parents 5a99a86 + bffd6ff commit d05d9c6
Show file tree
Hide file tree
Showing 6 changed files with 300 additions and 207 deletions.
9 changes: 9 additions & 0 deletions ChangeLog
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
2017-01-31 Nathan Russell <russell.nr2012@gmail.com>

* inst/include/Rcpp/sugar/matrix/upper_tri.h: Inherit from MatrixBase
and use correct comparators in get() to fix segfault
* inst/include/Rcpp/sugar/matrix/lower_tri.h: Idem
* inst/unitTests/cpp/sugar.cpp: Added unit tests for upper_tri and
lower_tri
* inst/unitTests/runit.sugar.R: Idem

2017-01-23 James J Balamuta <balamut2@illinois.edu>

* inst/include/Rcpp/DataFrame.h: Corrected return type for column size.
Expand Down
5 changes: 5 additions & 0 deletions inst/NEWS.Rd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
\item Added new size attribute aliases for number of rows and columns in
DataFrame (James Balamuta in \ghpr{638} addressing \ghit{630}).
}
\item Changes in Rcpp Sugar:
\itemize{
\item Fixed sugar functions \code{upper_tri()} and \code{lower_tri()}
(Nathan Russell in \ghpr{642} addressing \ghit{641}).
}
}
}

Expand Down
74 changes: 31 additions & 43 deletions inst/include/Rcpp/sugar/matrix/lower_tri.h
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; tab-width: 8 -*-
// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; indent-tabs-mode: nil; -*-
//
// lower_tri.h: Rcpp R/C++ interface class library -- lower.tri
//
// Copyright (C) 2010 - 2011 Dirk Eddelbuettel and Romain Francois
// Copyright (C) 2010 - 2017 Dirk Eddelbuettel and Romain Francois
// Copyright (C) 2017 Dirk Eddelbuettel, Romain Francois, and Nathan Russell
//
// This file is part of Rcpp.
//
Expand All @@ -22,60 +23,47 @@
#ifndef Rcpp__sugar__lower_tri_h
#define Rcpp__sugar__lower_tri_h

namespace Rcpp{
namespace sugar{
namespace Rcpp {
namespace sugar {

template <int RTYPE, bool LHS_NA, typename LHS_T>
class LowerTri : public VectorBase<
LGLSXP ,
false ,
LowerTri<RTYPE,LHS_NA,LHS_T>
> {
template <int RTYPE, bool NA, typename T>
class LowerTri : public MatrixBase<LGLSXP, false, LowerTri<RTYPE, NA, T> > {
public:
typedef Rcpp::MatrixBase<RTYPE,LHS_NA,LHS_T> LHS_TYPE ;
typedef Rcpp::MatrixBase<RTYPE, NA, T> MatBase;

LowerTri( const LHS_TYPE& lhs, bool diag) :
nr( lhs.nrow() ), nc( lhs.ncol() ),
getter( diag ? (&LowerTri::get_diag_true) : (&LowerTri::get_diag_false) ){}
LowerTri(const T& lhs, bool diag)
: nr(lhs.nrow()),
nc(lhs.ncol()),
getter(diag ? (&LowerTri::get_diag_true) : (&LowerTri::get_diag_false))
{}

// inline int operator[]( int index ) const {
// int i = Rcpp::internal::get_line( index, nr ) ;
// int j = Rcpp::internal::get_column( index, nr, i ) ;
// return get(i,j) ;
// }
inline int operator()( int i, int j ) const {
return get(i,j) ;
}
inline int operator()(int i, int j) const { return get(i, j); }

inline R_xlen_t size() const { return static_cast<R_xlen_t>(nr) * nc ; }
inline int nrow() const { return nr; }
inline int ncol() const { return nc; }
inline R_xlen_t size() const { return static_cast<R_xlen_t>(nr) * nc; }
inline int nrow() const { return nr; }
inline int ncol() const { return nc; }

private:
int nr, nc ;
typedef bool (LowerTri::*Method)(int,int) ;
typedef bool (LowerTri::*Method)(int, int) const;

Method getter ;
inline bool get_diag_true( int i, int j ){
return i <= j ;
}
inline bool get_diag_false( int i, int j ){
return i < j ;
}
inline bool get( int i, int j){
return (this->*getter)(i, j ) ;
}
int nr, nc;
Method getter;

} ;
inline bool get_diag_true(int i, int j) const { return i >= j; }

inline bool get_diag_false(int i, int j) const { return i > j; }

inline bool get(int i, int j) const { return (this->*getter)(i, j); }
};

} // sugar

template <int RTYPE, bool LHS_NA, typename LHS_T>
inline sugar::LowerTri<RTYPE,LHS_NA,LHS_T>
lower_tri( const Rcpp::MatrixBase<RTYPE,LHS_NA,LHS_T>& lhs, bool diag = false){
return sugar::LowerTri<RTYPE,LHS_NA,LHS_T>( lhs, diag ) ;
template <int RTYPE, bool NA, typename T>
inline sugar::LowerTri<RTYPE, NA, T>
lower_tri(const Rcpp::MatrixBase<RTYPE, NA, T>& lhs, bool diag = false) {
return sugar::LowerTri<RTYPE, NA, T>(lhs, diag);
}

} // Rcpp

#endif
#endif // Rcpp__sugar__lower_tri_h
71 changes: 32 additions & 39 deletions inst/include/Rcpp/sugar/matrix/upper_tri.h
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; tab-width: 8 -*-
// -*- mode: C++; c-indent-level: 4; c-basic-offset: 4; indent-tabs-mode: nil; -*-
//
// upper_tri.h: Rcpp R/C++ interface class library -- lower.tri
// upper_tri.h: Rcpp R/C++ interface class library -- upper.tri
//
// Copyright (C) 2010 - 2011 Dirk Eddelbuettel and Romain Francois
// Copyright (C) 2010 - 2017 Dirk Eddelbuettel and Romain Francois
// Copyright (C) 2017 Dirk Eddelbuettel, Romain Francois, and Nathan Russell
//
// This file is part of Rcpp.
//
Expand All @@ -22,55 +23,47 @@
#ifndef Rcpp__sugar__upper_tri_h
#define Rcpp__sugar__upper_tri_h

namespace Rcpp{
namespace sugar{
namespace Rcpp {
namespace sugar {

template <int RTYPE, bool LHS_NA, typename LHS_T>
class UpperTri : public VectorBase<
LGLSXP ,
false ,
UpperTri<RTYPE,LHS_NA,LHS_T>
> {
template <int RTYPE, bool NA, typename T>
class UpperTri : public MatrixBase<LGLSXP, false, UpperTri<RTYPE, NA, T> > {
public:
typedef Rcpp::MatrixBase<RTYPE,LHS_NA,LHS_T> LHS_TYPE ;
typedef Rcpp::MatrixBase<RTYPE, NA, T> MatBase;

UpperTri( const LHS_TYPE& lhs, bool diag) :
nr( lhs.nrow() ), nc( lhs.ncol() ),
getter( diag ? (&UpperTri::get_diag_true) : (&UpperTri::get_diag_false) ){}
UpperTri(const T& lhs, bool diag)
: nr(lhs.nrow()),
nc(lhs.ncol()),
getter(diag ? (&UpperTri::get_diag_true) : (&UpperTri::get_diag_false))
{}

inline int operator()( int i, int j ) const {
return get(i,j) ;
}
inline int operator()(int i, int j) const { return get(i, j); }

inline R_xlen_t size() const { return static_cast<R_xlen_t>(nr) * nc ; }
inline int nrow() const { return nr; }
inline int ncol() const { return nc; }
inline R_xlen_t size() const { return static_cast<R_xlen_t>(nr) * nc; }
inline int nrow() const { return nr; }
inline int ncol() const { return nc; }

private:
int nr, nc ;
typedef bool (UpperTri::*Method)(int,int) ;
typedef bool (UpperTri::*Method)(int, int) const;

Method getter ;
inline bool get_diag_true( int i, int j ){
return i >= j ;
}
inline bool get_diag_false( int i, int j ){
return i > j ;
}
inline bool get( int i, int j){
return (this->*getter)(i, j ) ;
}
int nr, nc;
Method getter;

} ;
inline bool get_diag_true(int i, int j) const { return i <= j; }

inline bool get_diag_false(int i, int j) const { return i < j; }

inline bool get(int i, int j) const { return (this->*getter)(i, j); }
};

} // sugar

template <int RTYPE, bool LHS_NA, typename LHS_T>
inline sugar::UpperTri<RTYPE,LHS_NA,LHS_T>
upper_tri( const Rcpp::MatrixBase<RTYPE,LHS_NA,LHS_T>& lhs, bool diag = false){
return sugar::UpperTri<RTYPE,LHS_NA,LHS_T>( lhs, diag ) ;
template <int RTYPE, bool NA, typename T>
inline sugar::UpperTri<RTYPE, NA, T>
upper_tri(const Rcpp::MatrixBase<RTYPE, NA, T>& lhs, bool diag = false) {
return sugar::UpperTri<RTYPE, NA, T>(lhs, diag);
}

} // Rcpp

#endif
#endif // Rcpp__sugar__upper_tri_h

0 comments on commit d05d9c6

Please sign in to comment.