bismark/align: intra-instance -p threading (replace --multicore fork model for Bowtie 2 + HISAT2)

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}"
         }
     }
     """

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') }
+            )
+        }
+    }
 }

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
+    }
+}