Shifted aspects of the isometric helper contracts back to runtime. (#99)

This aligns with the general policy of moving row-based parameters out of the template - in this case, the margin_ for vector operations is now a regular class member. This makes the isometric operation interfaces more flexible as developers aren't forced into templating the helpers to meet the contract. Doing this means that we have to replace most of the static constexpr's with runtime method calls. This shouldn't have any major perf impact as the comparison of 'margin_' with 'row' is already done at runtime due to the latter being a variable. Nonetheless, we allow the helper classes to skip this runtime check if the relevant methods are not implemented, in which case they are assumed to always return false (known at compile time via some SFINAE magic). We do require a static constexpr to check whether the developer wants to use the basic or advanced interface. This is made explicit for easier debugging, rather than trying to guess the interface from the methods via SFINAE.
tatami-inc · May 20, 2024 · f7a97e6 · f7a97e6
1 parent 0d970d4
commit f7a97e6
Show file tree

Hide file tree

Showing 19 changed files with 922 additions and 777 deletions.
diff --git a/docs/Doxyfile b/docs/Doxyfile
@@ -997,7 +997,8 @@ RECURSIVE              = YES
 EXCLUDE                = ../include/tatami/subset/utils.hpp \
                          ../include/tatami/sparse/secondary_extraction.hpp \
                          ../include/tatami/sparse/primary_extraction.hpp \
-                         ../include/tatami/isometric/binary/utils.hpp
+                         ../include/tatami/isometric/binary/utils.hpp \
+                         ../include/tatami/isometric/depends_utils.hpp
 
 # The EXCLUDE_SYMLINKS tag can be used to select whether or not files or
 # directories that are symbolic links (a Unix file system feature) are excluded

diff --git a/include/tatami/isometric/binary/DelayedBinaryIsometricOperation.hpp b/include/tatami/isometric/binary/DelayedBinaryIsometricOperation.hpp
@@ -5,13 +5,14 @@
 #include "../../utils/new_extractor.hpp"
 #include "../../utils/copy.hpp"
 #include "../../dense/SparsifiedWrapper.hpp"
+#include "../depends_utils.hpp"
 
 #include <memory>
 #include <vector>
 #include <type_traits>
 
 /**
- * @file DelayedBinaryIsometricOp.hpp
+ * @file DelayedBinaryIsometricOperation.hpp
  *
  * @brief Delayed binary isometric operations.
  */
@@ -23,31 +24,6 @@ namespace tatami {
  */
 namespace DelayedBinaryIsometricOperation_internal {
 
-template<class Operation_, bool oracle_, typename Index_>
-class MaybeOracleDepends {
-public:
-    MaybeOracleDepends(const MaybeOracle<oracle_, Index_>& oracle, bool row) {
-        if ((row  && Operation_::zero_depends_on_row) || (!row && Operation_::zero_depends_on_column)) {
-            my_oracle = oracle;
-        }
-    }
-
-    Index_ get(Index_ i) {
-        if constexpr(oracle_) {
-            if constexpr(Operation_::zero_depends_on_row || Operation_::zero_depends_on_column) {
-                if (my_oracle) {
-                    return my_oracle->get(my_used++);
-                }
-            }
-        }
-        return i;
-    }
-
-private:
-    MaybeOracle<oracle_, Index_> my_oracle;
-    typename std::conditional<oracle_, size_t, bool>::type my_used = 0;
-};
-
 /********************
  *** Dense simple ***
  ********************/
@@ -64,7 +40,7 @@ class DenseSimpleFull : public DenseExtractor<oracle_, Value_, Index_> {
         const Options& opt) :
         my_operation(operation),
         my_row(row),
-        my_oracle(oracle, row)
+        my_oracle(oracle, my_operation, row)
     {
         my_left_ext = new_extractor<false, oracle_>(left, my_row, oracle, opt);
         my_right_ext = new_extractor<false, oracle_>(right, my_row, std::move(oracle), opt);
@@ -83,7 +59,7 @@ class DenseSimpleFull : public DenseExtractor<oracle_, Value_, Index_> {
 private:
     const Operation_& my_operation;
     bool my_row;
-    MaybeOracleDepends<Operation_, oracle_, Index_> my_oracle;
+    DelayedIsometricOperation_internal::MaybeOracleDepends<oracle_, Operation_, Index_> my_oracle;
 
     std::unique_ptr<DenseExtractor<oracle_, Value_, Index_> > my_left_ext, my_right_ext;
     Index_ my_extent;
@@ -104,7 +80,7 @@ class DenseSimpleBlock : public DenseExtractor<oracle_, Value_, Index_> {
         const Options& opt) :
         my_operation(operation),
         my_row(row),
-        my_oracle(oracle, row),
+        my_oracle(oracle, my_operation, row),
         my_block_start(block_start),
         my_block_length(block_length)
     {
@@ -124,7 +100,7 @@ class DenseSimpleBlock : public DenseExtractor<oracle_, Value_, Index_> {
 private:
     const Operation_& my_operation;
     bool my_row;
-    MaybeOracleDepends<Operation_, oracle_, Index_> my_oracle;
+    DelayedIsometricOperation_internal::MaybeOracleDepends<oracle_, Operation_, Index_> my_oracle;
 
     Index_ my_block_start, my_block_length;
     std::unique_ptr<DenseExtractor<oracle_, Value_, Index_> > my_left_ext, my_right_ext;
@@ -144,7 +120,7 @@ class DenseSimpleIndex : public DenseExtractor<oracle_, Value_, Index_> {
         const Options& opt) :
         my_operation(operation),
         my_row(row),
-        my_oracle(oracle, row),
+        my_oracle(oracle, my_operation, row),
         my_indices_ptr(std::move(indices_ptr))
     {
         my_left_ext = new_extractor<false, oracle_>(left, my_row, oracle, my_indices_ptr, opt);
@@ -163,7 +139,7 @@ class DenseSimpleIndex : public DenseExtractor<oracle_, Value_, Index_> {
 private:
     const Operation_& my_operation;
     bool my_row;
-    MaybeOracleDepends<Operation_, oracle_, Index_> my_oracle;
+    DelayedIsometricOperation_internal::MaybeOracleDepends<oracle_, Operation_, Index_> my_oracle;
 
     VectorPtr<Index_> my_indices_ptr;
     std::unique_ptr<DenseExtractor<oracle_, Value_, Index_> > my_left_ext, my_right_ext;
@@ -186,7 +162,7 @@ class DenseExpandedFull : public DenseExtractor<oracle_, Value_, Index_> {
         Options opt) :
         my_operation(op),
         my_row(row),
-        my_oracle(oracle, row)
+        my_oracle(oracle, my_operation, row)
     {
         opt.sparse_extract_value = true;
         opt.sparse_extract_index = true;
@@ -210,9 +186,10 @@ class DenseExpandedFull : public DenseExtractor<oracle_, Value_, Index_> {
         i = my_oracle.get(i);
         auto num = my_operation.sparse(my_row, i, lres, rres, my_output_vbuffer.data(), my_output_ibuffer.data(), true, true);
 
-        // Avoid calling zero() if possible, as this might throw zero-related errors in non-IEEE platforms.
+        // Avoid calling my_operation.fill() if possible, as this might throw
+        // zero-related errors in non-IEEE platforms.
         if (num < my_extent) { 
-            std::fill_n(buffer, my_extent, my_operation.template fill<Value_>(i));
+            std::fill_n(buffer, my_extent, my_operation.template fill<Value_>(my_row, i));
         }
 
         for (Index_ j = 0; j < num; ++j) {
@@ -224,7 +201,7 @@ class DenseExpandedFull : public DenseExtractor<oracle_, Value_, Index_> {
 private:
     const Operation_& my_operation;
     bool my_row;
-    MaybeOracleDepends<Operation_, oracle_, Index_> my_oracle;
+    DelayedIsometricOperation_internal::MaybeOracleDepends<oracle_, Operation_, Index_> my_oracle;
 
     std::unique_ptr<SparseExtractor<oracle_, Value_, Index_> > my_left_ext, my_right_ext;
     Index_ my_extent;
@@ -246,7 +223,7 @@ class DenseExpandedBlock : public DenseExtractor<oracle_, Value_, Index_> {
         Options opt) :
         my_operation(operation),
         my_row(row),
-        my_oracle(oracle, row),
+        my_oracle(oracle, my_operation, row),
         my_block_start(block_start),
         my_block_length(block_length)
     {
@@ -271,9 +248,10 @@ class DenseExpandedBlock : public DenseExtractor<oracle_, Value_, Index_> {
         i = my_oracle.get(i);
         auto num = my_operation.sparse(my_row, i, lres, rres, my_output_vbuffer.data(), my_output_ibuffer.data(), true, true);
 
-        // Avoid calling zero() if possible, as this might throw zero-related errors in non-IEEE platforms.
+        // Avoid calling my_operation.fill() if possible, as this might throw
+        // zero-related errors in non-IEEE platforms.
         if (num < my_block_length) { 
-            std::fill_n(buffer, my_block_length, my_operation.template fill<Value_>(i));
+            std::fill_n(buffer, my_block_length, my_operation.template fill<Value_>(my_row, i));
         }
 
         for (Index_ j = 0; j < num; ++j) {
@@ -285,7 +263,7 @@ class DenseExpandedBlock : public DenseExtractor<oracle_, Value_, Index_> {
 private:
     const Operation_& my_operation;
     bool my_row;
-    MaybeOracleDepends<Operation_, oracle_, Index_> my_oracle;
+    DelayedIsometricOperation_internal::MaybeOracleDepends<oracle_, Operation_, Index_> my_oracle;
     Index_ my_block_start, my_block_length;
 
     std::unique_ptr<SparseExtractor<oracle_, Value_, Index_> > my_left_ext, my_right_ext;
@@ -306,7 +284,7 @@ class DenseExpandedIndex : public DenseExtractor<oracle_, Value_, Index_> {
         Options opt) :
         my_operation(operation),
         my_row(row),
-        my_oracle(oracle, row),
+        my_oracle(oracle, my_operation, row),
         my_extent(indices_ptr->size())
     {
         // Create a remapping vector to map the extracted indices back to the
@@ -342,9 +320,10 @@ class DenseExpandedIndex : public DenseExtractor<oracle_, Value_, Index_> {
         i = my_oracle.get(i);
         auto num = my_operation.sparse(my_row, i, lres, rres, my_output_vbuffer.data(), my_output_ibuffer.data(), true, true);
 
-        // Avoid calling zero() if possible, as this might throw zero-related errors in non-IEEE platforms.
+        // Avoid calling my_operation.fill() if possible, as this might throw
+        // zero-related errors in non-IEEE platforms.
         if (num < my_extent) { 
-            std::fill_n(buffer, my_extent, my_operation.template fill<Value_>(i));
+            std::fill_n(buffer, my_extent, my_operation.template fill<Value_>(my_row, i));
         }
 
         for (Index_ j = 0; j < num; ++j) {
@@ -356,7 +335,7 @@ class DenseExpandedIndex : public DenseExtractor<oracle_, Value_, Index_> {
 private:
     const Operation_& my_operation;
     bool my_row;
-    MaybeOracleDepends<Operation_, oracle_, Index_> my_oracle;
+    DelayedIsometricOperation_internal::MaybeOracleDepends<oracle_, Operation_, Index_> my_oracle;
     Index_ my_extent;
 
     std::vector<Index_> my_remapping;
@@ -383,7 +362,7 @@ class Sparse : public SparseExtractor<oracle_, Value_, Index_> {
         Options opt) :
         my_operation(operation),
         my_row(row),
-        my_oracle(oracle, row)
+        my_oracle(oracle, my_operation, row)
     {
         initialize(my_row ? left->ncol() : left->nrow(), opt);
         my_left_ext = new_extractor<true, oracle_>(left, my_row, oracle, opt);
@@ -401,7 +380,7 @@ class Sparse : public SparseExtractor<oracle_, Value_, Index_> {
         Options opt) :
         my_operation(operation),
         my_row(row),
-        my_oracle(oracle, row)
+        my_oracle(oracle, my_operation, row)
     {
         initialize(block_length, opt);
         my_left_ext = new_extractor<true, oracle_>(left, my_row, oracle, block_start, block_length, opt);
@@ -418,7 +397,7 @@ class Sparse : public SparseExtractor<oracle_, Value_, Index_> {
         Options opt) :
         my_operation(operation),
         my_row(row),
-        my_oracle(oracle, row)
+        my_oracle(oracle, my_operation, row)
     {
         initialize(indices_ptr->size(), opt); // do this before the move.
         my_left_ext = new_extractor<true, oracle_>(left, my_row, oracle, indices_ptr, opt);
@@ -466,7 +445,7 @@ class Sparse : public SparseExtractor<oracle_, Value_, Index_> {
 private:
     const Operation_& my_operation;
     bool my_row;
-    MaybeOracleDepends<Operation_, oracle_, Index_> my_oracle;
+    DelayedIsometricOperation_internal::MaybeOracleDepends<oracle_, Operation_, Index_> my_oracle;
 
     std::unique_ptr<SparseExtractor<oracle_, Value_, Index_> > my_left_ext, my_right_ext;
     std::vector<Value_> my_left_vbuffer, my_right_vbuffer;
@@ -513,7 +492,7 @@ class DelayedBinaryIsometricOperation : public Matrix<Value_, Index_> {
 
         my_prefer_rows_proportion = (my_left->prefer_rows_proportion() + my_right->prefer_rows_proportion()) / 2;
 
-        if constexpr(is_advanced) {
+        if constexpr(!Operation_::is_basic) {
             if (my_operation.is_sparse()) {
                 my_is_sparse = my_left->is_sparse() && my_right->is_sparse();
 
@@ -531,8 +510,6 @@ class DelayedBinaryIsometricOperation : public Matrix<Value_, Index_> {
     double my_is_sparse_proportion = 0;
     bool my_is_sparse = false;
 
-    static constexpr bool is_advanced = (!Operation_::zero_depends_on_row || !Operation_::zero_depends_on_column);
-
 public:
     Index_ nrow() const {
         return my_left->nrow();
@@ -650,11 +627,9 @@ class DelayedBinaryIsometricOperation : public Matrix<Value_, Index_> {
 
     template<bool oracle_, typename ... Args_>
     std::unique_ptr<DenseExtractor<oracle_, Value_, Index_> > dense_internal(bool row, Args_&& ... args) const {
-        if constexpr(is_advanced) {
+        if constexpr(!Operation_::is_basic) {
             if (my_left->is_sparse() && my_right->is_sparse()) {
-                // If we don't depend on the rows, then we don't need row indices when 'row = false'.
-                // Similarly, if we don't depend on columns, then we don't column row indices when 'row = true'.
-                if ((!Operation_::zero_depends_on_row && !row) || (!Operation_::zero_depends_on_column && row)) {
+                if (DelayedIsometricOperation_internal::can_dense_expand(my_operation, row)) {
                     return dense_expanded_internal<oracle_>(row, std::forward<Args_>(args)...);
                 }
             }
@@ -682,7 +657,7 @@ class DelayedBinaryIsometricOperation : public Matrix<Value_, Index_> {
 private:
     template<bool oracle_>
     std::unique_ptr<SparseExtractor<oracle_, Value_, Index_> > sparse_internal(bool row, MaybeOracle<oracle_, Index_> oracle, const Options& opt) const {
-        if constexpr(is_advanced) {
+        if constexpr(!Operation_::is_basic) {
             if (my_is_sparse) {
                 return std::make_unique<DelayedBinaryIsometricOperation_internal::Sparse<oracle_, Value_, Index_, Operation_> >(
                     my_left.get(),
@@ -704,7 +679,7 @@ class DelayedBinaryIsometricOperation : public Matrix<Value_, Index_> {
 
     template<bool oracle_>
     std::unique_ptr<SparseExtractor<oracle_, Value_, Index_> > sparse_internal(bool row, MaybeOracle<oracle_, Index_> oracle, Index_ block_start, Index_ block_length, const Options& opt) const {
-        if constexpr(is_advanced) {
+        if constexpr(!Operation_::is_basic) {
             if (my_is_sparse) {
                 return std::make_unique<DelayedBinaryIsometricOperation_internal::Sparse<oracle_, Value_, Index_, Operation_> >(
                     my_left.get(),
@@ -729,7 +704,7 @@ class DelayedBinaryIsometricOperation : public Matrix<Value_, Index_> {
 
     template<bool oracle_>
     std::unique_ptr<SparseExtractor<oracle_, Value_, Index_> > sparse_internal(bool row, MaybeOracle<oracle_, Index_> oracle, VectorPtr<Index_> indices_ptr, const Options& opt) const {
-        if constexpr(is_advanced) {
+        if constexpr(!Operation_::is_basic) {
             if (my_is_sparse) {
                 return std::make_unique<DelayedBinaryIsometricOperation_internal::Sparse<oracle_, Value_, Index_, Operation_> >(
                     my_left.get(),

diff --git a/include/tatami/isometric/binary/arithmetic_helpers.hpp b/include/tatami/isometric/binary/arithmetic_helpers.hpp
@@ -31,9 +31,7 @@ class DelayedBinaryIsometricArithmetic {
                                           op_ == ArithmeticOperation::SUBTRACT ||
                                           op_ == ArithmeticOperation::MULTIPLY);
 
-    static constexpr bool zero_depends_on_row = false;
-
-    static constexpr bool zero_depends_on_column = false;
+    static constexpr bool is_basic = false;
     /**
      * @endcond
      */
@@ -72,7 +70,7 @@ class DelayedBinaryIsometricArithmetic {
     }
 
     template<typename Value_, typename Index_>
-    Value_ fill(Index_) const {
+    Value_ fill(bool, Index_) const {
         if constexpr(known_sparse) {
             return 0;
         } else if constexpr(op_ == ArithmeticOperation::POWER) {

diff --git a/include/tatami/isometric/binary/boolean_helpers.hpp b/include/tatami/isometric/binary/boolean_helpers.hpp
@@ -28,9 +28,7 @@ struct DelayedBinaryIsometricBoolean {
     // It's sparse if f(0, 0) == 0.
     static constexpr bool known_sparse = (op_ != BooleanOperation::EQUAL);
 
-    static constexpr bool zero_depends_on_row = false;
-
-    static constexpr bool zero_depends_on_column = false;
+    static constexpr bool is_basic = false;
     /**
      * @endcond
      */
@@ -69,7 +67,7 @@ struct DelayedBinaryIsometricBoolean {
     }
 
     template<typename Value_, typename Index_>
-    Value_ fill(Index_) const {
+    Value_ fill(bool, Index_) const {
         if constexpr(known_sparse) {
             return 0;
         } else {

diff --git a/include/tatami/isometric/binary/compare_helpers.hpp b/include/tatami/isometric/binary/compare_helpers.hpp
@@ -30,9 +30,7 @@ struct DelayedBinaryIsometricCompare {
                                           op_ != CompareOperation::GREATER_THAN_OR_EQUAL && 
                                           op_ != CompareOperation::LESS_THAN_OR_EQUAL);
 
-    static constexpr bool zero_depends_on_row = false;
-
-    static constexpr bool zero_depends_on_column = false;
+    static constexpr bool is_basic = false;
     /**
      * @endcond
      */
@@ -76,7 +74,7 @@ struct DelayedBinaryIsometricCompare {
     }
 
     template<typename Value_, typename Index_>
-    Value_ fill(Index_) const {
+    Value_ fill(bool, Index_) const {
         if constexpr(known_sparse) {
             return 0;
         } else {