ksahlin / strobealign

Aligns short reads using dynamic seed size with strobemers
MIT License
128 stars 16 forks source link

Change default number of threads #401

Closed marcelm closed 3 months ago

marcelm commented 4 months ago

The default number of worker threads is currently set to 3: https://github.com/ksahlin/strobealign/blob/79cdd96df52fe3b5c9be88a1f11736b543e3d888/src/cmdline.hpp#L9

Since few CPUs have three cores, this is probably never the right number of threads to use. (I assume this was copied over from minimap2.)

I think there are three alternatives that make more sense (to me):

I suggest setting the default to 1, mainly because other tools do it this way and I would argue that this is the expected behavior.

Tools such as bwa mem, bowtie2 and make use one thread by default. Typically, only tools explicitly meant to run things in parallel (pigz, parallel) use all available cores.

Even Snakemake expects a tool to use one thread by default. Currently, if strobealign is run without -t and the rule does not contain threads: 3, snakemake may incorrectly schedule too many parallel strobealign jobs.

One downside to reducing the default to 1 is that an inexperienced user may not know that they could get their results faster by using the -t option. To remedy this a bit, I suggest to make it clearer in the log output which number of threads strobealign is using.

ksahlin commented 3 months ago

I agree with everything you write here. Let us change to 1 thread and make it clearer in the log output which number of threads strobealign is using, as you suggest.