Check that reads don't exceed the maximal length

[csoneson]

Also allow reads up to 65534 nt

[mbstadler]

I think the check for having read a read that is too long is great!

Maybe we have to reduce the default read-length, though, or indeed expose it as a parameters, to avoid memory issues.
For example, the re-building of the vignette block starting at L174 fails under Linux on GHA with std::bad_alloc, indicating that it could not allocate requested memory. My guess is that this happened here:

mutscan/src/digestFastqs.cpp

Line 1149 in 130c852

FastqBuffer *chunkBuffer = new FastqBuffer((size_t)chunkSize, fastqReverseVect[0].compare("") != 0);

which will try to allocate a FastqBuffer instance with BUFFER_SIZE * chunksize * x bytes (x is 2 for single-red and 4 for paired-end data), which will here be around 64kb * 1e5 * 4 =~ 26GB (probably not available on the GitHub action runner). We could reduce sequence length, chunksize, or both. What about BUFFER_SIZE = 4096 and keep the current chunksize = 100000? That would allocate about 1.6GB for a paired end experiment.

[csoneson]

Yeah, seems I went a bit over the top (worked fine locally 🙂). A read length of 4096 will not be enough for the ONT data, so maybe we indeed do need to expose it as a parameter.

[mbstadler]

Right, in that case an exposed parameter and a lower default value would probably be best.

[codecov-commenter]

Codecov Report

Merging #44 (aa3a249) into master (2359969) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #44   +/-   ##
=======================================
  Coverage   96.96%   96.96%           
=======================================
  Files          20       20           
  Lines        2369     2376    +7     
=======================================
+ Hits         2297     2304    +7     
  Misses         72       72           

Impacted Files	Coverage Δ
R/digestFastqs.R	100.00% <100.00%> (ø)
src/FastqBuffer_utils.h	72.41% <100.00%> (+0.98%)	⬆️
src/digestFastqs.cpp	95.73% <100.00%> (+0.01%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[mbstadler]

I guess here we could say * Add parameter to specify maximal read length instead of the second item

[csoneson]

Oops, yes, definitely - forgot to update this.

[mbstadler]

I think this is a neat solution that leaves open all options, and the defaults are likely to cover the most common cases without creating a too large memory footprint. Apart from the comment on the NEWS.md file, I have no suggestions. Thank you!

[mbstadler]

The current failing checks on windows might be related to r-lib/actions#713 and should resolve itself soon