diff --git a/modules/nf-core/bismark/align/main.nf b/modules/nf-core/bismark/align/main.nf index ff7af322352f..848dbab124b4 100644 --- a/modules/nf-core/bismark/align/main.nf +++ b/modules/nf-core/bismark/align/main.nf @@ -1,21 +1,22 @@ 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: @@ -23,37 +24,60 @@ process BISMARK_ALIGN { 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}" } } """ diff --git a/modules/nf-core/bismark/align/tests/main.nf.test b/modules/nf-core/bismark/align/tests/main.nf.test index b2fe71b6ffa1..17787b38b258 100644 --- a/modules/nf-core/bismark/align/tests/main.nf.test +++ b/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') } + ) + } + } } diff --git a/modules/nf-core/bismark/align/tests/nextflow.config b/modules/nf-core/bismark/align/tests/nextflow.config index 3313dfa884cb..6bdc178d47d2 100644 --- a/modules/nf-core/bismark/align/tests/nextflow.config +++ b/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 } }