From 9d0e74d118993a11498a73b18c616087af5720d9 Mon Sep 17 00:00:00 2001 From: Hugh Pendry Date: Thu, 18 Apr 2024 19:26:32 +0000 Subject: [PATCH] Merge branch 'DOR-662_mm2_optional_options' into 'master' DOR-662 Minimap2Options fields converted to std::optional Closes DOR-662 See merge request machine-learning/dorado!957 (cherry picked from commit 4eee9969d9cbe4268f0834ebdb4b6f3007741f43) 90bb19e3 Changed Minimap2Options fields to be std::optional and only apply them if... f342796e Merge remote-tracking branch 'origin/master' into DOR-662_mm2_optional_options 8efd78f6 Update following review 4e207aa4 Resolved merge conflict ee94c79c Updated generate_sequence_records_header to create an index entry if missing... --- dorado/alignment/IndexFileAccess.cpp | 7 +-- dorado/alignment/IndexFileAccess.h | 5 +- dorado/alignment/Minimap2Index.cpp | 25 ++++---- dorado/alignment/Minimap2Options.h | 33 ++++++----- dorado/cli/aligner.cpp | 4 +- dorado/cli/basecaller.cpp | 4 +- dorado/cli/cli_utils.h | 88 ++++++++++++++++------------ dorado/cli/duplex.cpp | 4 +- tests/AlignerTest.cpp | 4 +- tests/IndexFileAccessTest.cpp | 49 +++++++++------- tests/Minimap2IndexTest.cpp | 42 ++++++++----- 11 files changed, 151 insertions(+), 114 deletions(-) diff --git a/dorado/alignment/IndexFileAccess.cpp b/dorado/alignment/IndexFileAccess.cpp index e36e9ac8..eb91580a 100644 --- a/dorado/alignment/IndexFileAccess.cpp +++ b/dorado/alignment/IndexFileAccess.cpp @@ -111,10 +111,9 @@ bool IndexFileAccess::is_index_loaded(const std::string& index_file, return is_index_loaded_impl(index_file, options); } -std::string IndexFileAccess::generate_sequence_records_header( - const std::string& index_file, - const Minimap2Options& options) const { - auto loaded_index = get_exact_index(index_file, options); +std::string IndexFileAccess::generate_sequence_records_header(const std::string& index_file, + const Minimap2Options& options) { + auto loaded_index = get_index(index_file, options); assert(loaded_index && "Index must be loaded to generate header records"); auto sequence_records = loaded_index->get_sequence_records_for_header(); diff --git a/dorado/alignment/IndexFileAccess.h b/dorado/alignment/IndexFileAccess.h index 9db98fb0..7670576b 100644 --- a/dorado/alignment/IndexFileAccess.h +++ b/dorado/alignment/IndexFileAccess.h @@ -58,8 +58,11 @@ class IndexFileAccess { void unload_index(const std::string& index_file, const Minimap2IndexOptions& index_options); // returns a string containing the sequence records for the requested index + // Note, if not loaded will create an index from an existing compatible one. + // By contract there must be a loaded index for the index file with matching indexing + // options, if not there will be an assertion failure. std::string generate_sequence_records_header(const std::string& index_file, - const Minimap2Options& options) const; + const Minimap2Options& options); // Testability. Method needed to support utests bool is_index_loaded(const std::string& index_file, const Minimap2Options& options) const; diff --git a/dorado/alignment/Minimap2Index.cpp b/dorado/alignment/Minimap2Index.cpp index 008ffe90..4dfa48a5 100644 --- a/dorado/alignment/Minimap2Index.cpp +++ b/dorado/alignment/Minimap2Index.cpp @@ -33,33 +33,36 @@ IndexReaderPtr create_index_reader(const std::string& index_file, namespace dorado::alignment { void Minimap2Index::set_index_options(const Minimap2IndexOptions& index_options) { - m_index_options->k = index_options.kmer_size; - m_index_options->w = index_options.window_size; + m_index_options->k = index_options.kmer_size.value_or(m_index_options->k); + m_index_options->w = index_options.window_size.value_or(m_index_options->w); spdlog::trace("> Index parameters input by user: kmer size={} and window size={}.", m_index_options->k, m_index_options->w); - m_index_options->batch_size = index_options.index_batch_size; - m_index_options->mini_batch_size = index_options.index_batch_size; + //if not specified override the preset value of 8000000000 with 16000000000 + m_index_options->batch_size = index_options.index_batch_size.value_or(16000000000); + m_index_options->mini_batch_size = m_index_options->batch_size; spdlog::trace("> Index parameters input by user: batch size={} and mini batch size={}.", m_index_options->batch_size, m_index_options->mini_batch_size); } void Minimap2Index::set_mapping_options(const Minimap2MappingOptions& mapping_options) { - m_mapping_options->bw = mapping_options.bandwidth; - m_mapping_options->bw_long = mapping_options.bandwidth_long; - spdlog::trace("> Map parameters input by user: bandwidth={} and bandwidth long={}.", - m_mapping_options->bw, m_mapping_options->bw_long); + m_mapping_options->bw = mapping_options.bandwidth.value_or(m_mapping_options->bw); + m_mapping_options->bw_long = + mapping_options.bandwidth_long.value_or(m_mapping_options->bw_long); + spdlog::trace("> Map parameters: bandwidth={} and bandwidth long={}.", m_mapping_options->bw, + m_mapping_options->bw_long); - if (!mapping_options.print_secondary) { + if (!mapping_options.print_secondary.value_or(true)) { m_mapping_options->flag |= MM_F_NO_PRINT_2ND; } - m_mapping_options->best_n = mapping_options.best_n_secondary; + m_mapping_options->best_n = + mapping_options.best_n_secondary.value_or(m_mapping_options->best_n); spdlog::trace( "> Map parameters input by user: don't print secondary={} and best n secondary={}.", static_cast(m_mapping_options->flag & MM_F_NO_PRINT_2ND), m_mapping_options->best_n); - if (mapping_options.soft_clipping) { + if (mapping_options.soft_clipping.value_or(false)) { m_mapping_options->flag |= MM_F_SOFTCLIP; } if (mapping_options.secondary_seq) { diff --git a/dorado/alignment/Minimap2Options.h b/dorado/alignment/Minimap2Options.h index aebfd530..c6f518e8 100644 --- a/dorado/alignment/Minimap2Options.h +++ b/dorado/alignment/Minimap2Options.h @@ -1,16 +1,17 @@ #pragma once #include +#include #include #include namespace dorado::alignment { struct Minimap2IndexOptions { - short kmer_size; - short window_size; - uint64_t index_batch_size; - std::string mm2_preset; + std::optional kmer_size; + std::optional window_size; + std::optional index_batch_size; + std::string mm2_preset; // By default we use a preset, hence not an optional }; inline bool operator<(const Minimap2IndexOptions& l, const Minimap2IndexOptions& r) { @@ -40,12 +41,12 @@ inline bool operator!=(const Minimap2IndexOptions& l, const Minimap2IndexOptions } struct Minimap2MappingOptions { - int best_n_secondary; - int bandwidth; - int bandwidth_long; - bool soft_clipping; - bool secondary_seq; - bool print_secondary; + std::optional best_n_secondary; + std::optional bandwidth; + std::optional bandwidth_long; + std::optional soft_clipping; + bool secondary_seq; // Not available to be set by the user, hence not optional + std::optional print_secondary; }; inline bool operator<(const Minimap2MappingOptions& l, const Minimap2MappingOptions& r) { @@ -79,7 +80,7 @@ inline bool operator!=(const Minimap2MappingOptions& l, const Minimap2MappingOpt } struct Minimap2Options : public Minimap2IndexOptions, public Minimap2MappingOptions { - bool print_aln_seq; + bool print_aln_seq; // Not available to be set by the user, hence not optional }; inline bool operator==(const Minimap2Options& l, const Minimap2Options& r) { @@ -89,7 +90,11 @@ inline bool operator==(const Minimap2Options& l, const Minimap2Options& r) { inline bool operator!=(const Minimap2Options& l, const Minimap2Options& r) { return !(l == r); } -static const Minimap2Options dflt_options{{19, 19, 16000000000ull, "lr:hq"}, - {5, 500, 20000, false, false, true}, - false}; +static const std::string DEFAULT_MM_PRESET{"lr:hq"}; + +static const Minimap2Options dflt_options{ + {std::nullopt, std::nullopt, std::nullopt, DEFAULT_MM_PRESET}, + {std::nullopt, std::nullopt, std::nullopt, std::nullopt, false, std::nullopt}, + false}; + } // namespace dorado::alignment diff --git a/dorado/cli/aligner.cpp b/dorado/cli/aligner.cpp index 373fa583..b6e8cc50 100644 --- a/dorado/cli/aligner.cpp +++ b/dorado/cli/aligner.cpp @@ -132,7 +132,7 @@ int aligner(int argc, char* argv[]) { .action([&](const auto&) { ++verbosity; }) .append(); - cli::add_minimap2_arguments(parser, alignment::dflt_options); + cli::add_minimap2_arguments(parser, alignment::DEFAULT_MM_PRESET); try { cli::parse(parser, argc, argv); @@ -169,7 +169,7 @@ int aligner(int argc, char* argv[]) { auto threads(parser.visible.get("threads")); auto max_reads(parser.visible.get("max-reads")); - auto options = cli::process_minimap2_arguments(parser, alignment::dflt_options); + auto options = cli::process_minimap2_arguments(parser); alignment::AlignmentProcessingItems processing_items{reads, recursive_input, output_folder, false}; diff --git a/dorado/cli/basecaller.cpp b/dorado/cli/basecaller.cpp index 8956fa68..9ab310e5 100644 --- a/dorado/cli/basecaller.cpp +++ b/dorado/cli/basecaller.cpp @@ -500,7 +500,7 @@ int basecaller(int argc, char* argv[]) { .help("Configuration file for PolyA estimation to change default behaviours") .default_value(std::string("")); - cli::add_minimap2_arguments(parser, alignment::dflt_options); + cli::add_minimap2_arguments(parser, alignment::DEFAULT_MM_PRESET); cli::add_internal_arguments(parser); // Create a copy of the parser to use if the resume feature is enabled. Needed @@ -673,7 +673,7 @@ int basecaller(int argc, char* argv[]) { parser.visible.get("--emit-moves"), parser.visible.get("--max-reads"), parser.visible.get("--min-qscore"), parser.visible.get("--read-ids"), recursive, - cli::process_minimap2_arguments(parser, alignment::dflt_options), + cli::process_minimap2_arguments(parser), parser.hidden.get("--skip-model-compatibility-check"), parser.hidden.get("--dump_stats_file"), parser.hidden.get("--dump_stats_filter"), diff --git a/dorado/cli/cli_utils.h b/dorado/cli/cli_utils.h index 41e70c3a..da29e38a 100644 --- a/dorado/cli/cli_utils.h +++ b/dorado/cli/cli_utils.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -182,46 +183,36 @@ inline void add_internal_arguments(ArgParser& parser) { .default_value(std::string("")); } -template -void add_minimap2_arguments(ArgParser& parser, const Options& dflt) { +inline void add_minimap2_arguments(ArgParser& parser, const std::string& default_preset) { parser.visible.add_argument("-k") .help("minimap2 k-mer size for alignment (maximum 28).") - .template default_value(dflt.kmer_size) .template scan<'i', int>(); parser.visible.add_argument("-w") .help("minimap2 minimizer window size for alignment.") - .template default_value(dflt.window_size) .template scan<'i', int>(); - parser.visible.add_argument("-I") - .help("minimap2 index batch size.") - .default_value(to_size(double(dflt.index_batch_size))); + parser.visible.add_argument("-I").help("minimap2 index batch size."); - parser.visible.add_argument("--secondary") - .help("minimap2 outputs secondary alignments") - .default_value(to_yes_or_no(dflt.print_secondary)); + parser.visible.add_argument("--secondary").help("minimap2 outputs secondary alignments"); parser.visible.add_argument("-N") .help("minimap2 retains at most INT secondary alignments") - .default_value(dflt.best_n_secondary) .template scan<'i', int>(); parser.visible.add_argument("-Y") .help("minimap2 uses soft clipping for supplementary alignments") - .default_value(false) .implicit_value(true); parser.visible.add_argument("--bandwidth") .help("minimap2 chaining/alignment bandwidth and optionally long-join bandwidth " - "specified as NUM,[NUM]") - .default_value(to_size(dflt.bandwidth) + "," + to_size(dflt.bandwidth_long)); + "specified as NUM,[NUM]"); // Setting options to lr:hq which is appropriate for high quality nanopore reads. parser.visible.add_argument("--mm2-preset") .help("minimap2 preset for indexing and mapping. Alias for the -x " "option in minimap2.") - .default_value(dflt.mm2_preset); + .default_value(default_preset); parser.hidden.add_argument("--secondary-seq") .help("minimap2 output seq/qual for secondary and supplementary alignments") @@ -244,33 +235,52 @@ inline void parse(ArgParser& parser, int argc, const char* const argv[]) { utils::details::extract_dev_options(parser.hidden.get("--devopts")); } +template +std::optional get_optional_as(const std::optional& from_optional) { + if (from_optional) { + return std::make_optional(static_cast(*from_optional)); + } else { + return std::nullopt; + } +} + template -Options process_minimap2_arguments(const ArgParser& parser, const Options& dflt) { - Options res; - res.kmer_size = short(parser.visible.get("k")); - res.window_size = short(parser.visible.get("w")); - res.index_batch_size = cli::parse_string_to_size(parser.visible.get("I")); - res.print_secondary = cli::parse_yes_or_no(parser.visible.get("secondary")); - res.best_n_secondary = parser.visible.get("N"); - if (res.best_n_secondary == 0) { - spdlog::warn("Changed '-N 0' to '-N {} --secondary=no'", dflt.best_n_secondary); - res.print_secondary = false; - res.best_n_secondary = dflt.best_n_secondary; +Options process_minimap2_arguments(const ArgParser& parser) { + Options res{}; + res.kmer_size = get_optional_as(parser.visible.present("k")); + res.window_size = get_optional_as(parser.visible.present("w")); + auto index_batch_size = parser.visible.present("I"); + if (index_batch_size) { + res.index_batch_size = + std::make_optional(cli::parse_string_to_size(*index_batch_size)); } - auto bandwidth = cli::parse_string_to_sizes(parser.visible.get("--bandwidth")); - switch (bandwidth.size()) { - case 1: - res.bandwidth = int(bandwidth[0]); - res.bandwidth_long = dflt.bandwidth_long; - break; - case 2: - res.bandwidth = int(bandwidth[0]); - res.bandwidth_long = int(bandwidth[1]); - break; - default: - throw std::runtime_error("Wrong number of arguments for option '-r'."); + auto print_secondary = parser.visible.present("--secondary"); + if (print_secondary) { + res.print_secondary = std::make_optional(cli::parse_yes_or_no(*print_secondary)); + } + res.best_n_secondary = parser.visible.present("N"); + if (res.best_n_secondary.value_or(1) == 0) { + spdlog::warn("Ignoring '-N 0', using preset default"); + res.print_secondary = std::nullopt; + res.best_n_secondary = std::nullopt; + } + + auto optional_bandwidth = parser.visible.present("--bandwidth"); + if (optional_bandwidth) { + auto bandwidth = cli::parse_string_to_sizes(*optional_bandwidth); + switch (bandwidth.size()) { + case 1: + res.bandwidth = std::make_optional(bandwidth[0]); + break; + case 2: + res.bandwidth = std::make_optional(bandwidth[0]); + res.bandwidth_long = std::make_optional(bandwidth[1]); + break; + default: + throw std::runtime_error("Wrong number of arguments for option '-r'."); + } } - res.soft_clipping = parser.visible.get("Y"); + res.soft_clipping = parser.visible.present("Y"); res.mm2_preset = parser.visible.get("mm2-preset"); res.secondary_seq = parser.hidden.get("secondary-seq"); res.print_aln_seq = parser.hidden.get("print-aln-seq"); diff --git a/dorado/cli/duplex.cpp b/dorado/cli/duplex.cpp index d1c51582..065ae075 100644 --- a/dorado/cli/duplex.cpp +++ b/dorado/cli/duplex.cpp @@ -275,7 +275,7 @@ int duplex(int argc, char* argv[]) { .help("the minimum predicted methylation probability for a modified base to be emitted " "in an all-context model, [0, 1]"); - cli::add_minimap2_arguments(parser, alignment::dflt_options); + cli::add_minimap2_arguments(parser, alignment::DEFAULT_MM_PRESET); cli::add_internal_arguments(parser); std::set temp_model_paths; @@ -377,7 +377,7 @@ int duplex(int argc, char* argv[]) { hts_writer = pipeline_desc.add_node({}, hts_file, gpu_names); converted_reads_sink = hts_writer; } else { - auto options = cli::process_minimap2_arguments(parser, alignment::dflt_options); + auto options = cli::process_minimap2_arguments(parser); auto index_file_access = std::make_shared(); aligner = pipeline_desc.add_node({}, index_file_access, ref, "", options, std::thread::hardware_concurrency()); diff --git a/tests/AlignerTest.cpp b/tests/AlignerTest.cpp index 4d9433a3..adf2a05c 100644 --- a/tests/AlignerTest.cpp +++ b/tests/AlignerTest.cpp @@ -321,7 +321,7 @@ TEST_CASE_METHOD(AlignerNodeTestFixture, bam1_t* supplementary_rec = bam_records[2].get(); // Check aux tags. - if (options.soft_clipping) { + if (*options.soft_clipping) { CHECK(bam_aux_get(primary_rec, "MM") != nullptr); CHECK(bam_aux_get(primary_rec, "ML") != nullptr); CHECK(bam_aux_get(primary_rec, "MN") != nullptr); @@ -599,7 +599,7 @@ TEST_CASE_METHOD(AlignerNodeTestFixture, // Check aux tags. CHECK_THAT(bam_aux2Z(bam_aux_get(primary_rec, "SA")), Equals("read2,1,+,999S899M,60,0;")); - if (options.soft_clipping) { + if (*options.soft_clipping) { CHECK_THAT(bam_aux2Z(bam_aux_get(secondary_rec, "SA")), Equals("read3,1,+,999M899S,0,0;read2,1,+,999S899M,60,0;")); } else { diff --git a/tests/IndexFileAccessTest.cpp b/tests/IndexFileAccessTest.cpp index 56f8f39d..85d4a814 100644 --- a/tests/IndexFileAccessTest.cpp +++ b/tests/IndexFileAccessTest.cpp @@ -46,7 +46,8 @@ const std::string& valid_2read_reference_file() { const dorado::alignment::Minimap2Options& invalid_options() { static const dorado::alignment::Minimap2Options result = []() { dorado::alignment::Minimap2Options options{dorado::alignment::dflt_options}; - options.bandwidth = options.bandwidth_long + 1; + options.bandwidth_long = 1000; + options.bandwidth = *options.bandwidth_long + 1; return options; }(); return result; @@ -104,24 +105,28 @@ SCENARIO(TEST_GROUP " Load and retrieve index files", TEST_GROUP) { } } - GIVEN("load_index called with valid file and default options") { - Minimap2Options compatible_options{dflt_options}; - auto compatible_best_n = dflt_options.best_n_secondary + 1; + GIVEN("load_index called with valid file and options") { + auto original_options{dflt_options}; + original_options.best_n_secondary = 7; + + Minimap2Options compatible_options{original_options}; + auto compatible_best_n = + original_options.best_n_secondary ? *original_options.best_n_secondary + 1 : 1; compatible_options.best_n_secondary = compatible_best_n; - cut.load_index(valid_reference_file(), dflt_options, 1); + cut.load_index(valid_reference_file(), original_options, 1); THEN("is_index_loaded returns true") { - REQUIRE(cut.is_index_loaded(valid_reference_file(), dflt_options)); + REQUIRE(cut.is_index_loaded(valid_reference_file(), original_options)); } THEN("get_index returns non null index") { - REQUIRE(cut.get_index(valid_reference_file(), dflt_options) != nullptr); + REQUIRE(cut.get_index(valid_reference_file(), original_options) != nullptr); } THEN("is_index_loaded with compatible mapping options returns false") { REQUIRE_FALSE(cut.is_index_loaded(valid_reference_file(), compatible_options)); } AND_GIVEN("load_index called with same file and other valid indexing options") { - Minimap2Options other_options{dflt_options}; - other_options.kmer_size = other_options.kmer_size + 1; + Minimap2Options other_options{original_options}; + other_options.kmer_size = other_options.kmer_size.value_or(0) + 1; cut.load_index(valid_reference_file(), other_options, 1); THEN("is_index_loaded with other options returns true") { REQUIRE(cut.is_index_loaded(valid_reference_file(), other_options)); @@ -130,17 +135,18 @@ SCENARIO(TEST_GROUP " Load and retrieve index files", TEST_GROUP) { REQUIRE(cut.get_index(valid_reference_file(), other_options) != nullptr); } THEN("get_index with other options returns a different index") { - auto default_options_index = cut.get_index(valid_reference_file(), dflt_options); + auto default_options_index = + cut.get_index(valid_reference_file(), original_options); REQUIRE(cut.get_index(valid_reference_file(), other_options) != default_options_index); } AND_GIVEN("unload_index called with original options") { - cut.unload_index(valid_reference_file(), dflt_options); + cut.unload_index(valid_reference_file(), original_options); THEN("is_index_loaded with other options returns true") { REQUIRE(cut.is_index_loaded(valid_reference_file(), other_options)); } THEN("is_index_loaded with original options returns false") { - REQUIRE_FALSE(cut.is_index_loaded(valid_reference_file(), dflt_options)); + REQUIRE_FALSE(cut.is_index_loaded(valid_reference_file(), original_options)); } } } @@ -155,7 +161,7 @@ SCENARIO(TEST_GROUP " Load and retrieve index files", TEST_GROUP) { REQUIRE(cut.is_index_loaded(valid_reference_file(), compatible_options)); } THEN("is_index_loaded with original options returns true") { - REQUIRE(cut.is_index_loaded(valid_reference_file(), dflt_options)); + REQUIRE(cut.is_index_loaded(valid_reference_file(), original_options)); } THEN("get_index with compatible options returns a Minimap2Index with updated mapping " "options") { @@ -166,14 +172,15 @@ SCENARIO(TEST_GROUP " Load and retrieve index files", TEST_GROUP) { } THEN("get_index with original options returns a Minimap2Index with original mapping " "options") { - auto original_index = cut.get_index(valid_reference_file(), dflt_options); + auto original_index = cut.get_index(valid_reference_file(), original_options); CHECK(original_index); - REQUIRE(original_index->mapping_options().best_n == dflt_options.best_n_secondary); + REQUIRE(original_index->mapping_options().best_n == + *original_options.best_n_secondary); } THEN("get_index with compatible options returns a Minimap2Index with the same " "underlying minimap index") { - auto original_index = cut.get_index(valid_reference_file(), dflt_options); + auto original_index = cut.get_index(valid_reference_file(), original_options); CHECK(original_index); auto compatible_index = cut.get_index(valid_reference_file(), compatible_options); CHECK(compatible_index); @@ -181,12 +188,12 @@ SCENARIO(TEST_GROUP " Load and retrieve index files", TEST_GROUP) { REQUIRE(compatible_index->index() == original_index->index()); } AND_GIVEN("unload_index called with original options") { - cut.unload_index(valid_reference_file(), dflt_options); + cut.unload_index(valid_reference_file(), original_options); THEN("is_index_loaded with compatible options returns false") { REQUIRE_FALSE(cut.is_index_loaded(valid_reference_file(), compatible_options)); } THEN("is_index_loaded with original options returns false") { - REQUIRE_FALSE(cut.is_index_loaded(valid_reference_file(), dflt_options)); + REQUIRE_FALSE(cut.is_index_loaded(valid_reference_file(), original_options)); } } } @@ -209,8 +216,7 @@ TEST_CASE(TEST_GROUP " get_index called with compatible index returns non-null M IndexFileAccess cut{}; cut.load_index(valid_reference_file(), dflt_options, 1); Minimap2Options compatible_options{dflt_options}; - auto compatible_best_n = dflt_options.best_n_secondary + 1; - compatible_options.best_n_secondary = compatible_best_n; + compatible_options.best_n_secondary = dflt_options.best_n_secondary.value_or(0) + 1; REQUIRE(cut.get_index(valid_reference_file(), compatible_options) != nullptr); } @@ -222,8 +228,7 @@ TEST_CASE( IndexFileAccess cut{}; cut.load_index(valid_reference_file(), dflt_options, 1); Minimap2Options compatible_options{dflt_options}; - auto compatible_best_n = dflt_options.best_n_secondary + 1; - compatible_options.best_n_secondary = compatible_best_n; + compatible_options.best_n_secondary = dflt_options.best_n_secondary.value_or(0) + 1; auto index = cut.get_index(valid_reference_file(), compatible_options); diff --git a/tests/Minimap2IndexTest.cpp b/tests/Minimap2IndexTest.cpp index 5af4bb4f..c2b480d5 100644 --- a/tests/Minimap2IndexTest.cpp +++ b/tests/Minimap2IndexTest.cpp @@ -45,31 +45,42 @@ TEST_CASE(TEST_GROUP " initialise() with default options returns true", TEST_GRO REQUIRE(cut.initialise(dflt_options)); } -TEST_CASE(TEST_GROUP " initialise() with default options sets indexing options", TEST_GROUP) { +TEST_CASE(TEST_GROUP " initialise() with specified option sets indexing options", TEST_GROUP) { Minimap2Index cut{}; - cut.initialise(dflt_options); + auto options{dflt_options}; + options.kmer_size = 11; + options.window_size = 12; + options.index_batch_size = 123456789; - CHECK(cut.index_options().k == dflt_options.kmer_size); - CHECK(cut.index_options().w == dflt_options.window_size); - CHECK(cut.index_options().batch_size == dflt_options.index_batch_size); + cut.initialise(options); + + CHECK(cut.index_options().k == *options.kmer_size); + CHECK(cut.index_options().w == *options.window_size); + CHECK(cut.index_options().batch_size == *options.index_batch_size); } TEST_CASE(TEST_GROUP " initialise() with default options sets mapping options", TEST_GROUP) { Minimap2Index cut{}; + auto options{dflt_options}; + options.bandwidth = 300; + options.bandwidth_long = 12000; + options.best_n_secondary = 123456789; + options.soft_clipping = true; - cut.initialise(dflt_options); + cut.initialise(options); - CHECK(cut.mapping_options().bw == dflt_options.bandwidth); - CHECK(cut.mapping_options().bw_long == dflt_options.bandwidth_long); - CHECK(cut.mapping_options().best_n == dflt_options.best_n_secondary); + CHECK(cut.mapping_options().bw == options.bandwidth); + CHECK(cut.mapping_options().bw_long == options.bandwidth_long); + CHECK(cut.mapping_options().best_n == options.best_n_secondary); CHECK(cut.mapping_options().flag > 0); // Just checking it's been updated. } TEST_CASE(TEST_GROUP " initialise() with invalid options returns false", TEST_GROUP) { Minimap2Index cut{}; auto invalid_options{dflt_options}; - invalid_options.bandwidth = invalid_options.bandwidth_long + 1; + invalid_options.bandwidth_long = 100; + invalid_options.bandwidth = *invalid_options.bandwidth_long + 1; bool result{}; SuppressStderr::invoke( @@ -95,7 +106,8 @@ TEST_CASE_METHOD(Minimap2IndexTestFixture, TEST_GROUP) { cut.load(reference_file, 1); Minimap2Options invalid_compatible_options{dflt_options}; - invalid_compatible_options.bandwidth = invalid_compatible_options.bandwidth_long + 1; + invalid_compatible_options.bandwidth_long = 100; + invalid_compatible_options.bandwidth = *invalid_compatible_options.bandwidth_long + 1; std::shared_ptr compatible_index{}; { @@ -111,7 +123,7 @@ TEST_CASE_METHOD(Minimap2IndexTestFixture, TEST_GROUP) { cut.load(reference_file, 1); Minimap2Options compatible_options{dflt_options}; - ++compatible_options.best_n_secondary; + compatible_options.best_n_secondary = cut.mapping_options().best_n + 1; REQUIRE(cut.create_compatible_index(compatible_options) != nullptr); } @@ -123,7 +135,7 @@ TEST_CASE_METHOD(Minimap2IndexTestFixture, TEST_GROUP) { cut.load(reference_file, 1); Minimap2Options compatible_options{dflt_options}; - ++compatible_options.best_n_secondary; + compatible_options.best_n_secondary = cut.mapping_options().best_n + 1; auto compatible_index = cut.create_compatible_index(compatible_options); @@ -137,11 +149,11 @@ TEST_CASE_METHOD(Minimap2IndexTestFixture, TEST_GROUP) { cut.load(reference_file, 1); Minimap2Options compatible_options{dflt_options}; - ++compatible_options.best_n_secondary; + compatible_options.best_n_secondary = cut.mapping_options().best_n + 1; auto compatible_index = cut.create_compatible_index(compatible_options); - REQUIRE(compatible_index->mapping_options().best_n == dflt_options.best_n_secondary + 1); + REQUIRE(compatible_index->mapping_options().best_n == cut.mapping_options().best_n + 1); } } // namespace dorado::alignment::test