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

modules/nf-core/bismark/align/main.nf

@@ -1,59 +1,83 @@
 process BISMARK_ALIGN {
-    tag "$meta.id"
+    tag "${meta.id}"
     label 'process_high'
 
     conda "${moduleDir}/environment.yml"
-    container "${ workflow.containerEngine in ['singularity', 'apptainer'] && !task.ext.singularity_pull_docker_container ?
-        'https://community-cr-prod.seqera.io/docker/registry/v2/blobs/sha256/bd/bddea334e6ccbce005ce540214747acf822b040185d2198220dcfbb4b258c331/data' :
-        'community.wave.seqera.io/library/bismark:3.1.0--9557d6ab108a83e4' }"
+    container "${workflow.containerEngine in ['singularity', 'apptainer'] && !task.ext.singularity_pull_docker_container
+        ? 'https://community-cr-prod.seqera.io/docker/registry/v2/blobs/sha256/bd/bddea334e6ccbce005ce540214747acf822b040185d2198220dcfbb4b258c331/data'
+        : 'community.wave.seqera.io/library/bismark:3.1.0--9557d6ab108a83e4'}"
 
     input:
     tuple val(meta), path(reads)
-    tuple val(meta2), path(fasta, stageAs: 'tmp/*') // This change mounts as directory containing the FASTA file to prevent nested symlinks
+    tuple val(meta2), path(fasta, stageAs: 'tmp/*')
+    // This change mounts as directory containing the FASTA file to prevent nested symlinks
     tuple val(meta3), path(index)
 
     output:
-    tuple val(meta), path("*bam")       , emit: bam
+    tuple val(meta), path("*bam"), emit: bam
     tuple val(meta), path("*report.txt"), emit: report
-    tuple val(meta), path("*fq.gz")     , emit: unmapped, optional: true
+    tuple val(meta), path("*fq.gz"), emit: unmapped, optional: true
     tuple val("${task.process}"), val("bismark"), eval("bismark --version 2>&1 | grep -Eo '[0-9]+\\.[0-9]+\\.[0-9]+'"), topic: versions, emit: versions_bismark
 
     when:
     task.ext.when == null || task.ext.when
 
     script:
     def args = task.ext.args ?: ''
-    if(task.ext.prefix){
+    if (task.ext.prefix) {
         args += " --prefix ${task.ext.prefix}"
     }
     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
+            if (task.memory) {
+                def mcore = (task.memory.toBytes() / mem_per_multicore) as int
+                ccore = Math.min(ccore, mcore)
+            }
+            else {
+                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,134 @@ nextflow_process {
             )
         }
     }
+
+    test("bowtie2 | intra-instance -p threading | paired-end | 4 cpus") {
+
+        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)") {
+
+        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") {
+
+        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") {
+
+        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.config

@@ -2,7 +2,10 @@ process {
     withName: BISMARK_ALIGN {
         ext.args = params.bismark_args
     }
+    // Genome prep takes only the aligner-selection flag (+ --combined_genome); the alignment-only
+    // flags (--non_directional/--combined_index) are rejected by its parser. Tests that pass those
+    // set genomeprep_args separately; all others fall back to bismark_args.
     withName: BISMARK_GENOMEPREPARATION {
-        ext.args = params.bismark_args
+        ext.args = params.containsKey('genomeprep_args') ? params.genomeprep_args : params.bismark_args
     }
 }