diff --git a/CMakeLists.txt b/CMakeLists.txt index 2a9d492..c3aca1c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -59,17 +59,7 @@ if(PACKAGE) endif() if(ORIGINAL_DUPS) - string(TOUPPER ${ORIGINAL_DUPS} ORIGINAL_DUPS) message(STATUS "Building with original duplication analysis method") - if(NOT ORIGINAL_DUPS STREQUAL ON AND - NOT ORIGINAL_DUPS STREQUAL OFF AND - NOT ORIGINAL_DUPS STREQUAL THREADS) - message(FATAL_ERROR "ORIGINAL_DUPS must on/off/threads") - endif() - if(ORIGINAL_DUPS STREQUAL THREADS) - message(STATUS "Building original duplication for threads") - add_compile_options(-DORIGINAL_DUPS_THREADS) - endif() add_compile_options(-DORIGINAL_DUPS) endif() diff --git a/README.md b/README.md index 1e1a2c8..0aefeb1 100644 --- a/README.md +++ b/README.md @@ -130,30 +130,6 @@ might not matter to many users. Below are intended changes. Anything else is likely a bug and I'm happy to fix it. I'm also happy to reconsider these intended changes. -### Falco GC content calculations - -Falco calculates GC content of an individual read like this: -``` -floor(100.0*(n_g + n_c)/(n_a + n_c + n_g + n_t)) -``` -where `n_i` is the count of nucleotide `i` in the read, and `n_n` is not included. - -These values are tabulated in an array with 101 entries. The output includes -integer counts for each of the 101 possibilities. These are not the same as -generated by FastQC, which to the best of my interpretation, smoothes the -distribution while it is being tabulated. One weakness of the direct calculation -above is that mixing read lengths might lead to counter-intuitive results. For -example, read of length of 50 will not be able to contribute to each of the 101 -possible percentage values. Modifying falco so that it calculates the above -expression separately for each read length would be fairly easy, but I feel that -I'd need to know how the results across read lengths should be combined before -doing it. - -The smoothed CG percentages used for generating a grade attempts to replicate -what is done in FastQC, but I'm not entirely sure of the statistical basis. -Moving forward I will either convince myself that the approach in FastQC is -appropriate, or I will develop and implement the right approach. - ### Tiles results I found that the method for tile analysis is a bit unstable, and the tile grade @@ -188,10 +164,10 @@ produce more useful output, and to soon build [preseq](https://github.com/smithlabcode/preseq) into falco. The original duplication analysis method is still implemented in falco v2.0, but -it must be turned on at build time by supplying `-DORIGINAL_DUPS=[on|threads]` -to `cmake` (the difference is explained below). It's still there because I used -a trivial implementation and helps me debug. Here's how the old and new analyses -differ, to the best of my understanding. I'm happy to be corrected. +it must be turned on at build time by supplying `-DORIGINAL_DUPS=on` to +`cmake`. It's still there because I used a trivial implementation and helps me +debug. Here's how the old and new analyses differ, to the best of my +understanding. I'm happy to be corrected. #### Original method - 100,000 unique reads are hashed and counted (first 50nt of each read). @@ -207,12 +183,10 @@ differ, to the best of my understanding. I'm happy to be corrected. speed changing which 1M reads are hashed. Building with `-DORIGINAL_DUPS=on` enables the original duplication analysis, -but it only gives the same results when run using one thread. Building with -`-DORIGINAL_DUPS=threads` enables the original duplication analysis, giving the -same results when using multiple threads, but is slower when run using one -thread. The underlying issues with this duplication analysis were part of the -motivation for preseq, so I'm reluctant to put effort towards improving how the -original duplication analysis is implemented. +but it only gives the same results when run using one thread. The underlying +issues with this duplication analysis were part of the motivation for preseq, so +I'm reluctant to put effort towards improving how the original duplication +analysis is implemented. ## Citing falco diff --git a/src/duplication_results.cpp b/src/duplication_results.cpp index f20eead..35a9b58 100644 --- a/src/duplication_results.cpp +++ b/src/duplication_results.cpp @@ -126,20 +126,8 @@ duplication_results::add_and_consume(duplication_results &rhs) -> void { #ifndef ORIGINAL_DUPS for (const auto &[k, v] : rhs.dups) dups[k] += v; -#else // ORIGINAL_DUPS -#ifdef ORIGINAL_DUPS_THREADS - // ADS: these should be done earlier in a 'finalize' method before adding - // together - if (count_at_limit == 0) - count_at_limit = read_idx; - if (rhs.count_at_limit == 0) - rhs.count_at_limit = rhs.read_idx; - count_at_limit += rhs.count_at_limit; - read_idx += rhs.read_idx; - for (const auto &[k, v] : rhs.dups) - dups[k] += v; -#endif // ORIGINAL_DUPS_THREADS #endif // ORIGINAL_DUPS + // ADS: ORIGINAL_DUPS mode just uses dups from one of two duplication_results rhs.release(); } diff --git a/src/duplication_results.hpp b/src/duplication_results.hpp index 15efaa3..6710f7a 100644 --- a/src/duplication_results.hpp +++ b/src/duplication_results.hpp @@ -13,9 +13,6 @@ #ifdef ORIGINAL_DUPS #include -#ifdef ORIGINAL_DUPS_THREADS -#include -#endif // ORIGINAL_DUPS_THREADS #endif // ORIGINAL_DUPS class run_mode; @@ -39,12 +36,7 @@ struct dup_summary_t { struct duplication_results { static constexpr auto max_n_reads_total{1'000'000}; #ifdef ORIGINAL_DUPS -#ifdef ORIGINAL_DUPS_THREADS - static constexpr auto max_reads_to_hash{ - std::numeric_limits::max()}; -#else static constexpr auto max_reads_to_hash{100'000}; -#endif // ORIGINAL_DUPS_THREADS #endif // ORIGINAL_DUPS static constexpr auto default_read_skip{10}; static constexpr auto overrep_cutoff = 0.001;