From be7b47555dd71148d99bdd76c20dc80b5c6e3e13 Mon Sep 17 00:00:00 2001
From: Nigel Delaney <ndunofficial@gmail.com>
Date: Fri, 25 May 2018 11:46:59 -0700
Subject: [PATCH] coeffRef(it.row(), it.col()) -> it.valueRef()

Minor performance optimization.  In a few of the edited places, the code is iterating over the elements of a sparse matrix using an iterator.  However, when it came time to update the sparsematrix at the given position, rather then directly assign the value, the code called `coeffRef` to assign it.

This is inefficient because `coeffRef` involves an expensive binary search to find position `i,j` in the sparse matrix, even though the memory location of i,j is already known to the iterator on the stack.  To directly use the known value, we can just call .valueRef() on the iterator instead of looking for the position again.

A discussion of this difference is given in the `Iterating over the nonzero coefficients` section of this tutorial: https://eigen.tuxfamily.org/dox/group__TutorialSparse.html

Benchmark showed a modest 13% time reduction in a call to NormalizeData.
---
 src/data_manipulation.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/data_manipulation.cpp b/src/data_manipulation.cpp
index f44eb760b..db45a8282 100644
--- a/src/data_manipulation.cpp
+++ b/src/data_manipulation.cpp
@@ -24,14 +24,14 @@ Eigen::SparseMatrix<double> RunUMISampling(Eigen::SparseMatrix<double> data, int
           if (fmod(entry, 1) != 0){
             double rn = runif(1)[0];
             if(fmod(entry, 1) <= rn){
-              data.coeffRef(it.row(), it.col()) = floor(entry);
+              it.valueRef() = floor(entry);
             }
             else{
-              data.coeffRef(it.row(), it.col()) = ceil(entry);
+              it.valueRef() = ceil(entry);
             }
           }
           else{
-            data.coeffRef(it.row(), it.col()) = entry;
+            it.valueRef() = entry;
           }
         }
       }
@@ -52,14 +52,14 @@ Eigen::SparseMatrix<double> RunUMISamplingPerCell(Eigen::SparseMatrix<double> da
         if (fmod(entry, 1) != 0){
           double rn = runif(1)[0];
           if(fmod(entry, 1) <= rn){
-            data.coeffRef(it.row(), it.col()) = floor(entry);
+            it.valueRef() = floor(entry);
           }
           else{
-            data.coeffRef(it.row(), it.col()) = ceil(entry);
+            it.valueRef() = ceil(entry);
           }
         }
         else{
-          data.coeffRef(it.row(), it.col()) = entry;
+          it.valueRef() = entry;
         }
       }
     }
@@ -117,7 +117,7 @@ Eigen::SparseMatrix<double> LogNorm(Eigen::SparseMatrix<double> data, int scale_
   for (int k=0; k < data.outerSize(); ++k){
     p.increment();
     for (Eigen::SparseMatrix<double>::InnerIterator it(data, k); it; ++it){
-      data.coeffRef(it.row(), it.col()) = log1p(double(it.value()) / colSums[k] * scale_factor);
+      it.valueRef() = log1p(double(it.value()) / colSums[k] * scale_factor);
     }
   }
   return data;