Move dimension-related choices out of the class template parameters. (#…

…95)

This refers to previously-template parameters controlling the choice of
row-major/column-major for dense matrices, CSR/CSC for sparse matrices, whether
to combine/subset on the rows or columns, etc. By converting them into regular
arguments in the constructor, the class is more ergonomic for users who need to
make the choice at runtime. It should also reduce the amount of work that the
compiler needs to do, avoiding instantiation of near-redundant classes.

This change should not be associated with a performance penalty as the
dimension choice is not used within tight loops. At most, these parameters are
used to decide the subclass of the Extractor that is created by the various
*_row() and *_column() methods, after which the parameters are no longer used.
So there is no real downside to converting them into regular arguments.

The only exception is that of the delayed isometric unary class, which need
these parameters to be compile-time in order to enable constexpr if's. These
are allow us to avoid evaluating the untaken path where the methods might not
be implemented for a particular Operation_. So, we just leave them be. 

We also updated the convert_to_* functions so that they accept the output order
as a function argument. This allows streamlining of the tests. We removed the
overloads that automatically match the order of the output matrix with that of
the input matrix, as it is now trivial to do so with the other functions; this
is also required to avoid difficult-to-debug problems with overload selection.

include/tatami/base/Matrix.hpp

@@ -26,7 +26,7 @@ template<typename Index_>
 using VectorPtr = std::shared_ptr<const std::vector<Index_> >;
 
 /**
- * @brief Virtual class for a matrix with a defined type.
+ * @brief Virtual class for a matrix. 
  * 
  * @tparam Value Data value type, should be numeric.
  * @tparam Index Row/column index type, should be integer.

include/tatami/dense/DenseMatrix.hpp

@@ -158,41 +158,32 @@ struct SecondaryMyopicIndexDense : public MyopicDenseExtractor<Value_, Index_> {
 /**
  * @brief Dense matrix representation.
  *
- * @tparam row_ Whether this is a row-major representation.
- * If `false`, a column-major representation is assumed instead.
  * @tparam Value_ Type of the matrix values.
  * @tparam Index_ Type of the row/column indices.
  * @tparam Storage_ Vector class used to store the matrix values internally.
  * This does not necessarily have to contain `Value_`, as long as the type is convertible to `Value_`.
  * Methods should be available for `size()`, `begin()`, `end()` and `[]`.
  * If a method is available for `data()` that returns a `const Value_*`, it will also be used.
  */
-template<bool row_, typename Value_, typename Index_, class Storage_ = std::vector<Value_> >
+template<typename Value_, typename Index_, class Storage_ = std::vector<Value_> >
 class DenseMatrix : public Matrix<Value_, Index_> {
 public: 
     /**
      * @param nr Number of rows.
      * @param nc Number of columns.
-     * @param source Vector of values, or length equal to the product of `nr` and `nc`.
+     * @param vals Vector of values of length equal to the product of `nr` and `nc`.
+     * @param row Whether `vals` stores the matrix contents in a row-major representation.
+     * If `false`, a column-major representation is assumed instead.
      */
-    DenseMatrix(Index_ nr, Index_ nc, const Storage_& source) : nrows(nr), ncols(nc), values(source) {
-        check_dimensions(nr, nc, values.size());
-        return;
-    }
-
-    /**
-     * @param nr Number of rows.
-     * @param nc Number of columns.
-     * @param source Vector of values, or length equal to the product of `nr` and `nc`.
-     */
-    DenseMatrix(Index_ nr, Index_ nc, Storage_&& source) : nrows(nr), ncols(nc), values(source) {
+    DenseMatrix(Index_ nr, Index_ nc, Storage_ vals, bool row) : nrows(nr), ncols(nc), values(std::move(vals)), row_major(row) {
         check_dimensions(nr, nc, values.size());
         return;
     }
 
 private: 
     Index_ nrows, ncols;
     Storage_ values;
+    bool row_major;
 
     static void check_dimensions(size_t nr, size_t nc, size_t expected) { // cast to size_t is deliberate to avoid overflow on Index_ on product.
         if (nr * nc != expected) {
@@ -205,31 +196,31 @@ class DenseMatrix : public Matrix<Value_, Index_> {
 
     Index_ ncol() const { return ncols; }
 
-    bool prefer_rows() const { return row_; }
+    bool prefer_rows() const { return row_major; }
 
     bool uses_oracle(bool) const { return false; }
 
     bool sparse() const { return false; }
 
     double sparse_proportion() const { return 0; }
 
-    double prefer_rows_proportion() const { return static_cast<double>(row_); }
+    double prefer_rows_proportion() const { return static_cast<double>(row_major); }
 
     using Matrix<Value_, Index_>::dense;
 
     using Matrix<Value_, Index_>::sparse;
 
 private:
     Index_ primary() const {
-        if constexpr(row_) {
+        if (row_major) {
             return nrows;
         } else {
             return ncols;
         }
     }
 
     Index_ secondary() const {
-        if constexpr(row_) {
+        if (row_major) {
             return ncols;
         } else {
             return nrows;
@@ -241,23 +232,23 @@ class DenseMatrix : public Matrix<Value_, Index_> {
      *****************************/
 public:
     std::unique_ptr<MyopicDenseExtractor<Value_, Index_> > dense(bool row, const Options&) const {
-        if (row_ == row) {
+        if (row_major == row) {
             return std::make_unique<DenseMatrix_internals::PrimaryMyopicFullDense<Value_, Index_, Storage_> >(values, secondary());
         } else {
             return std::make_unique<DenseMatrix_internals::SecondaryMyopicFullDense<Value_, Index_, Storage_> >(values, secondary(), primary()); 
         }
     }
 
     std::unique_ptr<MyopicDenseExtractor<Value_, Index_> > dense(bool row, Index_ block_start, Index_ block_length, const Options&) const {
-        if (row_ == row) { 
+        if (row_major == row) { 
             return std::make_unique<DenseMatrix_internals::PrimaryMyopicBlockDense<Value_, Index_, Storage_> >(values, secondary(), block_start, block_length);
         } else {
             return std::make_unique<DenseMatrix_internals::SecondaryMyopicBlockDense<Value_, Index_, Storage_> >(values, secondary(), block_start, block_length);
         }
     }
 
     std::unique_ptr<MyopicDenseExtractor<Value_, Index_> > dense(bool row, VectorPtr<Index_> indices_ptr, const Options&) const {
-        if (row_ == row) {
+        if (row_major == row) {
             return std::make_unique<DenseMatrix_internals::PrimaryMyopicIndexDense<Value_, Index_, Storage_> >(values, secondary(), std::move(indices_ptr));
         } else {
             return std::make_unique<DenseMatrix_internals::SecondaryMyopicIndexDense<Value_, Index_, Storage_> >(values, secondary(), std::move(indices_ptr));
@@ -318,15 +309,31 @@ class DenseMatrix : public Matrix<Value_, Index_> {
  * Column-major matrix.
  * See `tatami::DenseMatrix` for details on the template parameters.
  */
-template<typename Value_, typename Index_ = int, class Storage_ = std::vector<Value_> >
-using DenseColumnMatrix = DenseMatrix<false, Value_, Index_, Storage_>;
+template<typename Value_, typename Index_, class Storage_ = std::vector<Value_> >
+class DenseColumnMatrix : public DenseMatrix<Value_, Index_, Storage_> {
+public:
+    /**
+     * @param nr Number of rows.
+     * @param nc Number of columns.
+     * @param vals Vector of values of length equal to the product of `nr` and `nc`, storing the matrix in column-major format.
+     */
+    DenseColumnMatrix(Index_ nr, Index_ nc, Storage_ vals) : DenseMatrix<Value_, Index_, Storage_>(nr, nc, std::move(vals), false) {}
+};
 
 /**
  * Row-major matrix.
  * See `tatami::DenseMatrix` for details on the template parameters.
  */
-template<typename Value_, typename Index_ = int, class Storage_ = std::vector<Value_> >
-using DenseRowMatrix = DenseMatrix<true, Value_, Index_, Storage_>;
+template<typename Value_, typename Index_, class Storage_ = std::vector<Value_> >
+class DenseRowMatrix : public DenseMatrix<Value_, Index_, Storage_> {
+public:
+    /**
+     * @param nr Number of rows.
+     * @param nc Number of columns.
+     * @param vals Vector of values of length equal to the product of `nr` and `nc`, storing the matrix in row-major format.
+     */
+    DenseRowMatrix(Index_ nr, Index_ nc, Storage_ vals) : DenseMatrix<Value_, Index_, Storage_>(nr, nc, std::move(vals), true) {}
+};
 
 }
 

include/tatami/dense/convert_to_dense.hpp

@@ -166,63 +166,19 @@ inline std::shared_ptr<Matrix<Value_, Index_> > convert_to_dense(const Matrix<In
     auto NC = incoming->ncol();
     std::vector<StoredValue_> buffer(static_cast<size_t>(NR) * static_cast<size_t>(NC));
     convert_to_dense(incoming, row, buffer.data(), threads);
-    if (row) {
-        return std::shared_ptr<Matrix<Value_, Index_> >(new DenseMatrix<true, Value_, Index_, decltype(buffer)>(NR, NC, std::move(buffer)));
-    } else {
-        return std::shared_ptr<Matrix<Value_, Index_> >(new DenseMatrix<false, Value_, Index_, decltype(buffer)>(NR, NC, std::move(buffer)));
-    }
-}
-
-/**
- * This overload makes it easier to control the desired output order when it is not known at compile time.
- *
- * @tparam Value_ Type of data values in the output interface.
- * @tparam Index Integer type for the indices in the output interface.
- * @tparam StoredValue_ Type of data values to be stored in the output.
- * @tparam Matrix_ Input matrix class, most typically a `tatami::Matrix`.
- *
- * @param incoming Pointer to a `tatami::Matrix`.
- * @param order Ordering of values in the output dense matrix - row-major (0) or column-major (1).
- * If set to -1, the ordering is chosen based on `tatami::Matrix::prefer_rows()`. 
- * @param threads Number of threads to use.
- *
- * @return A pointer to a new `tatami::DenseMatrix` with the same dimensions and type as the matrix referenced by `incoming`.
- */
-template <
-    typename Value_ = double,
-    typename Index_ = int,
-    typename StoredValue_ = Value_,
-    typename InputValue_,
-    typename InputIndex_
->
-std::shared_ptr<Matrix<Value_, Index_> > convert_to_dense(const Matrix<InputValue_, InputIndex_>* incoming, int order, int threads = 1) {
-    if (order < 0) {
-        order = static_cast<int>(!incoming->prefer_rows()); 
-    }
-    if (order == 0) {
-        return convert_to_dense<true, Value_, Index_, StoredValue_>(incoming, threads);
-    } else {
-        return convert_to_dense<false, Value_, Index_, StoredValue_>(incoming, threads);
-    }
+    return std::shared_ptr<Matrix<Value_, Index_> >(new DenseMatrix<Value_, Index_, decltype(buffer)>(NR, NC, std::move(buffer), row));
 }
 
 /**
  * @cond
  */
 // Backwards compatbility.
-template <bool row_, typename StoredValue_, typename InputValue_, typename InputIndex_>
+template<bool row_, typename StoredValue_, typename InputValue_, typename InputIndex_>
 void convert_to_dense(const Matrix<InputValue_, InputIndex_>* incoming, StoredValue_* store, int threads = 1) {
     convert_to_dense(incoming, row_, store, threads);
 }
 
-template <
-    bool row_,
-    typename Value_ = double, 
-    typename Index_ = int, 
-    typename StoredValue_ = Value_, 
-    typename InputValue_,
-    typename InputIndex_
->
+template<bool row_, typename Value_, typename Index_, typename StoredValue_ = Value_, typename InputValue_, typename InputIndex_>
 inline std::shared_ptr<Matrix<Value_, Index_> > convert_to_dense(const Matrix<InputValue_, InputIndex_>* incoming, int threads = 1) {
     return convert_to_dense<Value_, Index_, StoredValue_>(incoming, row_, threads);
 }

include/tatami/isometric/binary/arith_helpers.hpp

@@ -10,16 +10,14 @@
  * @file arith_helpers.hpp
  *
  * @brief Helper classes for binary arithmetic operations.
- * 
- * Classes defined here should be used as the `OP` in the `DelayedBinaryIsometricOp` class.
  */
 
 namespace tatami {
 
 /**
  * @brief Delayed binary arithmetic.
  *
- * This should be used as the `OP` in the `DelayedBinaryIsometricOp` class.
+ * This should be used as the `Operation_` in the `DelayedBinaryIsometricOp` class.
  *
  * @tparam op_ The arithmetic operation.
  */

include/tatami/isometric/binary/boolean_helpers.hpp

@@ -8,16 +8,14 @@
  * @file boolean_helpers.hpp
  *
  * @brief Helper classes for binary boolean operations.
- * 
- * Classes defined here should be used as the `OP` in the `DelayedBinaryIsometricOp` class.
  */
 
 namespace tatami {
 
 /**
  * @brief Delayed binary boolean operations.
  *
- * This should be used as the `OP` in the `DelayedBinaryIsometricOp` class.
+ * This should be used as the `Operation_` in the `DelayedBinaryIsometricOp` class.
  *
  * @tparam op_ The boolean operation.
  */

include/tatami/isometric/binary/compare_helpers.hpp

@@ -8,16 +8,14 @@
  * @file compare_helpers.hpp
  *
  * @brief Helper classes for binary comparison operations.
- * 
- * Classes defined here should be used as the `OP` in the `DelayedBinaryIsometricOp` class.
  */
 
 namespace tatami {
 
 /**
  * @brief Delayed binary comparison.
  *
- * This should be used as the `OP` in the `DelayedBinaryIsometricOp` class.
+ * This should be used as the `Operation_` in the `DelayedBinaryIsometricOp` class.
  *
  * @tparam op_ The comparison operation.
  */

include/tatami/isometric/unary/arith_helpers.hpp

@@ -9,8 +9,6 @@
  * @file arith_helpers.hpp
  *
  * @brief Helper classes for delayed unary arithmetic operations.
- * 
- * Classes defined here should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
  */
 
 namespace tatami {
@@ -66,7 +64,7 @@ Value_ delayed_arith_zero(Scalar_ scalar) {
 /**
  * @brief Delayed scalar arithmetic.
  *
- * This should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
+ * This should be used as the `Operation_` in the `DelayedUnaryIsometricOp` class.
  *
  * @tparam op_ The arithmetic operation.
  * @tparam right_ Whether the scalar should be on the right hand side of the arithmetic operation.
@@ -138,7 +136,7 @@ struct DelayedArithScalarHelper {
 /**
  * @brief Delayed vector arithmetic.
  *
- * This should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
+ * This should be used as the `Operation_` in the `DelayedUnaryIsometricOp` class.
  *
  * @tparam op_ The arithmetic operation.
  * @tparam right_ Whether the vector's values should be on the right hand side of the arithmetic operation.

include/tatami/isometric/unary/boolean_helpers.hpp

@@ -8,8 +8,6 @@
  * @file boolean_helpers.hpp
  *
  * @brief Helper classes for delayed unary boolean operations.
- * 
- * Classes defined here should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
  */
 
 namespace tatami {
@@ -37,7 +35,7 @@ bool delayed_boolean_actual_sparse(bool scalar) {
 /**
  * @brief Delayed scalar boolean operation.
  *
- * This should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
+ * This should be used as the `Operation_` in the `DelayedUnaryIsometricOp` class.
  *
  * @tparam op_ The boolean operation.
  * @tparam Value_ Type of the data value.
@@ -107,7 +105,7 @@ struct DelayedBooleanScalarHelper {
 /**
  * @brief Delayed boolean NOT operation.
  *
- * This should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
+ * This should be used as the `Operation_` in the `DelayedUnaryIsometricOp` class.
  *
  * @tparam Value_ Type of the data value.
  */
@@ -170,7 +168,7 @@ struct DelayedBooleanNotHelper {
 /**
  * @brief Delayed vector boolean operations.
  *
- * This should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
+ * This should be used as the `Operation_` in the `DelayedUnaryIsometricOp` class.
  *
  * @tparam op_ The boolean operation.
  * @tparam margin_ Matrix dimension along which the operation is to occur.

include/tatami/isometric/unary/compare_helpers.hpp

@@ -8,8 +8,6 @@
  * @file compare_helpers.hpp
  *
  * @brief Helper classes for delayed unary comparison operations.
- * 
- * Classes defined here should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
  */
 
 namespace tatami {
@@ -37,7 +35,7 @@ bool delayed_compare_actual_sparse(Scalar_ scalar) {
 /**
  * @brief Delayed scalar comparison.
  *
- * This should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
+ * This should be used as the `Operation_` in the `DelayedUnaryIsometricOp` class.
  *
  * @tparam op_ The comparison operation.
  * @tparam Value_ Type of the data value.
@@ -108,7 +106,7 @@ struct DelayedCompareScalarHelper {
 /**
  * @brief Delayed vector comparisons.
  *
- * This should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
+ * This should be used as the `Operation_` in the `DelayedUnaryIsometricOp` class.
  *
  * @tparam op_ The comparison operation.
  * @tparam margin_ Matrix dimension along which the operation is to occur.

include/tatami/isometric/unary/math_helpers.hpp

@@ -5,8 +5,6 @@
  * @file math_helpers.hpp
  *
  * @brief Helpers for unary math operations.
- *
- * These classes should be used as the `OP` in the `DelayedUnaryIsometricOp` class.
  */
 
 #include <cmath>