diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 433a578..25ff7df 100644 --- a/.github/workflows/run-tests.yaml +++ b/.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 dirty initialization + 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 diff --git a/include/scran_aggregate/aggregate_across_cells.hpp b/include/scran_aggregate/aggregate_across_cells.hpp index 99ac4d6..391d597 100644 --- a/include/scran_aggregate/aggregate_across_cells.hpp +++ b/include/scran_aggregate/aggregate_across_cells.hpp @@ -317,7 +317,11 @@ AggregateAcrossCellsResults aggregate_across_cells( AggregateAcrossCellsBuffers buffers; if (options.compute_sums) { - output.sums.resize(nlevels, std::vector(ngenes)); + output.sums.resize(nlevels, std::vector(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(); @@ -325,7 +329,11 @@ AggregateAcrossCellsResults aggregate_across_cells( } if (options.compute_detected) { - output.detected.resize(nlevels, std::vector(ngenes)); + output.detected.resize(nlevels, std::vector(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(); diff --git a/include/scran_aggregate/aggregate_across_genes.hpp b/include/scran_aggregate/aggregate_across_genes.hpp index 3ad0c83..1ce52de 100644 --- a/include/scran_aggregate/aggregate_across_genes.hpp +++ b/include/scran_aggregate/aggregate_across_genes.hpp @@ -337,7 +337,11 @@ AggregateAcrossGenesResults 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(); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 694c7cf..ca2e6eb 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -28,5 +28,10 @@ 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) diff --git a/tests/src/aggregate_across_cells.cpp b/tests/src/aggregate_across_cells.cpp index 9296eed..194477e 100644 --- a/tests/src/aggregate_across_cells.cpp +++ b/tests/src/aggregate_across_cells.cpp @@ -1,11 +1,11 @@ -#include - -#include "scran_aggregate/aggregate_across_cells.hpp" #include "scran_tests/scran_tests.hpp" #include #include +#include "utils.h" // must be included before scran_aggregate. +#include "scran_aggregate/aggregate_across_cells.hpp" + static std::vector create_groupings(size_t n, int ngroups) { std::vector 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 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 store; - scran_aggregate::AggregateAcrossCellsBuffers buffers; - store.sums.resize(ngroups, std::vector(nr, -1)); - store.detected.resize(ngroups, std::vector(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]); - } -} diff --git a/tests/src/aggregate_across_genes.cpp b/tests/src/aggregate_across_genes.cpp index 47dc79d..c75b454 100644 --- a/tests/src/aggregate_across_genes.cpp +++ b/tests/src/aggregate_across_genes.cpp @@ -1,25 +1,10 @@ -#include - -#include "scran_aggregate/aggregate_across_genes.hpp" #include "scran_tests/scran_tests.hpp" + #include #include -static std::vector > create_sets(size_t nsets, int ngenes, size_t seed) { - std::vector > 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 { 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 > 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 > 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(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 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 > gene_sets; - gene_sets.reserve(mock_sets.size()); - for (const auto& grp : mock_sets) { - gene_sets.emplace_back(grp.size(), grp.data(), static_cast(NULL)); - } - - // Setting up some buffers. - scran_aggregate::AggregateAcrossGenesResults store; - scran_aggregate::AggregateAcrossGenesBuffers 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]); - } -} diff --git a/tests/src/clean_factor.cpp b/tests/src/clean_factor.cpp index a528725..8c084df 100644 --- a/tests/src/clean_factor.cpp +++ b/tests/src/clean_factor.cpp @@ -2,12 +2,16 @@ #include +#include "utils.h" // must be included before scran_aggregate #include "scran_aggregate/clean_factor.hpp" -#include "scran_aggregate/combine_factors.hpp" template std::pair, std::vector > test_clean_factor(size_t n, const Factor_* factor) { - std::vector cleand(n); + std::vector 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)); } diff --git a/tests/src/combine_factors.cpp b/tests/src/combine_factors.cpp index a62511f..31388c5 100644 --- a/tests/src/combine_factors.cpp +++ b/tests/src/combine_factors.cpp @@ -2,11 +2,16 @@ #include +#include "utils.h" // must be included before scran_aggregate #include "scran_aggregate/combine_factors.hpp" template std::pair >, std::vector > test_combine_factors(size_t n, const std::vector& factors) { - std::vector combined(n); + std::vector 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 std::pair >, std::vector > test_combine_factors_unused(size_t n, const std::vector >& factors) { - std::vector combined(n); + std::vector 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)); } diff --git a/tests/src/utils.h b/tests/src/utils.h new file mode 100644 index 0000000..732a570 --- /dev/null +++ b/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