diff --git a/modules/nf-core/bismark/align/main.nf b/modules/nf-core/bismark/align/main.nf index ff7af322352f..0ad81a42522d 100644 --- a/modules/nf-core/bismark/align/main.nf +++ b/modules/nf-core/bismark/align/main.nf @@ -28,32 +28,54 @@ process BISMARK_ALIGN { } def fastq = meta.single_end ? reads : "-1 ${reads[0]} -2 ${reads[1]}" - // Try to assign sensible bismark --multicore if not already set - if(!args.contains('--multicore') && task.cpus){ + // Give each strand instance `-p = cpus / n_instances` threads (one index load per strand, + // byte-identical) instead of forking N --multicore chunks that each re-load the index. + // minimap2/rammap can't take -p from bismark yet, so they keep the legacy --multicore path. + def isMinimapLike = args.contains('--minimap2') || args.contains('--mm2') || args.contains('--rammap') || args.contains('--ram') + if(isMinimapLike){ - // Numbers based on recommendation by Felix for a typical mouse genome - def ccore = 1 - def cpu_per_multicore = 3 - def mem_per_multicore = (13.GB).toBytes() - if(args.contains('--non_directional')){ - cpu_per_multicore = 5 - mem_per_multicore = (18.GB).toBytes() - } + // Legacy --multicore auto-compute (unchanged) — minimap2/rammap only. + if(!args.contains('--multicore') && task.cpus){ + + // Numbers based on recommendation by Felix for a typical mouse genome + def ccore = 1 + def cpu_per_multicore = 3 + def mem_per_multicore = (13.GB).toBytes() + if(args.contains('--non_directional')){ + cpu_per_multicore = 5 + mem_per_multicore = (18.GB).toBytes() + } - // How many multicore splits can we afford with the cpus we have? - ccore = ((task.cpus as int) / cpu_per_multicore) as int + // How many multicore splits can we afford with the cpus we have? + ccore = ((task.cpus as int) / cpu_per_multicore) as int - // Check that we have enough memory - try { - def tmem = (task.memory as MemoryUnit).toBytes() - def mcore = (tmem / mem_per_multicore) as int - ccore = Math.min(ccore, mcore) - } catch (Exception e) { - log.warn "Error catched: ${e}" - log.warn "Not able to define bismark align multicore based on available memory" + // Check that we have enough memory + try { + def tmem = (task.memory as MemoryUnit).toBytes() + def mcore = (tmem / mem_per_multicore) as int + ccore = Math.min(ccore, mcore) + } catch (Exception e) { + log.warn "Error catched: ${e}" + log.warn "Not able to define bismark align multicore based on available memory" + } + if(ccore > 1){ + args += " --multicore ${ccore}" + } } - if(ccore > 1){ - args += " --multicore ${ccore}" + } else if(!args.contains('--multicore') && !args.contains('--parallel') && !args.contains('-p ') && task.cpus){ + + def n_instances = 2 + if(args.contains('--combined_index')){ + // combined index = one both-strands pass; parallel non-directional runs two. + n_instances = (args.contains('--non_directional') && args.contains('--combined_index_parallel')) ? 2 : 1 + } else if(args.contains('--non_directional')){ + n_instances = 4 + } + + // bismark requires -p >= 2, so omit it below that (one thread per instance). + def pthreads = ((task.cpus as int) / n_instances) as int + if(pthreads >= 2){ + args += " -p ${pthreads}" } } """ diff --git a/modules/nf-core/bismark/align/tests/main.nf.test b/modules/nf-core/bismark/align/tests/main.nf.test index b2fe71b6ffa1..60b34eb602a3 100644 --- a/modules/nf-core/bismark/align/tests/main.nf.test +++ b/modules/nf-core/bismark/align/tests/main.nf.test @@ -205,4 +205,142 @@ nextflow_process { ) } } + + test("bowtie2 | intra-instance -p threading | paired-end | 4 cpus") { + + config './nextflow_pthreads.config' + + when { + params { + bismark_args = '--bowtie2' + genomeprep_args = '--bowtie2' + } + process { + """ + input[0] = Channel.of([ + [ id:'test', single_end:false ], + [ + file(params.modules_testdata_base_path + 'genomics/sarscov2/illumina/fastq/test.methylated_1.fastq.gz', checkIfExists: true), + file(params.modules_testdata_base_path + 'genomics/sarscov2/illumina/fastq/test.methylated_2.fastq.gz', checkIfExists: true) + ] + ]) + input[1] = Channel.of([ + [ id:'sarscov2'], + file(params.modules_testdata_base_path + 'genomics/sarscov2/genome/genome.fasta', checkIfExists: true) + ]) + input[2] = BISMARK_GENOMEPREPARATION.out.index + """ + } + } + + then { + assertAll( + { assert process.success }, + // directional = 2 instances; 4 cpus / 2 -> -p 2 + { assert file(process.out.report[0][1]).text.contains('-p 2 --reorder') } + ) + } + } + + test("bowtie2 | non_directional | low-cpu omits -p (4 cpus / 4 instances < 2)") { + + config './nextflow_pthreads.config' + + when { + params { + bismark_args = '--bowtie2 --non_directional' + genomeprep_args = '--bowtie2' + } + process { + """ + input[0] = Channel.of([ + [ id:'test', single_end:true ], + file(params.modules_testdata_base_path + 'genomics/sarscov2/illumina/fastq/test.methylated_1.fastq.gz', checkIfExists: true) + ]) + input[1] = Channel.of([ + [ id:'sarscov2'], + file(params.modules_testdata_base_path + 'genomics/sarscov2/genome/genome.fasta', checkIfExists: true) + ]) + input[2] = BISMARK_GENOMEPREPARATION.out.index + """ + } + } + + then { + assertAll( + { assert process.success }, + // non-directional = 4 instances; 4 cpus / 4 = 1 (< 2) -> no -p + { assert !file(process.out.report[0][1]).text.contains('-p ') } + ) + } + } + + test("hisat2 | intra-instance -p threading | paired-end | 4 cpus") { + + config './nextflow_pthreads.config' + + when { + params { + bismark_args = '--hisat2' + genomeprep_args = '--hisat2' + } + process { + """ + input[0] = Channel.of([ + [ id:'test', single_end:false ], + [ + file(params.modules_testdata_base_path + 'genomics/sarscov2/illumina/fastq/test.methylated_1.fastq.gz', checkIfExists: true), + file(params.modules_testdata_base_path + 'genomics/sarscov2/illumina/fastq/test.methylated_2.fastq.gz', checkIfExists: true) + ] + ]) + input[1] = Channel.of([ + [ id:'sarscov2'], + file(params.modules_testdata_base_path + 'genomics/sarscov2/genome/genome.fasta', checkIfExists: true) + ]) + input[2] = BISMARK_GENOMEPREPARATION.out.index + """ + } + } + + then { + assertAll( + { assert process.success }, + // HISAT2 uses the same 2/4-instance model; directional 4/2 -> -p 2 + { assert file(process.out.report[0][1]).text.contains('-p 2 --reorder') } + ) + } + } + + test("bowtie2 | combined_index | intra-instance -p threading | single-end | 4 cpus") { + + config './nextflow_pthreads.config' + + when { + params { + bismark_args = '--bowtie2 --combined_index' + genomeprep_args = '--bowtie2 --combined_genome' + } + process { + """ + input[0] = Channel.of([ + [ id:'test', single_end:true ], + file(params.modules_testdata_base_path + 'genomics/sarscov2/illumina/fastq/test.methylated_1.fastq.gz', checkIfExists: true) + ]) + input[1] = Channel.of([ + [ id:'sarscov2'], + file(params.modules_testdata_base_path + 'genomics/sarscov2/genome/genome.fasta', checkIfExists: true) + ]) + input[2] = BISMARK_GENOMEPREPARATION.out.index + """ + } + } + + then { + assertAll( + { assert process.success }, + // combined index = 1 pass; 4 cpus / 1 -> -p 4 + { assert file(process.out.report[0][1]).text.contains('-p 4 --reorder') } + ) + } + } } diff --git a/modules/nf-core/bismark/align/tests/nextflow_pthreads.config b/modules/nf-core/bismark/align/tests/nextflow_pthreads.config new file mode 100644 index 000000000000..595274efdab3 --- /dev/null +++ b/modules/nf-core/bismark/align/tests/nextflow_pthreads.config @@ -0,0 +1,12 @@ +process { + // 4 cpus = nf-core CI ceiling; enough to exercise the -p divisor across paths. + withName: BISMARK_ALIGN { + cpus = 4 + ext.args = params.bismark_args + } + // Prep gets only the aligner flag (+ --combined_genome), not the alignment-only flags + // (--non_directional/--combined_index) that its parser rejects. + withName: BISMARK_GENOMEPREPARATION { + ext.args = params.genomeprep_args + } +}