fix: Bwa mem threads

[tdayris]

Description

Currently, master/bio/bwa-mem2/mem uses more threads than the number requested by user: for x threads requested by user, x is used in bwa-mem. Then either x are used by samtools (view or sort), or 1 by Picard. So for x threads requested by user, from x+1 to 2x threads are used.

This leads to bwa-mem jobs using official wrappers being killed by cluster manager for not respecting fair use rules where I work. Coworkers asked for a fix.

In this PR, I introduce a function to split the total number of threads between bwa-mem in one hand, and samtools/picard in the other hand.

Ultimately, the introduced changes also allow an uncompressed SAM output.

Tests have been modified, since this wrapper now requires at least two threads when samtools/picard are used. No more when sam output is requested.

QC

• I confirm that:

For all wrappers added by this PR,

• there is a test case which covers any introduced changes,
• input: and output: file paths in the resulting rule can be changed arbitrarily,
• either the wrapper can only use a single core, or the example rule contains a threads: x statement with x being a reasonable default,
• rule names in the test case are in snake_case and somehow tell what the rule is about or match the tools purpose or name (e.g., map_reads for a step that maps reads),
• all environment.yaml specifications follow the respective best practices,
• wherever possible, command line arguments are inferred and set automatically (e.g. based on file extensions in input: or output:),
• all fields of the example rules in the Snakefiles and their entries are explained via comments (input:/output:/params: etc.),
• stderr and/or stdout are logged correctly (log:), depending on the wrapped tool,
• temporary files are either written to a unique hidden folder in the working directory, or (better) stored where the Python function tempfile.gettempdir() points to (see here; this also means that using any Python tempfile default behavior works),
• the meta.yaml contains a link to the documentation of the respective tool or command,
• Snakefiles pass the linting (snakemake --lint),
• Snakefiles are formatted with snakefmt,
• Python wrapper scripts are formatted with black.
• Conda environments use a minimal amount of channels, in recommended ordering. E.g. for bioconda, use (conda-forge, bioconda, nodefaults, as conda-forge should have highest priority and defaults channels are usually not needed because most packages are in conda-forge nowadays).

[fgvieira]

This issue has appeared before in #32 and at the time it was concluded it was fine, since bwa and samtools/picard never run at the same time. Is this not the case for bwa-mem2?

[tdayris]

/usr/bin/time -v bwa-mem2 mem -t 4 ... | samtools sort -@ 3 ... on 10 real human WES datasets.

Should return a percentage of CPU used close-to-but-below 800% if I were right, or close-to-but-below 400% if you were right. My tests shows this number being 470% ± 15%.

Bwa and Samtools do not run with all their workers in the same time. But at one moment, we go over the value of 4 threads and almost reach 5 threads.

My solution is not optimal as it would largely overestimate the number of threads used.

[fgvieira]

Should return a percentage of CPU used close-to-but-below 800% if I were right, or close-to-but-below 400% if you were right. My tests shows this number being 470% ± 15%.

If so, can't we just give samtools (or whichever is not the limiting step) one thread less?
So, when running the wrapper with 4 threads, it would run /usr/bin/time -v bwa-mem2 mem -t 4 ... | samtools sort -@ 2.

[tdayris]

New tests on the very same 15 human WES mapping shows 380% ± 15% of CPU usage. I do not get kicked anymore. Thank you for your advises.