Fgumi initial commit by freerkvandijk · Pull Request #165 · nf-cmgg/preprocessing

freerkvandijk · 2026-04-07T13:31:40Z

PR checklist

v2.0.6 Release PR

- module updates - faster bclconvert output handling - modified output structure, featuring qc reports per library and enhanced run QC - fastq output when aligner equals false, with falco report - per sample analysis definition for more flexibility

matthdsm · 2026-04-07T19:24:24Z

assets/schema_input.json

            "umi_aware": {
                "meta": ["umi_aware"],
                "type": "boolean",
-                "description": "Run markdup in UMI-aware mode. This applies to Samtools only and requires the UMI to be in the read name.",


Don't change this, but add another option

matthdsm · 2026-04-07T19:24:38Z

assets/schema_sampleinfo.json

            "umi_aware": {
                "meta": ["umi_aware"],
                "type": "boolean",
-                "description": "Run markdup in UMI-aware mode. This applies to Samtools only and requires the UMI to be in the read name.",


Don't change this, but add another option

matthdsm · 2026-04-07T19:26:09Z

conf/modules.config

+        ext.prefix = { "${meta.id}.fgumi.unmapped" }
+        ext.args   = {
+            [
+                "--read-structures ${params.fgumi_read_structures}",


Avoid using pipeline wide params for sample specific settings, instead use meta values

matthdsm · 2026-04-07T19:26:34Z

conf/modules.config

+    //// FGUMI extract (step 1)
+    withName: '.*FASTQ_TO_CRAM:FGUMI_EXTRACT' {
+        cpus       = 8
+        memory     = 32.GB


Benchmarked? Seems high

matthdsm · 2026-04-07T19:27:54Z

conf/modules.config

+                "${params.fgumi_snap_extra_args}",
+            ].join(" ").trim()
+        }
+        ext.args2  = {


Snap params should be equivalent to default alignment config

matthdsm · 2026-04-07T19:28:54Z

conf/modules.config

+                "--strategy ${params.fgumi_group_strategy}",
+                "--edits ${params.fgumi_group_edits}",
+                "--threads ${task.cpus}",
+                "--queue-memory ${params.fgumi_queue_memory}",


Should be handled in the module code and depend on process.memory

matthdsm · 2026-04-07T19:30:33Z

modules/local/fgumi/extract/main.nf

+    script:
+    def args = task.ext.args ?: ''
+    prefix = task.ext.prefix ?: "${meta.id}.fgumi.unmapped"
+    def sample_name = meta.samplename ?: meta.id


Assume meta.samplename and meta.library do not exist to make it portable to nf-core

matthdsm · 2026-04-07T19:31:06Z

modules/local/fgumi/extract/main.nf

+    prefix = task.ext.prefix ?: "${meta.id}.fgumi.unmapped"
+    def sample_name = meta.samplename ?: meta.id
+    def library_name = meta.library ?: meta.id
+    def input_files = (reads instanceof List ? reads : [reads]).collect { read -> "${read}" }.join(' ')


nextflow automatically parses lists, so this isn't needed

matthdsm · 2026-04-07T19:32:34Z

modules/local/fgumi/filter/main.nf

+        --ref ${fasta} \
+        ${args}
+
+    fgumi sort \


Separate modules here?

matthdsm · 2026-04-07T19:33:35Z

modules/local/fgumi/snapzippersort/main.nf

+            -o -sam - \
+            -t ${task.cpus} \
+            ${snap_args} \
+        | samtools sort \


Drop samtools, and let snap sort the output

matthdsm · 2026-04-07T19:35:21Z

subworkflows/local/fastq_to_aligned_cram/main.nf


 // MODULES
 include { BIOBAMBAM_BAMSORMADUP } from "../../../modules/nf-core/biobambam/bamsormadup/main.nf"
+include { FGUMI_DUPLEX_METRICS  } from "../../../modules/local/fgumi/duplexmetrics/main.nf"


Put all of this in a subworkflow

matthdsm · 2026-04-07T19:36:01Z

subworkflows/local/fastq_to_aligned_cram/main.nf

-    // ALIGNMENT([meta,fastq], index, sort)
+    ch_meta_reads_aligner_index_fasta_datatype.dna
+        .branch { meta, reads, aligner, index, fasta ->
+            umi: meta.umi_aware == true


Do not override existing settings, this will break samtools UMI handling

matthdsm · 2026-04-07T19:36:32Z

subworkflows/local/fastq_to_aligned_cram/main.nf

        .set { ch_cram_crai }
    ch_cram_crai.dump(tag: "FASTQ_TO_CRAM: cram and crai", pretty: true)

+    // Keep a dedicated channel for UMI-aware sample CRAM outputs.


matthdsm · 2026-04-07T19:37:18Z

tests/inputs/fgumi/grouped.bam

Don't add test data in the repo, use nf-cmgg/test-datasets or data available on S3

matthdsm · 2026-04-07T19:38:08Z

tests/inputs/test.yml

+  markdup: bamsormadup
+  umi_aware: true


This block is inherently wrong, bamsormadup does not support UMI markdup

matthdsm

See comments

…ests

matthdsm · 2026-04-08T12:09:40Z

conf/modules.config

-                "--read-structures ${params.fgumi_read_structures}",
-                params.fgumi_extract_umis_from_read_names ? "--extract-umis-from-read-names" : "",
+                "--read-structures ${meta.fgumi_read_structures ?: params.fgumi_read_structures}",
+                ((meta.fgumi_extract_umis_from_read_names != null ? meta.fgumi_extract_umis_from_read_names : params.fgumi_extract_umis_from_read_names) ? "--extract-umis-from-read-names" : ""),


Drop the params

matthdsm · 2026-04-08T12:10:29Z

conf/modules.config

+                "-sa",
+                "-xf 2",
+                meta.readgroup ? "-R \"@RG\\t" + meta.readgroup.findResults { rg -> rg.value?.trim() ? "${rg.key}:${rg.value}" : null }.join("\\t") + "\"" : "",
                "${params.fgumi_snap_extra_args}",


Drop this param, should go in modules.config

matthdsm · 2026-04-08T12:11:13Z

conf/modules.config

-                "--family-size-histogram ${meta.id}.fgumi.group.family_size_histogram.txt",
-            ].join(" ").trim()
-        }
+        ext.strategy          = { params.fgumi_group_strategy }


only ext.args is allowed

matthdsm · 2026-04-08T12:11:42Z

conf/modules.config

        ext.prefix = { "${meta.id}.fgumi.filter" }
        ext.args   = {
            [
                "--min-reads ${params.fgumi_filter_min_reads}",


matthdsm · 2026-04-08T12:13:56Z

modules/local/fgumi/extract/main.nf

-    def library_name = meta.library ?: meta.id
-    def input_files = (reads instanceof List ? reads : [reads]).collect { read -> "${read}" }.join(' ')
+    // Keep module portable: only meta.id is assumed, with optional task.ext overrides.
+    def sample_name = task.ext.sample_name ?: meta.id


only ext.args is allowed, check out nf-core/modules for examples of complex module config

matthdsm · 2026-04-08T12:15:16Z

modules/local/fgumi/group/main.nf

    fgumi group \
        --input ${bam} \
        --output ${prefix}.bam \
+        --strategy ${strategy} \


Anything that isn't resource related must be specified in modules.config under ext.args

matthdsm · 2026-04-08T12:15:41Z

modules/local/fgumi/sort/main.nf

+    fgumi sort \
+        --input ${bam} \
+        --output ${prefix}.bam \
+        --order coordinate \


Don't hard code options

matthdsm · 2026-04-08T12:18:47Z

subworkflows/local/umi_consensus/main.nf

+    FGUMI_SIMPLEX(
+        FGUMI_GROUP.out.bam
+    )
+
+    FGUMI_DUPLEX_METRICS(
+        FGUMI_GROUP.out.bam
+    )
+


How does the workflow differentiate between duplex and simplex?

matthdsm · 2026-04-08T12:19:51Z

tests/modules/local/fgumi/duplexmetrics/main.nf.test

                input[0] = [
                    [id: "test"],
-                    file("${projectDir}/tests/inputs/fgumi/grouped.bam", checkIfExists: true)
+                    file("${projectDir}/tests/inputs/fgumi/template.bam", checkIfExists: true)


No test-data in repo

matthdsm · 2026-04-08T12:20:52Z

tests/modules/local/fgumi/sort/main.nf.test

Also add test with data, not only stubs

matthdsm · 2026-04-08T12:22:50Z

workflows/preprocessing.nf

    umi_filtered_consensus_bam = FASTQ_TO_CRAM.out.filtered_consensus_bam
    umi_duplex_metrics         = FASTQ_TO_CRAM.out.duplex_metrics
-    umi_crams                  = FASTQ_TO_CRAM.out.umi_cram_crai
+    umi_crams                  = FASTQ_TO_CRAM.out.cram_crai.filter { meta, _cram, _crai -> meta.fgumi_aware == true }


What's the point of separate outputs?

matthdsm

More being difficult

matthdsm · 2026-04-10T14:11:07Z

conf/modules.config

Still some params left over.
threads and memory need to go in to module code

matthdsm · 2026-04-10T14:11:53Z

conf/modules.config

+                "--strategy ${params.fgumi_group_strategy}",
+                "--edits ${params.fgumi_group_edits}",
+                "--compression-level ${params.fgumi_compression_level}",


params -> meta

matthdsm and others added 3 commits June 23, 2025 12:48

Merge pull request #132 from nf-cmgg/dev

e3021ca

v2.0.6 Release PR

Preprocessing v3.0.0 release

253c025

- module updates - faster bclconvert output handling - modified output structure, featuring qc reports per library and enhanced run QC - fastq output when aligner equals false, with falco report - per sample analysis definition for more flexibility

Initial commit fgumi implementation

f5a2972

freerkvandijk requested a review from matthdsm April 7, 2026 13:31

matthdsm reviewed Apr 7, 2026

View reviewed changes

modules/local/fgumi/filter/main.nf Outdated

--ref ${fasta} \

${args}

fgumi sort \

Copy link
Copy Markdown

Member

matthdsm Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate modules here?

matthdsm reviewed Apr 7, 2026

View reviewed changes

matthdsm requested changes Apr 7, 2026

View reviewed changes

freerkvandijk added 10 commits April 8, 2026 09:38

Fixed assets. Changed to fgumi_aware parameter.

bd23c30

Changed pipeline wide read-structures to meta values

1795ab6

Added usage of default SNAP parameters in config

9ea17ff

Process memory now handled per nf-core style

f205480

Changes to fgumi extract to make it portable (nf-core)

b2c80a7

List input instead of own conversion

ac9c547

Split FGUMI_FILTER into seperate filter and sort module and updated t…

05ee645

…ests

decoupled umi_aware and fgumi_aware

cfdaac0

Refactor: fgumi modules to dedicated umi_consensus subworkflow

31f924d

removed output channel

3b30b71

freerkvandijk added 3 commits April 8, 2026 10:36

Changed tests to use template.bam and removed grouped.bam

289d935

Changed some defaul resource requirements. Needs benchmarking though

53de497

Added some comments + fixed fgumi simplex parameter bug

53549c2

matthdsm reviewed Apr 8, 2026

View reviewed changes

tests/modules/local/fgumi/sort/main.nf.test

Copy link
Copy Markdown

Member

matthdsm Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add test with data, not only stubs

matthdsm reviewed Apr 8, 2026

View reviewed changes

matthdsm requested changes Apr 8, 2026

View reviewed changes

freerkvandijk added 7 commits April 9, 2026 09:43

Dropped params

8af63a6

Fixed ext.args

0240c8b

Moved hard coded options

fde7ed8

Removed duplex metrics from pipeline

a6691fb

Removed output redundancy

1cd4b3b

Removed test BAMs + added paths to github repo test BAMs

249df56

Updated filepaths in tests

56cf6cc

matthdsm reviewed Apr 10, 2026

View reviewed changes

freerkvandijk added 3 commits April 10, 2026 17:24

removed thread and mem args from config

092a382

params to meta

c1d00d5

Fixed paths in test

b6091f0

Conversation

freerkvandijk commented Apr 7, 2026

PR checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthdsm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthdsm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants