Skip to content

Pair tumor and normal BAMs regardless of per-BAM kinetics #219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robert-a-forsyth wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Pair tumor and normal BAMs regardless of per-BAM kinetics #219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a911a0
Pair tumor and normal BAMs regardless of per-BAM kinetics
robert-a-forsyth Oct 6, 2026
1391c61
Merge origin/dev into fix/pairing-keys
robert-a-forsyth Oct 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions subworkflows/local/paired/paired_smallvar_germline.nf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ workflow PAIRED_SMALLVAR_GERMLINE {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return [ new_meta, meta.clair3_model, bam, bai ]
}
.set { normal_bams_model }
Expand Down Expand Up @@ -112,8 +111,7 @@ workflow PAIRED_SMALLVAR_GERMLINE {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
def intervals = []
return [new_meta, bam, bai, intervals]
}
Expand Down Expand Up @@ -188,8 +186,7 @@ workflow PAIRED_SMALLVAR_GERMLINE {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return[new_meta, vcf, tbi]
}
.set{germline_vcf}
Expand Down
3 changes: 1 addition & 2 deletions subworkflows/local/paired/paired_smallvar_somatic.nf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,7 @@ workflow PAIRED_SMALLVAR_SOMATIC {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return[new_meta, vcf, tbi]
}
.set{somatic_vcf}
Expand Down
12 changes: 4 additions & 8 deletions subworkflows/local/phasing_haplotyping.nf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ workflow PHASING_HAPLOTYPING {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return [ new_meta, bam, bai ]
}
.set{ tumor_only_ch }
Expand All @@ -80,8 +79,7 @@ workflow PHASING_HAPLOTYPING {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return [ new_meta, bam, bai ]
}
.set{ paired_normal_ch }
Expand All @@ -96,8 +94,7 @@ workflow PHASING_HAPLOTYPING {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return [ new_meta, bam, bai ]
}
.set{ paired_tumor_ch }
Expand Down Expand Up @@ -353,8 +350,7 @@ workflow PHASING_HAPLOTYPING {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return [ new_meta, mods ]
}
.set{modcall_vcf_ch}
Expand Down
6 changes: 2 additions & 4 deletions subworkflows/local/small_variant_consensus.nf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,7 @@ workflow SMALL_VARIANT_CONSENSUS {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return [ new_meta, vcfs, tbi]
}
.set{clair_ch}
Expand All @@ -152,8 +151,7 @@ workflow SMALL_VARIANT_CONSENSUS {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return [ new_meta, vcfs, tbi]
}
.set{deepvariant_ch}
Expand Down
6 changes: 2 additions & 4 deletions subworkflows/local/tumor_only/tumoronly_smallvar.nf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,7 @@ workflow TUMORONLY_SMALLVAR {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return[new_meta, vcf, tbi]
}
.set{somatic_vcf}
Expand All @@ -375,8 +374,7 @@ workflow TUMORONLY_SMALLVAR {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return[new_meta, vcf, tbi]
}
.set{germline_vcf}
Expand Down
15 changes: 15 additions & 0 deletions subworkflows/local/utils_nfcore_lrsomatic_pipeline/main.nf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,21 @@ def validateInputSamplesheet(input) {

return [ metas[0], bams ]
}

//
// Caller models key the tumor/normal and replicate joins, so all BAMs of a sample must agree on them
//
def validateSampleModels(metas) {
def model_keys = ['clair3_model', 'clairS_model', 'clairSTO_model']
metas.groupBy { meta -> meta.id }.each { id, rows ->
def differing = model_keys.findAll { key -> rows.collect { meta -> meta[key] }.unique().size() > 1 }
if (differing) {
def detail = differing.collect { key -> "${key}: ${rows.collect { meta -> "${meta.type}=${meta[key]}" }.join(', ')}" }.join('; ')
error("Sample '${id}': its BAMs resolve to different caller models (${detail}). Set ${differing.join(', ')} explicitly in the samplesheet for every row of this sample.")
}
}
return true
}
//
// Get attribute from genome config file e.g. fasta
//
Expand Down
49 changes: 49 additions & 0 deletions subworkflows/local/utils_nfcore_lrsomatic_pipeline/tests/main.function.nf.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,55 @@ nextflow_function {
}
}

// --- validateSampleModels: a sample's BAMs must agree on caller models, or the pairing joins drop it

test("validateSampleModels accepts a pair that differs only in kinetics") {

function "validateSampleModels"

when {
function {
"""
input[0] = [
[id: 's1', type: 'tumor', clair3_model: 'hifi', clairS_model: 'hifi_revio_ssrs', clairSTO_model: 'hifi_revio_ssrs', kinetics: 'false'],
[id: 's1', type: 'normal', clair3_model: 'hifi', clairS_model: 'hifi_revio_ssrs', clairSTO_model: 'hifi_revio_ssrs', kinetics: 'true']
]
"""
}
}

then {
assertAll(
{ assert function.success },
{ assert function.result == true }
)
}
}

test("validateSampleModels stops on a tumor/normal clair3_model mismatch") {

function "validateSampleModels"

when {
function {
"""
input[0] = [
[id: 's1', type: 'tumor', clair3_model: 'r1041_e82_400bps_sup_v500', clairS_model: 'ont_r10_dorado_sup_5khz', clairSTO_model: 'ont_r10_dorado_sup_5khz'],
[id: 's1', type: 'normal', clair3_model: 'r1041_e82_400bps_sup_v430', clairS_model: 'ont_r10_dorado_sup_5khz', clairSTO_model: 'ont_r10_dorado_sup_5khz'],
[id: 's2', type: 'tumor', clair3_model: 'hifi', clairS_model: 'hifi_revio_ssrs', clairSTO_model: 'hifi_revio_ssrs']
]
"""
}
}

then {
assertAll(
{ assert function.failed },
{ assert function.stdout.any { line -> line.contains("Sample 's1'") && line.contains('clair3_model') } }
)
}
}

// --- validateCondaSupport: container-only steps fail at launch under conda

test("validateCondaSupport names every container-only step left on under conda") {
Expand Down
26 changes: 14 additions & 12 deletions workflows/lrsomatic.nf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ include { reportGenePanelTokens } from '../subworkflows/local/utils_nfcore_lrso
include { reportGenePanelIsFile } from '../subworkflows/local/utils_nfcore_lrsomatic_pipeline'
include { resolveVepPlugins; validateVepPluginParams } from '../subworkflows/local/utils_nfcore_lrsomatic_pipeline'
include { padfootGenome; padfootAnnotationOk } from '../subworkflows/local/utils_nfcore_lrsomatic_pipeline'
include { validateClairstoCnaResources } from '../subworkflows/local/utils_nfcore_lrsomatic_pipeline'
include { validateClairstoCnaResources; validateSampleModels } from '../subworkflows/local/utils_nfcore_lrsomatic_pipeline'
include { PREPARE_VEP_PLUGINS } from '../subworkflows/local/prepare_vep_plugins'

//
Expand Down Expand Up @@ -291,6 +291,12 @@ workflow LRSOMATIC {
// clair3_model, clairS_model, clairSTO_model, kinetics
// bams are grouped per sample (multiple runs merged into a list)

// Fail fast if a sample's BAMs resolve to different caller models; they key the pairing joins
ch_samplesheet
.map { meta, _bam -> meta }
.collect()
.map { metas -> validateSampleModels(metas) }

@claude claude Bot Oct 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing CHANGELOG.md entry. The PR changes behaviour that users will see:

  • Paired samples whose BAMs differ in kinetics are no longer dropped silently.
  • A run with mismatched per-BAM caller models now stops with an error.
  • The tumor/normal join now uses failOnMismatch.

REVIEW.md asks for an entry under v1.2.0dev that cites #219. This should probably go under Fixed, and possibly Changed for the new launch-time error.


//
// SUBWORKFLOW: PREPARE_REFERENCE_FILES -- decompress and index the FASTA, fetch Clair3 models, unpack ASCAT references
// Input: params.fasta, ASCAT file paths, basecall_meta, clair3_modelMap
Expand Down Expand Up @@ -438,7 +444,6 @@ workflow LRSOMATIC {
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics',
'n_replicates')
// groupKey: release each sample as soon as its own replicates arrive, not when all samples have
return [groupKey(new_meta, new_meta.n_replicates), bam, bai]
Expand Down Expand Up @@ -522,8 +527,7 @@ workflow LRSOMATIC {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return[new_meta, bam, bai]
}
.set{paired_normal_bams}
Expand All @@ -539,11 +543,11 @@ workflow LRSOMATIC {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return[new_meta, bam, bai]
}
.join(paired_normal_bams)
// failOnMismatch: a tumor/normal meta drift must stop the run, not drop the pair
.join(paired_normal_bams, failOnMismatch: true)

@claude claude Bot Oct 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No regression test for the bug this PR fixes. The only new tests are the two validateSampleModels function tests. They don't run the pairing join itself.

Nothing tagged small covers a paired PacBio sample where one BAM has kinetics and the other doesn't, so CI never runs that case. The core fix (dropping kinetics from the subMaps) and the new failOnMismatch: true are therefore only checked indirectly.

Suggestions:

  • Add a small stub or fixture test with a tumor/normal pair whose METAEXTRACT kinetics differs. It should show that somatic_smallvar_input emits the pair and that the run reaches LRSOMATICREPORT.
  • If that isn't practical, run the Slurm test array with such a samplesheet and note the result in the PR.

.set { somatic_smallvar_input }
// somatic_smallvar_input: [meta, tumor_bam, tumor_bai, normal_bam, normal_bai]

Expand All @@ -570,8 +574,7 @@ workflow LRSOMATIC {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
def normal_bam = []
def normal_bai = []
return [new_meta, normal_bam, normal_bai, bam, bai]
Expand Down Expand Up @@ -877,8 +880,7 @@ workflow LRSOMATIC {
'fiber',
'clair3_model',
'clairS_model',
'clairSTO_model',
'kinetics')
'clairSTO_model')
return[new_meta, bam, bai]
}
.map{meta, tumor_bam, tumor_bai->
Expand Down Expand Up @@ -1050,7 +1052,7 @@ workflow LRSOMATIC {
// so build its input from PHASING_HAPLOTYPING's haplotagged BAMs rather than the
// unphased ones severus_input carries.
def savana_meta_keys = ['id', 'paired_data', 'platform', 'sex', 'fiber',
'clair3_model', 'clairS_model', 'clairSTO_model', 'kinetics']
'clair3_model', 'clairS_model', 'clairSTO_model']

PHASING_HAPLOTYPING.out.tumor_normal_hapbams_ch
.branch { meta, _bam, _bai ->
Expand Down
Loading