Repository navigation
Fix/sigprofiler probabilities per mutation - #220
Closed
robert-a-forsyth wants to merge 343 commits into
Closed
robert-a-forsyth wants to merge 343 commits into
robert-a-forsyth wants to merge 343 commits into
Conversation
…#188) - Restore the `!task.ext.singularity_pull_docker_container` escape hatch in the MODKIT_PILEUP container ternary (nf-core default and SIGPROFILER_ASSIGNMENT keep it), so sites where an oras:// pull is blocked can fall back to the Docker image. Module patch refreshed to match; the module comment now names the fork source. - Split the changelog: the container fix stays under Fixed and names both image URIs; the new `--cpg --modified-bases 5mC` default moves to Changed, with a note that reruns produce different bedMethyl content and how to restore the old output. - `modkit_args` docs/schema now say the value replaces the default rather than extending it. `--modkit_args ''` (and a bare `--modkit_args`) reach the config as the flag value `true`, which would have been passed to modkit verbatim; the ext.args closure now treats that as "no arguments" so the documented escape works. - `skip_modcall` description clarified: it skips Longphase modcall (phasing evidence), not the modkit pileup. - `modkit_phased` docs note the dependency on small-variant calling, phasing and haplotagging; `modkit_args`/`modkit_phased` moved out of the "Skipping options" table into a "Modkit options" section; output docs state there is no separate file for untagged reads. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Both found by the first real end-to-end run (Mindwell, GRCh38, BL1 + DLBCL1), which is also the first time the EVE path has ever executed. The EXIT trap added in ceccb7f referred to a variable that no longer exists by the time it fires. `work` is declared `local`, so a RETURN trap sees it but an EXIT trap -- which runs after the function has returned -- does not, and under `set -u` that is a fatal "work: unbound variable". VEPPLUGIN_REVEL therefore finished its work, wrote both output files, and then exited 1. The trap now expands the path when it is defined rather than when it fires. `find ... | head -n1` aborts under `set -o pipefail` whenever the producer is still writing: head exits after one line, sort takes SIGPIPE, and the pipeline returns 141. With the EVE release's ~3000 per-protein VCFs that fired every time, so VEPPLUGIN_EVE failed three attempts and took the run down with it. The REVEL copy of the same construct only survived because find matches a single file there and finishes before head exits -- a race, not a safe pattern, so both are now collected with mapfile instead. Verified against the staged inputs from the failed run: REVEL prep exits 0, writes revel_grch38.tsv.gz plus its index, and leaves no temp directory behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rking --modkit_args escape (#188) Review follow-up for PR #188. - MODKIT_PILEUP: drop the conda directive and stop with an error under -profile conda/mamba (deepvariant/sigprofiler pattern). environment.yml would install stock ont-modkit 0.6.1, which silently reintroduces the PacBio read drop this PR fixes. Verified: the module test under -profile conda fails with the new message. - modules/nf-core/modkit/pileup/tests: the test passed three inputs to a four-input patched module (fasta/fai split), so it could not run and the 0.6.1 -> 0.6.4 snapshot edit had no run behind it. Inputs fixed for all ten cases; snapshot regenerated from a real run of the patched image on Mindwell (10/10 pass, versions 0.6.4, bedMethyl md5s follow the binary). Module patch refreshed (a trailing newline was added to the snapshot so the last hunk reverse-applies). - --modkit_args '' does not work from the CLI: Nextflow turns it into `true`, which nf-schema rejects as a boolean on Nextflow 25.10.4 and which arrives as the string 'true' on Nextflow 26 (verified with -preview on both). Docs, schema and changelog now say --modkit_args='' or a params file entry; the modules.config guard stays as a backstop. - docs/usage.md and CHANGELOG.md: the image is linux/amd64 only and conda and the arm64 profile are unsupported for this step (--skip_modkit); N_other note copied from the schema into the usage table. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
fix(modkit): patched modkit 0.6.4 for PacBio 5mC+5hmC reads, 5mC-CpG pileup defaults and --modkit_phased option
…the new tests Second review round. All four are follow-ups to the first round's changes. The new nf-test function tests never ran in CI. .github/workflows/nf-test.yml selects `--tag small` on pull_request, and the file carried only its four descriptive tags, so the shard step found nothing for it. Tagged `small`; the file takes about 90 s. nextflow_schema.json still described the state before the CADD opt-in and the URL pins. Reworded the help text for vep_cadd_snv and vep_cadd_indel (no longer defaults, and the prepared REVEL/EVE files are published now), vep_polyphen_sift_db (release-115, not current_variation), vep_clinvar (the contig-naming claim is softened the way docs/usage.md already was) and skip_vep_plugins (about 1.4 GB on GRCh38 and 14 GB on CHM13, not "tens of GB"). Two more strings in the same file were falsified by the index guard added in 837efe0: --vep_revel_tbi and --vep_eve_tbi called themselves "unused" alongside a release zip, where they are now rejected. The vep_revel and vep_eve texts also said to pass a .zip, which stopped being the test in 2e7eb85 -- EVE's own download URL carries no extension at all. CITATIONS.md linked current_variation/ for the pangenome database, so the citation named a moving target rather than the release actually used. docs/usage.md had no blank line before "For mutational signatures", so Markdown ran that paragraph into the one above it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md # CITATIONS.md
…-key Key Clair3 model downloads on the downloaded model so Clair3 runs once per normal BAM
resolveVepPlugins() built the --plugin string and the workflow wrote it to params.vep_plugin_args, which conf/modules.config read back in the SOMATIC_VEP and GERMLINE_VEP ext.args closures. A param assigned in the workflow body is not visible to those closures, so the read found nothing and the `?: ''` fallback turned it into an empty string. Every VEP task ran with no plugins at all, and Nextflow said so once per task: "Access to undefined parameter `vep_plugin_args`". Both cluster test runs had finished COMPLETED 0:0 that way. The GRCh38 one prepared REVEL and EVE and staged AlphaMissense and both CADD tables first, then annotated without a single --plugin, so the preparation was wasted and the run still reported success. params.vep_genome, assigned the same way ten lines earlier and equally undeclared, works -- because it is read in the workflow body and handed to the module as an input. The plugin arguments now travel that same route: a new `val plugin_args` input on ENSEMBLVEP_VEP, a module this pipeline already patches, in the shape the ASCAT patch already uses for val(genomeVersion). SOMATIC_VEP and GERMLINE_VEP get vep_plugins.args; SV_VEP gets '', matching the [] it already passes for extra_files. All three ext.args closures are now identical, and the `?: ''` that made the failure silent is gone. Declaring vep_plugin_args in nextflow.config would not have fixed this, though Nextflow's own warning suggests exactly that: it would have silenced the warning and left VEP plugin-free. Moving the plugin arguments out of ext.args also drops an ordering dependency. The --custom rewrite in the module now only ever sees the user's own entry from --vep_args, so ClinVar keeps the filename it was staged under no matter how the arguments are ordered. Verified on both assemblies. GRCh38 parses the same 43,630 CSQ entries before and after: AlphaMissense 0 -> 132, CADD 0 -> 42,946, REVEL 0 -> 121, EVE 0 -> 19, ClinVar 0 -> 96, while the cache-derived SIFT and PolyPhen are unchanged at 150 and 135. CHM13 gains AlphaMissenseProtein, PolyPhen_SIFT and ClinVar. SV_VEP stays at zero on both, as intended. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The report tool is vendored rather than submoduled, so upstream releases only reach the pipeline by hand. The copy at assets/lrsomatic_report was v1.3.2; upstream v1.4.0 makes the small-variant table read the VEP plugin annotations this pipeline already produces — AlphaMissense, ClinVar, CADD, REVEL, EVE, and SIFT/PolyPhen from the PolyPhen_SIFT plugin on T2T, whose cache carries neither. Each predictor's class and score become separate columns, so the class gets a tickbox filter and the score sorts numerically, and a column appears only when the annotated VCF declared that field. Until now the pipeline computed those annotations and the report dropped them. The release also fixes the dbSNP/COSMIC and Existing_variation handling on the VEP text output path, draws the circos y-axes on the tracks they label, gives the SV palette and y-positions one definition pinned to the SCSS, guards the CNV axis, and drops the panel hero cards. environment.yml is untouched — the only new package reference upstream is DT::, already a dependency — so both frozen Wave images stay as they are. Version bumped in the seven places VENDORED.md lists: the module's versions topic, meta.yml twice, the module snapshot and the five pipeline snapshots. The rendered HTML is not snapshotted, so no snapshot content beyond that changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Written after the PR was opened, so it can name #195. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
VENDORED.md's re-sync checklist ends at the CHANGELOG: "the vendored version named in the #176 entry". That Added entry is the one the release notes will read as the shipped version, and v1.2.0dev is still unreleased, so leaving it at v1.3.2 would have gone out naming a version the tree no longer holds. Every earlier re-sync bumped it the same way (v1.1.0 -> v1.2.1 -> v1.3.0 -> v1.3.2). The separate "#176 - Re-synced ... to v1.3.2" entry under Changed stays as it is: it records what that PR did, not what is vendored now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
assets/lrsomatic_report was a hand-maintained copy of the report tool -- 38 files, ~9,900 lines -- because `nextflow run IntGenomicsLab/lrsomatic` does not fetch submodules. Every release of that tool meant re-copying six paths and bumping a version string in seven places that nothing cross-checked; its own VENDORED.md said so: "Nothing checks these for consistency; miss one and the snapshots go stale silently." PR CI never ran the report module test either, since nf-test.yml filters pull requests to `tags: small`, so a bad vendor landed green. The tool now ships inside its own image and the copy is deleted. Updating it is a tag bump on two lines, which is how MODKIT_PILEUP and the three SIGPROFILER_* modules already work here. The version is no longer hardcoded. lrsomatic_report v1.5.0 adds --version, so the versions topic reads it out of the container with an eval and cannot disagree with the image it ran in. Note this renumbers nxf_out_eval_N for processes declared after LRSOMATICREPORT and so invalidates -resume for them; the module sits near the end of the workflow, so the blast radius is small, but do not expect a fast resume on existing work dirs. Builtin gene panels moved to assets/gene_lists and reach the tool through its new --gene-lists-dir. They had to go somewhere: launch-time validation of --report_gene_panel globs that directory on the host, which cannot see inside an image. Staging them as ordinary user panels would not have been equivalent -- they would have taken the user-panel branch and been renamed lymphoid-custom against the image's own copy -- hence the upstream flag. The pipeline now depends on nothing inside the container except render_report.R being on PATH, and adding a panel is a pipeline change rather than a tool release. Verified that Sys.glob follows the staged symlink: both panels load with 72 and 140 genes and reference-suffix resolution works. The module loses its conda directive and gains the deepvariant/modkit-style guard, because an environment.yml would install the R/Quarto stack without render_report.R. conf/test.config does not set skip_report, so the report runs in all five pipeline tests and the conda shard of nf-test.yml would fail on every dev->main PR; conda is dropped from that matrix until lrsomatic-report reaches bioconda. The alternative, skip_report in test.config, would have cost the report its CI coverage under docker and singularity too. The script block also loses its activate.d sourcing loop -- the image sets QUARTO_SHARE_PATH in ENV -- and --report_src is gone, since it pointed at source that no longer exists here. .gitattributes drops the linguist-vendored line, which leaves it identical to the nf-core template, so its files_unchanged exemption goes with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#195 re-synced the vendored copy to report v1.4.0 and is being closed unmerged: containerising the tool supersedes it, and #196 is branched off it, so its commits land either way. The entry it added would otherwise cite a closed PR and describe a re-sync of a directory that no longer exists. The content stays, because it is still true: report v1.5.0 contains everything v1.4.0 added, so the plugin annotation columns that entry describes do reach users -- through the container rather than through a vendored copy. Dropped its closing note that the container was unchanged, which is no longer the story. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verdict tags tumour-only calls as germline, somatic or subclonal somatic from copy number and tumour purity, which it derives from ASCAT loci, allele and GC content files. Those files only describe the assembly they were built on, and ClairS-TO resolved them under hardcoded GRCh38 names, so a CHM13 run left germline variants untagged and they leaked into somatic.vcf.gz. CLAIRSTO now runs ghcr.io/ljwharbers/clairs-to:0.5.1-verdict-chm13, the fork that resolves those resources from --cna_resource_dir and checks the loci against --ref_fn, so a set that cannot belong to the reference disables Verdict with a warning rather than applying the wrong coordinates. With --genome CHM13 the pipeline builds the resource set from the ASCAT files it already downloads for CHM13, whose names happen to be exactly the ones the resolver expects; only the GC file is renamed. CHM13 has no ascat_loci_rt attribute, so the built set carries no RT_*.txt and LogR is corrected for GC content only, as ASCAT does and as ClairS-TO recommends for CHM13. The three unzips move out from behind the ASCAT gate, since ClairS-TO now wants the same files; UNZIP_RT stays behind it. --clairsto_cna_resources takes a prepared directory instead, and is checked at startup for the layout and for links reaching outside it: only the directory itself is staged into the task, so those files would be missing inside the container and Verdict would be quietly disabled, which is the failure this change exists to prevent. GRCh38 results move too. Besides 0.4.2 to 0.5.1, the fork fixes Verdict's GC window selection, which read the 25bp window's correlations with the other windows rather than with LogR, so the correction window was picked by the wrong quantity. Also in the module: --conda_prefix and the SIF are now selected under -profile apptainer as well as -profile singularity, and --sample_name is no longer passed twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
0.5.1 names its output VCFs after --sample_name, so the module's snv.vcf.gz and indel.vcf.gz went missing and the task failed on collection. The prefix cannot be forced back: ClairS-TO rewrites --snv_output_prefix whenever its value is still the default, so passing 'snv' explicitly changes nothing. Rename after the run instead, keeping the published names, VCFSPLIT and the snapshot file listings as they were. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three PREPARE_REFERENCE_FILES outputs each emit once, and that one item is itself a list of paths. merge() flattens those lists into a single run of files, so the closure was handed five arguments rather than three and every CHM13 run would have died at channel construction. The GRCh38 tests never take this branch, so nothing caught it. Key the three on a shared meta and join instead: join appends the non-key part of each as one element, giving [meta, [loci...], [alleles...], [gc]]. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ran default, clair_only, consensus, deep_only and union with -profile debug,test,apptainer. The only change is the recorded clairsto version in the four snapshots with a tumour-only sample; deep_only has none and is unchanged. No file listing or md5 moved, which is the check that the 0.5.1 output rename was undone correctly. ClairS-TO VCF content is excluded by tests/.nftignore, so the calls themselves are not covered here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CLAIRSTO_CNA_RESOURCES.out.cna_resources is a queue channel carrying a single item, so passing it straight to CLAIRSTO capped the process at one task: with several tumour-only samples on CHM13, only the first was called. .first() turns it into a value channel. The other branch already hands over a value channel, so GRCh38 was never affected, and the test samplesheet has one tumour-only sample, so no snapshot moves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Narrative comment blocks cut to at most two lines that keep the why. The structured annotations stay as they are: MODULE/Input/Output headers, channel-shape lines and the samplesheet meta dictionary. No code changes, so no output moves; comments inside a script block do change the task hash, so CLAIRSTO and CLAIRSTO_CNA_RESOURCES will not resume. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
UNZIP emits its output directory, and the ASCAT loci and allele archives each hold a folder of the same name, so the collected channel carries that one nested directory rather than the per-contig files. `cp -L loci/*` therefore hit a directory and refused: "cp: -r not specified; omitting directory 'loci/G1000_loci_CHM13'". Every CHM13 run died there, three minutes in. ASCAT already consumes this shape -- it builds its prefix as "<staged name>/<staged name>_chr" -- and a set given as a plain path instead arrives as the files themselves, so the module has to accept both. Copy whatever files are under each input directory rather than its immediate entries, and resolve the GC file the same way. The module's fixtures were flat per-contig files, which is the one shape the pipeline never produces for CHM13, so all four tests passed against a layout that could not occur. Add fixtures with the archive's nesting and a test that asserts the per-contig files are flattened out of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sandbox `XDG_CACHE_HOME` in LRSOMATICREPORT alongside `HOME`/`TMPDIR`. Deno prefers it over `$HOME/.cache`, and Singularity/Apptainer inherit the host environment, so wherever a site points it outside the bind-mounted work tree the render dies with `Read-only file system (os error 30): mkdir '<...>/.cache/quarto'` — masked on the R side by `Error running quarto CLI from R`. Reproduced by @AmberVerhasselt on wICE; KU Leuven VSC sets `XDG_CACHE_HOME=$VSC_DATA/.cache` by default, so it hits both clusters. Pre-existing rather than introduced here. `XDG_DATA_HOME`/`XDG_CONFIG_HOME` are unset and fall back to the already-sandboxed `HOME`. Key the third version element in `meta.yml` on the literal command instead of `"versions"`, in both the `output:` and `topics:` blocks, matching every other eval-based module in the repo (sigprofiler/assignment, vcfsplit, cramino, metaextract, fibertoolsrs/*). Nits from the same review: drop the stray blank line that made the fourth #196 changelog entry a loose list item, drop the period before the attribution paren on the v1.4.0-content entry, and drop the hardcoded `:1.5.0` tags from `docs/usage.md` so the tag lives only on the two lines of the module that pin it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1.5.0 renders the SV table's panel_hit column once per row node, when DataTables creates the <tr>, and the table is deferRender + Scroller, so the node is reused for every later draw. Ticking a gene panel filtered the rows correctly but left every already-drawn row with the empty cell it was born with -- a breakend filtered in on MYC 139 kb away, with no reason shown beside it, which is what a reader hit on a T2T DLBCL report. 1.5.1 invalidates the table in redraw(), so display, sort and the column's search box all follow the selection. See ljwharbers/lrsomatic_report#26. The snapshots move because the version reaches them through `render_report.R --version`; nothing else in the report output changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two #196 entries still said v1.5.0, and the one summarising what the bump brings stopped at v1.4.0's changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1.6.0 adds the optional `applies_to` panel column, which scopes a gene to
one table, and makes the report footer read the tool's VERSION at render
time instead of carrying it by hand.
The builtin `lymphoid` panel is re-synced from that release and now uses
the column: 128 genes with per-reference spans from NHL_Genes_{T2T,hg38}.bed
as the SV set, 149 symbols from twist_genes.tsv as the coding-mutation set,
42 on both, giving 235 rows against the previous 72. Coordinates are
carried only on rows that can match an SV, so the snv-only rows have none.
sarcoma is untouched and, carrying no `applies_to` column, still filters
both tables.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three hg38 rows carried CHM13 coordinates: EWSR1, KDM6B and SIK3 were the only three of 128 coordinate rows whose hg38 and t2t cells were identical, off by +463 kb, -96 kb and +15 kb against GENCODE v46. All three are sv-scoped and so matched on position alone, which filtered a real hg38 EWSR1 rearrangement out of a lymphoid-ticked SV table. Replaced with the GENCODE v46 gene spans. PRKBC was a transposition of PRKCB. On an snv row with no coordinates, symbol matching is all it has, so the row matched nothing and PRKCB -- in the panel before the rebuild -- was silently absent. RCK is an obsolete alias for DDX6 and already carried DDX6's coordinates; renamed so panel_hit names a symbol VEP output can carry. FAM46C dropped as a dead duplicate of TENT5C, taking the panel from 235 rows to 234. The versions eval becomes `env -u R_HOME render_report.R --version`. Apptainer forwards the host environment, and R prints "WARNING: ignoring environment value of R_HOME" on stdout when it is set, which lands in the captured version string and fails every versions snapshot. Docker does not forward it, so CI cannot see this. Verified by an apptainer nf-test run that exports R_HOME: 4/4 pass. docs/usage.md gains the applies_to column and the coordinate exception for snv rows, and states the lymphoid behaviour change for anyone repeating a run across it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The image ships its GRCh38 Verdict resources mode 0640, owned by the uid that built the tarball upstream. nf-core's docker profile starts containers with `-u $(id -u):$(id -g)`, so CI could stat those files but not open them, and alleleCounter reports a file it cannot open as one that "does not appear to exist" -- which is what turned the pipeline tests red on this branch. Apptainer maps image ownership onto the caller, so every run on the cluster passed and nothing local could reproduce it. 0.5.1-verdict-chm13-e2987bf makes them world-readable, and disables Verdict with a warning naming permissions when a resource set exists but cannot be read, rather than failing an hour later inside alleleCounter. Pin the commit-suffixed tag rather than the floating one: Nextflow caches SIFs by name, so a floating tag keeps serving whatever was cached first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Amber's review on #196: the entry gave the coding-mutation source as "149 symbols from twist_genes.tsv", so its own arithmetic read 128 + 149 - 42 = 235 against the 234 rows stated in the same sentence. The drop of the FAM46C duplicate is what accounts for the difference and was already described further down the entry, just not next to the numbers. Name it where the count is given. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ic-report Ship lrsomatic_report as a container instead of vendoring its source
dev moved WAKHAN to 0.5.0 (#214), whose module no longer has `vcf_files` / `bed_files`: each solution is one `solution_<ploidy>_<purity>_<conf>/` directory with a merged `integer_profile.{bed,vcf}`, and `solution_rank_N` links rank them. With Wakhan on (the default) the pipeline aborted at launch ("No such property: vcf_files" / "bed_files"); CI could not see it because the test profiles skip Wakhan. - Both subworkflows take `WAKHAN.out.solution_dirs` and use `solution_rank_1`: Padfoot gets its `integer_profile.vcf` (same CN1/CN2/COV1 format Padfoot reads), ReConPlot its `integer_profile.bed` plus `solutions_ranks.tsv`. - The wrapper's Wakhan parser reads the merged profile (hp1_/hp2_copynumber_state; unphased copynumber_state too) and keeps the 0.4 layout as a fallback; the module's bed_output relayout is gone. - PADFOOT_ANNOTATION: a fit table without a matching SV/CN pair arrives as a right-only row of the remainder join and crashed the 4-argument closure; both branches now filter on row[1]. - Fixtures regenerated in the 0.5.0 layout (`wakhan/solution_rank_1/integer_profile.{bed,vcf}`, `wakhan/solutions_ranks.tsv`), headers as Wakhan 0.5.0 writes them; the stub tests use ranked solution directories and cover the right-only rows (a sample with ranks but no rank-1 profile, a SAVANA fit table without copy number). - tests/sv_annotation_compose.nf.test (`small`): `-preview` of the test profile with Wakhan, ASCAT, Padfoot and ReConPlot on, so a renamed or removed caller output fails CI at launch. It fails on the previous wiring with exactly the launch error. Checked on a real Wakhan 0.5.0 run of P215003155 (the dev module on the September inputs): both subworkflows run through the real solution_rank_1 link; the ReConPlot CN table and Padfoot's gene table differ from the 0.4-based ones only by Wakhan's own calling changes (a chr8 haplotype swap, 7 segment states on chr12/chr17). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…NA figures without SVs, check --padfoot_genome
From the independent review of the branch:
- errorStrategy 'ignore' on PADFOOT_* / RECONPLOT_* had replaced base.config's retry on resource
kills (130-145, 104, 175-177); those are now retried first, then ignored.
- Padfoot passes the RepeatMasker species unquoted, so "Mus musculus" split into two arguments
("Ambiguous search term 'Mus'"); mm10 now uses mus_musculus, which the image resolves to taxid
10090 (the Dockerfile smoke test uses the same term).
- SAVANA writes an empty classified.somatic.bedpe for a sample without somatic SVs; the wrapper
stopped ("fewer than 7 columns") and the sample lost its SAVANA figures. An empty BEDPE is now
an empty SV set.
- --padfoot_genome on another assembly than --genome (e.g. hg38 on CHM13) silently annotated T2T
coordinates with hg38 genes; it is now an error at start-up, and the skip warning no longer
suggests it. Two function tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Restore modules/nf-core/untar, which dev dropped but PREPARE_AA_DATA_REPO still includes, and move the BCFTOOLS_NORM joins to its renamed index emit. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…repeats, robust Wakhan input Six Tim-Yu/Padfoot commits on top of 3846215, from the independent review and its follow-ups: - --gff / --rm (the route for CHM13 and other genomes without bundled annotations) crashed three ways: relative paths after Padfoot's chdir, the RepeatMasker .out / BED parsing, and a gzip read/write mismatch. They now take a GENCODE-style GFF3 (plain or gzipped) and a RepeatMasker .out or a chr/start/end/class BED; on the fixture they reproduce the bundled hg38 annotation. - External commands are quoted, so a species such as "Mus musculus" no longer splits. - temp_bps.bed was not closed before bedtools read it, so breakpoint repeat classes were lost after the last flushed 8 KiB block: on P215003155, 28 Severus and 44 SAVANA SVs gain their Repeat_Anno, checked against an independent scan of the bundled repeat file; nothing else in the tables changes. - A header-only Wakhan VCF (copy-neutral tumour) and a very low per-copy coverage no longer crash; contig lengths for the copy-number padding fall back to the reference .fai; the mm10 hint names mus_musculus; insertion alignments are written in a fixed order (they followed the hash seed). Image docker.io/timmy9527/padfoot-repeatmasker:4.2.4-dfam4-padfoot-1748e84 (+ -sif). A new real PADFOOT test runs the custom-annotation route with the chm13 preset on fixtures in tests/fixtures/padfoot_reconplot/custom_annotation/; the docs and schema describe the accepted formats and that --padfoot_genome must match --genome. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…de /home, non-fatal focus panel, cache and gate fixes - PADFOOT stub: `printf '%s\n'` in the GString put a real newline into .command.sh, so the indented heredoc never closed and versions.yml was malformed; any -stub run with Padfoot failed in softwareVersionsToYAML. The stub snapshots recorded the broken file. - Padfoot image 4.2.4-dfam4-padfoot-1748e84-r2 (same Padfoot commit): Dfam moved from /home/mambauser/dfam to /opt/conda/envs/padfoot/share/dfam. With /home bound into the container (site singularity.conf or runOptions) RepeatMasker failed and every Padfoot result was silently ignored; checked with -B <empty>:/home on both images. The image records PADFOOT_COMMIT, which versions.yml now reports (Padfoot's own version string never changes). - ReConPlot: the optional focus panel no longer fails the task (a failure is logged and the per-chromosome and genome-wide figures are kept, as the docs say); an empty region list is no longer a crash in the wrapper and is rejected at start-up; --reconplot_genes is checked against a symbol pattern (a quote broke the command line) and both it and --reconplot_baf_track warn when set without --reconplot_regions. - -resume: the wrapper directory is cached on its top level only, so a change under R/ reused stale figures; a digest of the wrapper files now goes into its meta map. - Padfoot/ReConPlot are skipped (with a start-up warning) when no container engine is in use, not only under the conda/mamba profiles. - Tests: two ReConPlot tests run the pipeline's own arguments with a focus panel (thousands separators, an unknown gene, the BAF track) and a failing focus panel; two validator tests. Docs: the Severus SV table columns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…graph The -preview tests only asserted that the run composed, which is also true when a gate silently drops Padfoot or ReConPlot. They now write the workflow graph (-with-dag dag.mmd) and assert on its process nodes: with Padfoot on, both PADFOOT_* and all three RECONPLOT_* steps and their callers must be present; with --skip_padfoot, no PADFOOT_* step and all three RECONPLOT_* steps. Checked against deliberately broken copies of the branch: the tests now fail when Padfoot's genome gate or the container-engine gate is wrongly false and when --skip_padfoot is ignored (the old tests passed the first of these), and still fail at launch on the pre-Wakhan-0.5.0 wiring. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…test Re-align the fixture README table as prettier writes it, keep the Wakhan fixture BED's empty last column (a trailing tab, as Wakhan writes it) out of the whitespace trimmer, and exempt the -preview pipeline test from the versions.yml snapshot rule: a preview run publishes nothing, so it asserts on the workflow graph instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
savana_contigs, savana_g1000_vcf and aa_data_repo_url are set from --genome at runtime like the other genome attributes, so they cannot be declared in nextflow.config and nf-core lint rejected their schema entries. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CLAIR3_MODEL fell through to the default publishDir and copied the model into <outdir>/clair3/. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
SAVANA writes <id>.classified.somatic.bedpe only from its model classifier
(ONT). With --pb, `savana classify` and `savana to` run classify_pacbio,
which writes the classified somatic VCF alone. RECONPLOT_FIGURES joined on
the BEDPE, so every PacBio sample, paired or tumour-only, silently lost its
reconplot/savana figures (seen on the C8 PacBio pair).
- RECONPLOT_FIGURES takes the classified somatic VCF, which SAVANA writes on
every platform and Padfoot already reads; the now-unused somatic_bedpe
emits leave PAIRED_SAVANA / TUMORONLY_SAVANA.
- Wrapper: --sv-format defaults to auto (the BEDPE when staged, else the
VCF). On a real ONT sample (BL1, 2,084 junctions) both give the same
junctions and CN table; only an insertion's second position differs by
the 1 bp SAVANA's BEDPE adds. VERSION bumped.
- New RECONPLOT module test for the VCF-only case.
- The PADFOOT_ANNOTATION / RECONPLOT_FIGURES stub tests call their local
closures with .call(): Nextflow >= 26.04 strict syntax rejects mk(...)
("`mk` is not defined"), which failed the latest-everything shards
(hidden by continue-on-error).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pr189 Add Padfoot annotation and ReConPlot figures for the SV/CNA callers
Resolve conflict in validateInputParameters(): keep both dev's validateSvAnnotationParams() and this branch's validateSeverusWhitelist(). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
meta.yml (versions_tabix and topics.versions) still documented the old grep -oP command; it now matches the sed expression in main.nf. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Optional whitelist Severus calling (--severus_whitelist), patched Severus 1.7
…on-busybox fix: VEPPLUGIN tabix version eval fails with BusyBox grep (REVEL, EVE)
…imit With dev merged, the LRSOMATIC body exceeded Groovy's 65,535-character string limit and failed to compile. Moving the fiber-seq block and the AmpliconArchitect MD5 default out brings it to ~61 K; behaviour unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Unmerged coral (aa6c6cf) and the merged branch both produce these md5s for the haplotagged sample1/sample3 tumour BAMs, so the old values were stale before the merge. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The nf-core module's process_low label gives it 2 cpus, which cannot merge two whole-genome replicate BAMs inside the label's 4 h: every validation-cohort replicate sample timed out on the first attempt. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Addresses #127 (comment 4809238681): export_probabilities_per_mutation only works with VCF input, so the COSMIC fit now reads the PASS VCF; the matrices only gate which contexts are fitted. cosmic_fit ignores --volume, so the genome volume is set through SIGPROFILER{MATRIXGENERATOR,ASSIGNMENT}_VOLUME. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This PR is against the
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| --collapse_to_SBS96 ${collapse} \\ | ||
| --genome_build ${genome} \\ | ||
| --cosmic_version ${cosmic_version} \\ | ||
| --export_probabilities_per_mutation True \\ |
There was a problem hiding this comment.
No CI test runs this code path. The only small test for this module is -stub. The stub just touches Decomposed_Mutation_Probabilities_<sample>.txt, and conf/test.config sets skip_signatures = true. So nothing exercises the real command: cosmic_fit --input_type vcf --export_probabilities_per_mutation True for --context_type 96 / DINUC / ID.
Untested parts include:
- the internal SigProfilerMatrixGenerator run picking up the genome through
SIGPROFILERMATRIXGENERATOR_VOLUME; - per-mutation export for DBS78 and ID83, not just SBS96;
- the real output paths matching the
*/*/Assignment_Solution/Activities/Decomposed_Mutation_Probabilities/*.txtglob. That emit isoptional: true, so a wrong path fails silently.
Please run the Slurm test array (or a manual run with the SigProfiler genome) on GRCh38 and CHM13-T2T. Check that the per-mutation files exist for every fitted context.
| mkdir -p input_${ctx} | ||
| ${reader} ${vcf} > input_${ctx}/${sample}.vcf | ||
| SigProfilerAssignment cosmic_fit \\ | ||
| input_${ctx}/ \\ |
There was a problem hiding this comment.
--sigprofiler_matrix_args no longer applies to the fit. With --input_type vcf, cosmic_fit rebuilds the matrix itself from input_${ctx}/. Options such as --exome or --bed_file passed through --sigprofiler_matrix_args therefore change the published matrices/ but not the matrix the COSMIC fit uses.
There is a second effect: the matrix_total gate on line 42 reads the restricted matrix, but the fit runs on the whole VCF. A context can be skipped even though the VCF has mutations of that type outside the restricted region.
Either pass the matching options to cosmic_fit (it accepts --exome / --bed_file too), or document in docs/usage.md and the schema help_text that they must also go in --sigprofiler_assignment_args.
| - [#197](https://github.com/IntGenomicsLab/lrsomatic/pull/197) - Added CHM13 support for ClairS-TO's Verdict module, which tags tumour-only calls as germline, somatic or subclonal somatic; its resources were GRCh38-only, so on CHM13 germline variants leaked into `somatic.vcf.gz`. With `--genome CHM13 --skip_ascat` the pipeline builds a CHM13 resource set from the ASCAT files it already downloads and passes it as `--cna_resource_dir`; a prepared directory can be given with `--clairsto_cna_resources` (validated at launch). Without `--skip_ascat` tagging comes from ASCAT's own tables instead (next entry) (@ljwharbers). | ||
| - [#197](https://github.com/IntGenomicsLab/lrsomatic/pull/197) - Added `CLAIRSTO_VERDICT_TAG`: when ASCAT is in the run, Verdict's germline tagging is computed from ASCAT's purity, ploidy and segments instead of Verdict's own estimate, so `CLAIRSTO` runs with `--disable_verdict` and ASCAT runs before small variant calling. Output names are unchanged. The tables the tags were computed from are published as `<sample>_Tumor_Purity_Ploidy.txt` and `<sample>_Tumor_CNA.txt`, also on `--skip_ascat` runs (@ljwharbers). | ||
| - [#197](https://github.com/IntGenomicsLab/lrsomatic/pull/197) - Added a stub nf-test for `TUMORONLY_SMALLVAR` covering both germline tagging paths (tag `small`) (@ljwharbers). | ||
| - [#190](https://github.com/IntGenomicsLab/lrsomatic/pull/190) - Added mutational signature analysis: SigProfilerMatrixGenerator matrices (SBS/DBS/ID) and per-sample COSMIC signature fitting with SigProfilerAssignment on the phased somatic VCF, with CHM13-T2T support via a purpose-built image (`ghcr.io/ljwharbers/sigprofiler`) and CHM13-renormalised COSMIC signatures; new `--sigprofiler_*` / `--download_sigprofiler_genome` / `--skip_signatures` parameters (@ljwharbers). |
There was a problem hiding this comment.
No CHANGELOG entry for #220. REVIEW.md asks for an entry under v1.2.0dev with the PR number. This PR changes SigProfilerAssignment to fit from the VCF and adds the probabilities_per_mutation output (Decomposed_Mutation_Probabilities/), so it needs one.
|
Automated review: 3 findings, all inline. Scope: this PR targets Findings:
A human reviewer needs to run: the Slurm test array, or a run with the SigProfiler genome installed and 🤖 Generated with Claude Code |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR checklist
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).