More comprehensive tests against any assumption of zeroed arrays.

We use a test-only macro to insert a non-zero initial value that changes
with each invocation. So, any reliance on a zeroed input array would
manifest as different results across repeated calls in the test suite.
In effect, all tests are now checking against "dirty" input buffers,
eliminating the need for dedicated tests with less coverage.

.github/workflows/run-tests.yaml

@@ -19,10 +19,14 @@ jobs:
             os: ubuntu-latest,
             cov: true
           }
+        - {
+            name: "Ubuntu Latest GCC, dirty initialization", 
+            os: ubuntu-latest,
+            dirty: true
+          }
         - {
             name: "macOS Latest Clang", 
-            os: macos-latest,
-            cov: false
+            os: macos-latest
           }
 
     steps:
@@ -35,6 +39,10 @@ jobs:
       if: ${{ matrix.config.cov }}
       run: cmake -S . -B build -DCODE_COVERAGE=ON
 
+    - name: Configure the build with coverage
+      if: ${{ matrix.config.dirty }}
+      run: cmake -S . -B build -DTEST_DIRTY_INIT=ON
+
     - name: Configure the build with custom parallelization
       if: ${{ ! matrix.config.cov }}
       run: cmake -S . -B build

include/scran_aggregate/aggregate_across_cells.hpp

@@ -317,15 +317,23 @@ AggregateAcrossCellsResults<Sum_, Detected_> aggregate_across_cells(
     AggregateAcrossCellsBuffers<Sum_, Detected_> buffers;
 
     if (options.compute_sums) {
-        output.sums.resize(nlevels, std::vector<Sum_>(ngenes));
+        output.sums.resize(nlevels, std::vector<Sum_>(ngenes
+#ifdef SCRAN_AGGREGATE_TEST_INIT
+        , SCRAN_AGGREGATE_TEST_INIT
+#endif
+        ));
         buffers.sums.resize(nlevels);
         for (size_t l = 0; l < nlevels; ++l) {
             buffers.sums[l] = output.sums[l].data();
         }
     }
 
     if (options.compute_detected) {
-        output.detected.resize(nlevels, std::vector<Detected_>(ngenes));
+        output.detected.resize(nlevels, std::vector<Detected_>(ngenes
+#ifdef SCRAN_AGGREGATE_TEST_INIT
+        , SCRAN_AGGREGATE_TEST_INIT
+#endif
+        ));
         buffers.detected.resize(nlevels);
         for (size_t l = 0; l < nlevels; ++l) {
             buffers.detected[l] = output.detected[l].data();

include/scran_aggregate/aggregate_across_genes.hpp

@@ -337,7 +337,11 @@ AggregateAcrossGenesResults<Sum_> aggregate_across_genes(
     buffers.sum.resize(nsets);
 
     for (size_t s = 0; s < nsets; ++s) {
-        output.sum[s].resize(NC);
+        output.sum[s].resize(NC
+#ifdef SCRAN_AGGREGATE_TEST_INIT
+        , SCRAN_AGGREGATE_TEST_INIT
+#endif
+        );
         buffers.sum[s] = output.sum[s].data();
     }
 

tests/CMakeLists.txt

@@ -28,5 +28,11 @@ if(CODE_COVERAGE AND CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
     target_link_options(libtest PRIVATE --coverage)
 endif()
 
+option(TEST_DIRTY_INIT "Test dirty vector initialization" OFF)
+if(TEST_DIRTY_INIT)
+    target_compile_definitions(dirtytest PRIVATE "SCRAN_AGGREGATE_TEST_INIT=initial_value()")
+endif()
+
 include(GoogleTest)
 gtest_discover_tests(libtest)
+gtest_discover_tests(dirtytest)

tests/src/aggregate_across_cells.cpp

@@ -1,11 +1,11 @@
-#include <gtest/gtest.h>
-
-#include "scran_aggregate/aggregate_across_cells.hpp"
 #include "scran_tests/scran_tests.hpp"
 
 #include <map>
 #include <random>
 
+#include "utils.h" // must be included before scran_aggregate. 
+#include "scran_aggregate/aggregate_across_cells.hpp"
+
 static std::vector<int> create_groupings(size_t n, int ngroups) {
     std::vector<int> groupings(n);
     for (size_t g = 0; g < groupings.size(); ++g) {
@@ -116,51 +116,3 @@ TEST(AggregateAcrossCells, Skipping) {
     EXPECT_EQ(skipped.sums.size(), 0);
     EXPECT_EQ(skipped.detected.size(), 0);
 }
-
-TEST(AggregateAcrossCells, DirtyBuffers) {
-    int nr = 88, nc = 126;
-    auto vec = scran_tests::simulate_vector(nr * nc, []{
-        scran_tests::SimulationParameters sparams;
-        sparams.density = 0.1;
-        sparams.seed = 69;
-        return sparams;
-    }());
-
-    tatami::DenseRowMatrix<double, int> dense_row(nr, nc, std::move(vec));
-    auto sparse_column = tatami::convert_to_compressed_sparse(&dense_row, false);
-    size_t ngroups = 3;
-    auto grouping = create_groupings(dense_row.ncol(), ngroups);
-
-    // Setting up some dirty buffers.
-    scran_aggregate::AggregateAcrossCellsResults<double, int> store;
-    scran_aggregate::AggregateAcrossCellsBuffers<double, int> buffers;
-    store.sums.resize(ngroups, std::vector<double>(nr, -1));
-    store.detected.resize(ngroups, std::vector<int>(nr, -1));
-    buffers.sums.resize(ngroups);
-    buffers.detected.resize(ngroups);
-    for (size_t l = 0; l < ngroups; ++l) {
-        buffers.sums[l] = store.sums[l].data();
-        buffers.detected[l] = store.detected[l].data();
-    }
-
-    scran_aggregate::AggregateAcrossCellsOptions opt;
-    auto ref = scran_aggregate::aggregate_across_cells(dense_row, grouping.data(), opt);
-    scran_aggregate::aggregate_across_cells(dense_row, grouping.data(), buffers, opt);
-    EXPECT_EQ(ref.sums.size(), store.sums.size());
-    EXPECT_EQ(ref.detected.size(), store.detected.size());
-    for (size_t l = 0; l < ngroups; ++l) {
-        EXPECT_EQ(ref.sums[l], store.sums[l]);
-        EXPECT_EQ(ref.detected[l], store.detected[l]);
-    }
-
-    // Same for column-major iteration.
-    for (size_t l = 0; l < ngroups; ++l) {
-        std::fill_n(buffers.sums[l], nr, -1);
-        std::fill_n(buffers.detected[l], nr, -1);
-    }
-    scran_aggregate::aggregate_across_cells(*sparse_column, grouping.data(), buffers, opt);
-    for (size_t l = 0; l < ngroups; ++l) {
-        EXPECT_EQ(ref.sums[l], store.sums[l]);
-        EXPECT_EQ(ref.detected[l], store.detected[l]);
-    }
-}

tests/src/aggregate_across_genes.cpp

@@ -1,25 +1,10 @@
-#include <gtest/gtest.h>
-
-#include "scran_aggregate/aggregate_across_genes.hpp"
 #include "scran_tests/scran_tests.hpp"
+
 #include <map>
 #include <random>
 
-static std::vector<std::vector<int> > create_sets(size_t nsets, int ngenes, size_t seed) {
-    std::vector<std::vector<int> > groupings(nsets);
-    std::mt19937_64 rng(seed);
-    std::uniform_real_distribution runif;
-    for (auto& grp : groupings) {
-        for (int g = 0; g < ngenes; ++g) {
-            if (runif(rng) < 0.15) {
-                grp.push_back(g);
-            }
-        }
-    }
-    return groupings;
-}
-
-/*********************************************/
+#include "utils.h" // must be included before scran_aggregate
+#include "scran_aggregate/aggregate_across_genes.hpp"
 
 class AggregateAcrossGenesTest : public ::testing::TestWithParam<int> {
 protected:
@@ -45,10 +30,19 @@ TEST_P(AggregateAcrossGenesTest, Unweighted) {
 
     const size_t nsets = 100;
     int ngenes = dense_row->nrow();
-    auto mock_sets = create_sets(nsets, ngenes, /* seed = */ nsets * nthreads);
+    std::vector<std::vector<int> > mock_sets(nsets);
+    std::mt19937_64 rng(nsets * nthreads);
+    std::uniform_real_distribution runif;
+    for (auto& grp : mock_sets) {
+        for (int g = 0; g < ngenes; ++g) {
+            if (runif(rng) < 0.15) {
+                grp.push_back(g);
+            }
+        }
+    }
 
     std::vector<std::tuple<size_t, const int*, const double*> > gene_sets;
-    gene_sets.reserve(mock_sets.size());
+    gene_sets.reserve(nsets);
     for (const auto& grp : mock_sets) {
         gene_sets.emplace_back(grp.size(), grp.data(), static_cast<double*>(NULL));
     }
@@ -204,51 +198,3 @@ TEST(AggregateAcrossGenes, OutOfRange) {
         scran_aggregate::aggregate_across_genes(mat, gene_sets, opt);
     }, "out of range");
 }
-
-TEST(AggregateAcrossGenes, DirtyBuffers) {
-    int nr = 110, nc = 78;
-    auto vec = scran_tests::simulate_vector(nr * nc, []{
-        scran_tests::SimulationParameters sparams;
-        sparams.density = 0.1;
-        return sparams;
-    }());
-
-    tatami::DenseRowMatrix<double, int> dense_row(nr, nc, std::move(vec));
-    auto sparse_column = tatami::convert_to_compressed_sparse(&dense_row, false);
-
-    // Setting up the gene sets.
-    size_t nsets = 100;
-    auto mock_sets = create_sets(nsets, nr, /* seed = */ 99);
-
-    std::vector<std::tuple<size_t, const int*, const double*> > gene_sets;
-    gene_sets.reserve(mock_sets.size());
-    for (const auto& grp : mock_sets) {
-        gene_sets.emplace_back(grp.size(), grp.data(), static_cast<double*>(NULL));
-    }
-
-    // Setting up some buffers.
-    scran_aggregate::AggregateAcrossGenesResults<double> store;
-    scran_aggregate::AggregateAcrossGenesBuffers<double> buffers;
-    store.sum.resize(nsets);
-    buffers.sum.resize(nsets);
-    for (size_t s = 0; s < nsets; ++s) {
-        store.sum[s].resize(nc, -1); // using -1 to simulate uninitialized values.
-        buffers.sum[s] = store.sum[s].data();
-    }
-
-    // Checking that the dirtiness doesn't affect the results.
-    scran_aggregate::AggregateAcrossGenesOptions opt;
-    auto ref = scran_aggregate::aggregate_across_genes(dense_row, gene_sets, opt);
-    scran_aggregate::aggregate_across_genes(dense_row, gene_sets, buffers, opt);
-    for (size_t s = 0; s < nsets; ++s) {
-        EXPECT_EQ(ref.sum[s], store.sum[s]);
-    }
-
-    for (size_t s = 0; s < nsets; ++s) {
-        std::fill_n(store.sum[s].data(), nc, -1); // dirtying it again for another run.
-    }
-    scran_aggregate::aggregate_across_genes(*sparse_column, gene_sets, buffers, opt);
-    for (size_t s = 0; s < nsets; ++s) {
-        EXPECT_EQ(ref.sum[s], store.sum[s]);
-    }
-}

tests/src/clean_factor.cpp

@@ -2,12 +2,16 @@
 
 #include <random>
 
+#include "utils.h" // must be included before scran_aggregate
 #include "scran_aggregate/clean_factor.hpp"
-#include "scran_aggregate/combine_factors.hpp"
 
 template<typename Factor_>
 std::pair<std::vector<Factor_>, std::vector<int> > test_clean_factor(size_t n, const Factor_* factor) {
-    std::vector<int> cleand(n);
+    std::vector<int> cleand(n
+#ifdef SCRAN_AGGREGATE_TEST_INIT
+        , SCRAN_AGGREGATE_TEST_INIT
+#endif
+    );
     auto levels = scran_aggregate::clean_factor(n, factor, cleand.data());
     return std::make_pair(std::move(levels), std::move(cleand));
 }

tests/src/combine_factors.cpp

@@ -2,11 +2,16 @@
 
 #include <random>
 
+#include "utils.h" // must be included before scran_aggregate
 #include "scran_aggregate/combine_factors.hpp"
 
 template<typename Factor_>
 std::pair<std::vector<std::vector<Factor_> >, std::vector<int> > test_combine_factors(size_t n, const std::vector<const Factor_*>& factors) {
-    std::vector<int> combined(n);
+    std::vector<int> combined(n
+#ifdef SCRAN_AGGREGATE_TEST_INIT
+        , SCRAN_AGGREGATE_TEST_INIT
+#endif
+    );
     auto levels = scran_aggregate::combine_factors(n, factors, combined.data());
     return std::make_pair(std::move(levels), std::move(combined));
 }
@@ -131,7 +136,11 @@ TEST(CombineFactors, Multiple) {
 
 template<typename Factor_, typename Number_>
 std::pair<std::vector<std::vector<Factor_> >, std::vector<int> > test_combine_factors_unused(size_t n, const std::vector<std::pair<const Factor_*, Number_> >& factors) {
-    std::vector<int> combined(n);
+    std::vector<int> combined(n
+#ifdef SCRAN_AGGREGATE_TEST_INIT
+        , SCRAN_AGGREGATE_TEST_INIT
+#endif
+    );
     auto levels = scran_aggregate::combine_factors_unused(n, factors, combined.data());
     return std::make_pair(std::move(levels), std::move(combined));
 }

tests/src/utils.h

@@ -0,0 +1,19 @@
+#ifndef UTILS_H
+#define UTILS_H
+
+// This generates a different initial value for each vector() that might store results.
+// The aim is to check that our functions ignore the initial contents of each output array.
+// Otherwise, we might miss subtle bugs from an incorrect assumption of default-initialized contents.
+// We make the values vary across calls rather than using a constant,
+// as this allows us to catch bugs via tests that check for consistency between calls.
+inline int initial_value() {
+    static int counter = 0;
+    if (counter == 255) {
+        counter = 1;
+    } else {
+        ++counter;
+    }
+    return counter;
+}
+
+#endif