Move SigProfiler to the corrected CHM13-T2T payload (SPMG 7894689) - #216
Merged
Merged
Conversation
The CHM13 archive at chm13_release_2026-08 was replaced by the rebuild that fixes SigProfilerMatrixGenerator#251 (sha256 fe68e840...dc1d), while the 1.3.6-chm13-28a9ce8 image still checks the superseded checksums, so --download_sigprofiler_genome failed verification in SIGPROFILER_INSTALL. Use the 1.3.6-chm13-7894689 image, which carries the new checksums, and fetch the payload from the versioned chm13_release_2026-09 path, so the 2026-08 path can keep serving the original archive for older commits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ljwharbers
marked this pull request as ready for review
October 2, 2026 13:26
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| ? 'oras://ghcr.io/ljwharbers/sigprofiler-sif:1.3.6-chm13-28a9ce8' | ||
| : 'ghcr.io/ljwharbers/sigprofiler:1.3.6-chm13-28a9ce8'}" | ||
| ? 'oras://ghcr.io/ljwharbers/sigprofiler-sif:1.3.6-chm13-7894689' | ||
| : 'ghcr.io/ljwharbers/sigprofiler:1.3.6-chm13-7894689'}" |
There was a problem hiding this comment.
Old CHM13 volumes passed with --sigprofiler_genome_dir are not caught. The PR says volumes installed before this change "no longer pass verification with the new image and must be reinstalled". The only verification, though, is the is_genome_installed assert in SIGPROFILER_INSTALL. When the user passes --sigprofiler_genome_dir, PREPARE_SIGNATURES (subworkflows/local/prepare_signatures.nf:34-41) only checks that tsb/<genome>/ exists and holds 24 .txt files. That includes the published <outdir>/cache/sigprofiler/volume from earlier runs, which docs/usage.md tells users to reuse. A stale chm13_release_2026-08 volume passes that check and goes straight to this image, with one of two results:
- this process fails late with an unclear checksum error from the tool, or
- the tool doesn't re-verify, and the strand matrices (SBS288/SBS384 etc.) are built from the old payload without any warning.
Neither result is documented outside the CHANGELOG. Suggested fixes:
- run the same
rgm.ReferenceGenomeManager(...).is_genome_installed(genome)check on the user-supplied volume, either in a small validation step or at the top of this script, so a stale volume fails up front with a "reinstall with--download_sigprofiler_genome" message - at the least, add a note to the
--sigprofiler_genome_dirhelp_textinnextflow_schema.jsonand todocs/usage.mdsaying that CHM13 volumes installed before this release must be reinstalled
| ? 'oras://ghcr.io/ljwharbers/sigprofiler-sif:1.3.6-chm13-28a9ce8' | ||
| : 'ghcr.io/ljwharbers/sigprofiler:1.3.6-chm13-28a9ce8'}" | ||
| ? 'oras://ghcr.io/ljwharbers/sigprofiler-sif:1.3.6-chm13-7894689' | ||
| : 'ghcr.io/ljwharbers/sigprofiler:1.3.6-chm13-7894689'}" |
There was a problem hiding this comment.
Image registry: REVIEW.md asks for custom images to use the oras://docker.io/...-sif / docker.io/... pair, with Docker Hub rather than ghcr for large Apptainer images. This PR builds and pins new 1.3.6-chm13-7894689 tags on ghcr.io in all three SigProfiler modules (install, matrixgenerator, assignment). Since new images are being published anyway, please push them to Docker Hub too and point the singularity/docker ternary at oras://docker.io/ljwharbers/sigprofiler-sif:1.3.6-chm13-7894689 / docker.io/ljwharbers/sigprofiler:1.3.6-chm13-7894689. If that changes, the ghcr.io/ljwharbers/sigprofiler mention in docs/usage.md:392 needs the same update.
| // not yet hosted on the AlexandrovLab FTP, so it is fetched from the IntGenomicsLab Globus collection | ||
| sigprofiler_genome : "CHM13-T2T", | ||
| sigprofiler_genome_url : "https://g-608c0c.273595.03c0.data.globus.org/chm13_release_2026-08/CHM13-T2T.tar.gz", | ||
| sigprofiler_genome_url : "https://g-608c0c.273595.03c0.data.globus.org/chm13_release_2026-09/CHM13-T2T.tar.gz", |
There was a problem hiding this comment.
No CI coverage: No small nf-test runs this code. The test profile sets skip_signatures = true, and all tests are GRCh38 anyway. So CI checks neither the new URL, the new image tags, nor the image's embedded CHM13 checksums against this payload. Before merging, please run the Slurm test array or the standalone CHM13 --download_sigprofiler_genome run listed in the PR's to-do, plus the old-vs-new BL1 matrix/fit comparison. Please also run a GRCh38 signatures run on the new image: the image changed for every genome, not just CHM13, and nothing here tests the GRCh38 checksums in the rebuilt SPMG fork.
|
Automated review: 3 findings, posted inline.
For a human reviewer to run (I can't run tests, so none of this is verified):
|
|
Contributor
|
Checked this against the failing runs on our side (Mindwell jobs 11647055 and 11648551, both failing
The validation in the description looks good. Fine to merge once the stale |
Addresses review points 1 and 3 on #216. SIGPROFILER_MATRIXGENERATOR already re-checks the payload for every sample and fails each task with a generic checksum error. The new SIGPROFILER_VERIFY step checks a --sigprofiler_genome_dir volume once, before any sample work, and stops with the checksum table and reinstall instructions. The 7894689 image also carries upstream's corrected GRCh38 payload (the AlexandrovLab FTP has served it as GRCh38.tar.gz since 2026-09-25; the old one is GRCh38_Legacy), so GRCh38 volumes installed before this change need reinstalling as well. The docs, schema help_text and CHANGELOG now say so. The new small-tagged module test feeds stale GRCh38 and CHM13-T2T volumes and checks that each is rejected, with the chr1 checksum the image expects, so CI ties the pinned image to the payload revisions it downloads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
@claude review |
| // No conda: the image uses CHM13-T2T forks of SigProfilerMatrixGenerator (#250) and SigProfilerAssignment; see meta.yml | ||
| container "${(workflow.containerEngine == 'singularity' || workflow.containerEngine == 'apptainer') && !task.ext.singularity_pull_docker_container | ||
| ? 'oras://ghcr.io/ljwharbers/sigprofiler-sif:1.3.6-chm13-7894689' | ||
| : 'ghcr.io/ljwharbers/sigprofiler:1.3.6-chm13-7894689'}" |
There was a problem hiding this comment.
Registry convention (still open from the first review). The new SIGPROFILER_VERIFY module and the three bumped modules still pull the custom image from ghcr.io. REVIEW.md asks for the oras://docker.io/...-sif / docker.io/... pair for custom images, and says "Docker Hub, not ghcr, for large Apptainer images". This bundled SigProfiler SIF is one of those large images. Please push 1.3.6-chm13-7894689 to Docker Hub and point all four modules there, or say in the PR why ghcr is acceptable here.
Collaborator
Author
There was a problem hiding this comment.
We won't address this in this PR. The SigProfiler image stays on ghcr (ghcr.io/ljwharbers/sigprofiler / oras://ghcr.io/ljwharbers/sigprofiler-sif), the registry it has used since #190.
Comment on lines
+33
to
+40
| if not manager.is_genome_installed("${genome}"): | ||
| manager.print_genome_checksum_verification_report("${genome}") | ||
| sys.exit( | ||
| "ERROR: the ${genome} payload in --sigprofiler_genome_dir does not match the checksums of this pipeline's " | ||
| "SigProfilerMatrixGenerator. GRCh38 and CHM13-T2T payloads installed before lrsomatic PR #216 are a " | ||
| "superseded revision. Reinstall it with --download_sigprofiler_genome (published to " | ||
| "<outdir>/cache/sigprofiler/volume) and pass that directory on later runs." | ||
| ) |
There was a problem hiding this comment.
The error message can name the wrong cause. is_genome_installed returns false for any mismatch: a stale revision, a truncated or corrupted chromosome file, or a genome whose checksums this image doesn't register (for example a non-default --sigprofiler_genome). The message always says the payload "is a superseded revision" from before PR #216. A user with a damaged download, or on a genome that isn't GRCh38 or CHM13-T2T, gets the wrong diagnosis.
Users also see a release, not a PR number. Please word the message (and the matching schema help_text) by cause, for example "does not match the checksums … (stale revision from lrsomatic < 1.2.0, or an incomplete/corrupted install)". If unregistered genomes are meant to be supported, check genome in CHECKSUMS separately.
Collaborator
Author
There was a problem hiding this comment.
Fixed in f20a2a2. The check now reports one of three causes, using rgm.CHECKSUMS from the pinned image:
- Genome with no checksums in the image (
genome not in CHECKSUMS, tested separately): "this pipeline's SigProfilerMatrixGenerator has no checksums for (registered: …). Set --sigprofiler_genome to one of them or use --skip_signatures." - Missing files: "is an incomplete install: N of M chromosome files are missing (…)".
- All files present, checksums differ: "does not match the checksums … Either it is a stale payload (GRCh38 and CHM13-T2T installed for lrsomatic < 1.2.0 are a superseded revision) or the copy is corrupted."
The reinstall instructions follow in each case. The schema help_text and docs/usage.md now say "lrsomatic < 1.2.0" instead of the PR number, and cover the corrupted/incomplete case.
The module test now covers all three cases. The new mismatch case uses a fixture with all 24 CHM13-T2T files present but empty. The assertions match only the rendered message. Nextflow echoes the script source when a task fails, so plain substrings such as "Reinstall it with" also matched the template text. Results: 5/5 pass under apptainer (Slurm job 11649374).
| genome | ||
| ) | ||
| // Hand on the user's directory itself, released only once it has been verified | ||
| sigprofiler_volume = SIGPROFILER_VERIFY.out.verified.map { _verified -> volume_dir } |
There was a problem hiding this comment.
No small test covers this wiring or the image bump. The new small module test only checks the rejection path on a 1-file fixture. Nothing in PR CI runs:
- the success path, where
verifiedis emitted and this.maphandsvolume_dirtoSIGPROFILER_MATRIXGENERATORas a value channel, so every sample gets it; PREPARE_SIGNATURESitself;- the new image in
SIGPROFILER_INSTALL,SIGPROFILER_MATRIXGENERATORandSIGPROFILER_ASSIGNMENT.
That's because conf/test.config sets skip_signatures = true. The Slurm results in the description are the only evidence for these paths. Please confirm they were run on the current head 1d3b7c7 with more than one sample using --sigprofiler_genome_dir. The GRCh38 C8 check covered only one sample. A stub-mode pipeline or subworkflow test with signatures on would catch future regressions in this wiring.
Collaborator
Author
There was a problem hiding this comment.
Added stub tests tagged small in f20a2a2, so PR CI now runs them:
subworkflows/local/tests/prepare_signatures.nf.test--sigprofiler_genome_dirbranch:SIGPROFILER_VERIFYruns andSIGPROFILER_INSTALLdoes not.volumeemits exactly one item, the user's directory itself.- Download branch: the reverse.
modules/local/sigprofiler/matrixgenerator/tests/main.nf.test:PREPARE_SIGNATURESruns insetup. Itsvolumeis mapped exactly as inworkflows/lrsomatic.nf:909and given to two samples. Both producesbs96, so the volume reaches every sample.modules/local/sigprofiler/assignment/tests/main.nf.test: stub fit of a single sample.
In the matrixgenerator and assignment tests the versions eval runs inside the pinned image, so the docker and singularity shards each pull 1.3.6-chm13-7894689. SIGPROFILER_INSTALL uses the same tag, and its stub runs in that image too. All 9 SigProfiler tests pass under apptainer (Slurm job 11649374). Docker is checked by CI on this push.
I have not yet run the full Slurm pipeline array on the new head (multi-sample, --sigprofiler_genome_dir, CHM13 and GRCh38). Only test code and the error-message strings changed since 1d3b7c7. That run is still to be done.
|
Automated re-review of
The first review's stale-volume finding is addressed by A human needs to check these (I can't run tests, so none of this is verified):
🤖 Generated with Claude Code |
… chain SIGPROFILER_VERIFY reported every failed check as a superseded payload from before PR #216. It now tells apart a genome the image has no checksums for, an incomplete install (lists the missing chromosome files), and a checksum mismatch (stale payload from lrsomatic < 1.2.0, or a corrupted copy). The schema help_text and usage docs say the same. PR CI ran nothing of the signatures step (conf/test.config skips it). New stub tests, all tagged small: - PREPARE_SIGNATURES: the --sigprofiler_genome_dir branch verifies and hands on the user's directory; the download branch installs. - SIGPROFILER_MATRIXGENERATOR: the volume from PREPARE_SIGNATURES, wired as in workflows/lrsomatic.nf, reaches two samples. - SIGPROFILER_ASSIGNMENT: runs in the pinned image. The versions evals run inside the image, so the image tag is checked under each CI profile. The verify module test gains the mismatch and unregistered genome cases and asserts on rendered text only, since Nextflow echoes the script source on failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
@claude review |
| else: | ||
| cause = ( | ||
| "does not match the checksums of this pipeline's SigProfilerMatrixGenerator. Either it is a stale " | ||
| "payload (GRCh38 and CHM13-T2T installed for lrsomatic < 1.2.0 are a superseded revision) or the " |
There was a problem hiding this comment.
The error points at the wrong version. No released lrsomatic has a SigProfiler payload. Signatures came in with #190, under v1.2.0dev, and manifest.version is still 1.1.0. So "installed for lrsomatic < 1.2.0" describes no real volume.
Stale volumes come from two places: 1.2.0dev builds from before #216 (image 1.3.6-chm13-28a9ce8), and a standalone SigProfilerMatrixGenerator install GRCh38 run before 2026-09-25. Someone who installed their volume from a dev commit last month thinks of it as 1.2.0. They will read this message as "not stale, so my copy must be corrupted", which is the wrong cause.
The same wording is in the sigprofiler_genome_dir help_text in nextflow_schema.json (line 484). Something like this would fit both places:
Suggested change
| "payload (GRCh38 and CHM13-T2T installed for lrsomatic < 1.2.0 are a superseded revision) or the " | |
| "payload (GRCh38 and CHM13-T2T volumes installed before SigProfilerMatrixGenerator 7894689 / lrsomatic " | |
| "#216, i.e. with an earlier 1.2.0dev build or an upstream GRCh38 install before 2026-09-25, are a " | |
| "superseded revision) or the " |
|
Automated re-review of
The earlier findings are now addressed:
A human needs to check these (I can't run tests, so none of this is verified):
🤖 Generated with Claude Code |
ljwharbers
added a commit
that referenced
this pull request
Oct 2, 2026
Brings in #216 (SigProfiler CHM13 payload fix). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
2 participants
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.
What and why
A user reported that
--download_sigprofiler_genomeon CHM13 fails inSIGPROFILER_INSTALLwithCHM13-T2T failed the SigProfilerMatrixGenerator checksum verification.Cause: on 2026-09-25 the CHM13-T2T archive at
chm13_release_2026-08/was replaced by the rebuild that fixes SigProfilerMatrixGenerator#251, which no longer drops inclusive transcript-end bases. The new archive (sha256fe68e840…dc1d) matches the per-chromosome checksums in #250 commit7894689. The pipeline image1.3.6-chm13-28a9ce8still expected the old ones, so every fresh install failed. The checksum assert did its job.Already done outside this PR:
chm13_release_2026-09/, and the original (9fe0d9e7…e0ff) is back atchm13_release_2026-08/. Currentdevcan download again, and older commits stay reproducible. Both URLs were checked over HTTPS.ghcr.io/ljwharbers/sigprofiler:1.3.6-chm13-7894689andoras://ghcr.io/ljwharbers/sigprofiler-sif:1.3.6-chm13-7894689were built from the forkcontainer/chm13(run). The smoke test now asserts the new chr1 checksum, and in the SIFCHECKSUMS["CHM13-T2T"]["1"]reads8683547c….This PR:
SIGPROFILER_INSTALL,SIGPROFILER_MATRIXGENERATORandSIGPROFILER_ASSIGNMENTto1.3.6-chm13-7894689sigprofiler_genome_urlatchm13_release_2026-09CHM13 volumes installed before this change no longer pass verification with the new image and must be reinstalled. The payload change is about 0.1% of bases, almost all strand-label recoding (one strand → both strands transcribed). On real samples only the transcribed-strand matrices move, and only slightly; SBS96 and the COSMIC fit are unchanged (results below).
Validation
All runs on Mindwell with Apptainer, PR head
f7a45c4. CI does not cover this: the test profile setsskip_signatures = true.Standalone download test (Slurm job 11649322, 1.5 min). Runs the real
PREPARE_SIGNATURESwithdownload_genome = true→SIGNATURES_BCFTOOLS_VIEW→SIGPROFILER_MATRIXGENERATORon BL1 (ONT) and FL11 (PacBio).SIGPROFILER_INSTALLdownloadedchm13_release_2026-09and passed the checksum assert: 24 chromosome files, chr18683547c….Matrices vs the 2026-09-04 run on the old payload. SBS96, DBS78 and ID83 are byte-identical for both samples. SBS288 and SBS384 differ, as expected:
COSMIC v3.6 SBS96 fit of BL1 with the new image (Slurm job 11649326).
Assignment_Solution_Activities.txtis byte-identical to the old fit: SBS1 2942, SBS5 21875.CHANGELOG PR number filled in.
Review follow-up (
1d3b7c7)The new image also carries upstream's corrected GRCh38 payload. The AlexandrovLab FTP has served it as
GRCh38.tar.gzsince 2026-09-25; the old archive is nowGRCh38_Legacy. So GRCh38 volumes installed before this PR must be reinstalled too. The old image's GRCh38 download is presumably already broken ondev, since it expects the old checksums.New
SIGPROFILER_VERIFYstep: it checks a--sigprofiler_genome_dirvolume once, before any sample work, and stops with the checksum table and reinstall instructions. The docs, schemahelp_textand CHANGELOG now cover both genomes. Slurm array 11649347, 5/5 pass:modules/local/sigprofiler/verify(--profile=+singularity), 3/3 pass. This newsmalltest runs in PR CI: stale GRCh38 and CHM13-T2T volumes are rejected, and the report lists the chr1 MD5 the image expects for each.chm13_release_2026-09, passed with--sigprofiler_genome_dir: verified; BL1 SBS96/SBS384/DBS78/ID83 identical to the download-test run.chm13_release_2026-08volume:SIGPROFILER_VERIFYstops the run with the reinstall message; noSIGPROFILER_MATRIXGENERATORtask runs.--download_sigprofiler_genomeGRCh38 from the FTP with the new image: install verified (chr1570ba2c0…). C8 (GRCh38, 147 PASS calls), new image + new payload vs old image + old payload: SBS96, SBS288, SBS384 and ID83 identical; no DBS in this sample. That is a small sample, so it is only a light check of the GRCh38 strand matrices.PR checklist
nf-core pipelines lint): 0 failures. The 12 warnings are pre-existing (local subworkflows withoutmeta.yml).prek run --all-filespasses.CHANGELOG.mdis updated.🤖 Generated with Claude Code