Removing 'threads' more for original duplicates because it had the possibility of using extreme resources

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()
 

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
 

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();
 }
 

src/duplication_results.hpp

@@ -13,9 +13,6 @@
 
 #ifdef ORIGINAL_DUPS
 #include <iterator>
-#ifdef ORIGINAL_DUPS_THREADS
-#include <limits>
-#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<std::int64_t>::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;