Repository navigation
Update Clair3, ClairS, DeepVariant and DeepSomatic - #213
Merged
AmberVerhasselt merged 8 commits intoOct 2, 2026
Merged
Conversation
Clair3 1.2.0 -> 2.0.3. v2 runs on PyTorch and cannot load the v1 TensorFlow models the pipeline downloaded as tarballs from ONT's CDN. The image bundles HKU's models and ONT's recent R10.4.1 ones, so a model in that set reaches CLAIR3 as an empty path and runs from the image; any other model is fetched as a PyTorch directory (pileup.pt, full_alignment.pt) from HKU by the new CLAIR3_MODEL step, from the Rerio conversion for r9/r10 names. WGET/UNTAR are no longer used for models and the untar module is removed. The flag is --model_path now that run_clair3 is argparse-based. No GPU image was published for 2.0.3; v2.0.2_gpu differs only by --gender, which is not used. ClairS 0.4.4 -> 0.5.1: bundles Clair3 2.0.3; all three platforms the pipeline uses are in the image. DeepVariant 1.9.0 -> 1.10.0 through nf-core update, with the GPU image, labels, --sample_name and optional-gVCF patches redone on the new files; the subworkflow's new with_phasing input is false (LongPhase phases later). DeepSomatic 1.7.0 -> 1.10.0. For both, make_examples now gets the same --checkpoint as call_variants and reads channels, pileup size and calling flags from the model's model.example_info.json. The hard-coded flags no longer matched: the DeepVariant ONT model is 199 wide with the fuzzy channel, the paired DeepSomatic PacBio model is 147 wide with 9 channels. Checked on the chr19 test BAMs: example shapes match each model. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The old deepvariant/postprocessvariants carried two edits its .diff did
not record: a ${meta.id}_germline default prefix and the process_short
label, while makeexamples did not carry the process_short its .diff
listed. Redoing the patches from the .diff files put process_short on
the wrong module and dropped the _germline prefix. Without it the
tumour-only germline VCF is <id>.vcf.gz, the name PHASING_HAPLOTYPING's
BCFTOOLS_CONCAT writes, so the concat clobbered its own input
("Failed to open sample3.vcf.gz: unknown file type", deep_only test).
Found by rebuilding every patched module as upstream@sha + its .diff and
comparing with the files at c17991e; DeepVariant was the only mismatch.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drift: caller versions, WGET/UNTAR replaced by CLAIR3_MODEL (clair_only downloads its non-bundled model), and sample2's WhatsHap stats from the Clair3 v2 germline calls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drift beyond the caller versions is the WhatsHap phasing stats, which follow the new germline calls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The deepvariant subworkflow update brought in an upstream equality test that includes deepvariant/rundeepvariant, a module this pipeline does not install, so nextflow lint failed with 'Invalid include source'. Component tests belong to nf-core/modules and are not run here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ljwharbers
marked this pull request as ready for review
October 1, 2026 13:14
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| ? '--channel_list "BASE_CHANNELS,haplotype,base_6ma" --alt_aligned_pileup "diff_channels" --pileup_image_width "147"' | ||
| : '--channel_list "BASE_CHANNELS,haplotype" --alt_aligned_pileup "diff_channels" --pileup_image_width "99"' | ||
| } | ||
| ext.args = { "--checkpoint '/opt/models/${meta.platform == 'pb' ? 'pacbio' : 'ont_r104'}'" } |
There was a problem hiding this comment.
No PR-CI coverage for the DeepVariant/DeepSomatic changes. PR CI only runs tests tagged small, and only tests/default.nf.test has that tag. It runs with germline_var_keep = 'clair' and somatic_var_keep = 'clair', and its snapshot has no DEEPVARIANT_*/DEEPSOMATIC_* entries. That leaves several changes without CI coverage: the 1.10 image bump, the new --checkpoint-driven make_examples flags here and at L208, the rebuilt DeepVariant patches (including the ${meta.id}_germline prefix), and the new gzi/with_phasing subworkflow arguments. Only the extended tests (deep_only, consensus, union, chm13) exercise them. Please run the Slurm test array and link the results before merge.
Collaborator
Author
There was a problem hiding this comment.
Ran the Slurm array on fc0e823 (this head, with #212 merged in) on Mindwell, -profile debug,test,apptainer --update-snapshot, job 11645738. All seven pass: default, clair_only, consensus, deep_only, union, chm13 and the tumour-only subworkflow test. git status was clean afterwards, so no snapshot moved with the review changes.
The DeepVariant/DeepSomatic outputs hold real calls, not empty files (records / PASS):
| sample | DV germline | DS somatic |
|---|---|---|
| sample1 | 18529 / 11465 | 9094 / 6 |
| sample2 | 7499 / 4377 | 39297 / 0 |
| sample3 | 20753 / 12879 | 14429 / 208 |
The deep_only and union counts are identical. The few somatic PASS calls are expected on the small chr subsets.
The real-data C8 comparison found a bcftools 1.24 problem in #212 (see the PR description and #212). It's fixed and resubmitted as Slurm 11645745. I'll post the caller comparison here when it finishes.
| ? '--channel_list "BASE_CHANNELS,haplotype" --alt_aligned_pileup "diff_channels" --pileup_image_width "99"' | ||
| : '--channel_list "BASE_CHANNELS,haplotype,allele_frequency" --alt_aligned_pileup "diff_channels" --pileup_image_width "99"' | ||
| } | ||
| ext.args = { "--checkpoint '/opt/models/deepsomatic/${meta.platform == 'pb' ? 'pacbio' : 'ont'}${meta.paired_data ? '' : '_tumor_only'}'" } |
There was a problem hiding this comment.
Possible silent loss of calls if a DeepSomatic checkpoint enables the small model. With --checkpoint, make_examples_somatic now takes whatever flags model.example_info.json sets. If any of the four DS 1.10 checkpoints (pacbio, ont, *_tumor_only) turns on the small model, those sites are removed from the examples and written to ${prefix}.examples_call_variant_outputs…. The local module's glob is ${prefix}_call_variant_outputs.tfrecord-* with arity: "0..*", and the PR description says it is missing .examples. So the glob would match nothing, postprocess would get no --small_model_cvo_records, and those calls would drop out of the VCF without an error. The DeepVariant JSON does enable the small model, so this is a real possibility. Please confirm none of the DS JSONs sets the small-model flag, or pass --nocall_small_model_examples (or the 1.10 equivalent) explicitly so a future model can't change this.
Collaborator
Author
There was a problem hiding this comment.
No calls are lost with the current models, and fc0e823 makes sure that stays true.
I checked the four DeepSomatic 1.10 checkpoints in google/deepsomatic:1.10.0. Paired pacbio and ont set trained_small_model_path and the small-model GQ thresholds, but not call_small_model_examples. That flag defaults to false. pacbio_tumor_only and ont_tumor_only set none of these. DeepVariant's pacbio/ont_r104 JSONs do set call_small_model_examples: true, which is where the difference comes from.
apply_flags_for_calling (make_examples_core.py) skips any JSON flag that is also given on the command line. So DEEPSOMATIC_MAKEEXAMPLES now passes --nocall_small_model_examples with --checkpoint, and a future checkpoint can't turn the small model on behind the module's back. Enabling it properly (the .examples glob fix) stays a follow-up.
| container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ? | ||
| 'docker.io/hkubal/clairs:v0.4.4': | ||
| 'docker.io/hkubal/clairs:v0.4.4' }" | ||
| container "docker.io/hkubal/clairs:v0.5.1" |
There was a problem hiding this comment.
This removes the singularity/docker ternary, which REVIEW.md says container directives should keep. Please keep the ternary with both branches at v0.5.1, or use the oras://docker.io/...-sif / docker.io/... pair that REVIEW.md recommends for large Apptainer images.
Collaborator
Author
There was a problem hiding this comment.
Fixed in fc0e823. The ternary is back with docker.io/hkubal/clairs:v0.5.1 on both branches, written in the same containerEngine in ['singularity', 'apptainer'] form as the Clair3 module. HKU's image is a third-party Docker Hub image, so there's no -sif counterpart to point at.
| UNTAR ( | ||
| WGET.out.outfile | ||
| ) | ||
| CLAIR3_MODEL ( clair3_model_sources.download ) |
There was a problem hiding this comment.
The new CLAIR3_MODEL download path has no small test. Only tests/clair_only.nf.test, which is tagged extended, needs a non-bundled model. default.nf.test resolves to a bundled model, so PR CI never runs CLAIR3_MODEL or the --model_path=<downloaded dir> branch in CLAIR3.
Several things depend on the hard-coded clair3_bundled list and the URL scheme, and none of them is checked in CI:
r1041_e82_400bps_sup_v430,sup_v420and260bps_sup_v400(all reachable fromclair3_modelMap) exist underclair3_models_rerio_pytorch/<name>/{pileup,full_alignment}.pt.- Every name in
clair3_bundledis present in both the biocontainer'smodels/and the GPU image's/opt/models.
Also, the PR description says SUP v6.0 is bundled, but the list has only hac_v600; r1041_e82_400bps_sup_v600 is missing. A user who sets that model would go to the download branch.
Please cover this in the Slurm run, or add a small check (for example a stub-free module test of CLAIR3_MODEL, tagged small).
Collaborator
Author
There was a problem hiding this comment.
I checked these by hand rather than adding a small test. A PR-CI test of CLAIR3_MODEL would bring back the HKU download that this PR takes out of CI, and that download was the flaky step (bio8.cs.hku.hk timeouts). The extended clair_only test still covers the download branch on push to dev.
- Bundled list:
clair3_bundled(21 names) matchesmodels/indepot.galaxyproject.org/singularity/clair3:2.0.3--py311hbc58adc_0exactly. It also matches the GPU image:Dockerfile.gpuat v2.0.2 copies all of HKU'sclair3_models_pytorch/(18 dirs) plus Reriosup_v520,hac_v520andhac_v600into/opt/models, which gives the same 21. - Download URLs:
r1041_e82_400bps_sup_v430,r1041_e82_400bps_sup_v420andr1041_e82_260bps_sup_v400all return 200 forclair3_models_rerio_pytorch/<model>/{pileup,full_alignment}.pt, and 404 underclair3_models_pytorch/, which is what the^r(9|10)branch expects. - SUP v6.0:
r1041_e82_400bps_sup_v600doesn't exist in either HKU collection or in either image, so the list is right and the PR description was wrong. I've corrected it to HAC v6.0 only.
|
Automated review: 4 findings posted inline.
A human reviewer needs to check these by running the tests (the automated review cannot run nf-test or the pipeline):
|
- DEEPSOMATIC_MAKEEXAMPLES passes --nocall_small_model_examples. The 1.10 checkpoints do not enable it today (their JSONs set trained_small_model_path but not call_small_model_examples), but the module does not collect small-model calls, so a future checkpoint enabling it would drop them. Command-line flags override model.example_info.json. - CLAIRS keeps the singularity/docker ternary REVIEW.md asks for. 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
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.
Part 2 of the module updates, stacked on #212 (the base is
chore/module-updates; GitHub retargets todevonce #212 merges). This updates the four small-variant callers. Calls change, so this needs a real-data check before merging; see below.hkubal/clair3:v2.0.2_gpu, the newest GPU tag)What had to change beyond the image tags
Clair3 models. Clair3 v2 runs on PyTorch and cannot read the v1 TensorFlow models the pipeline downloaded from ONT's CDN, so the old WGET→UNTAR path would fail. Now:
hifi_revio, and ONT's R10.4.1 HAC v4.1/v5.0/v5.2/v6.0 and SUP v4.1/v5.0/v5.2 models (plus the_with_mvvariants). There is no SUP v6.0 Clair3 model at HKU or in either image. The list inprepare_reference_files.nfmatches the biocontainer'smodels/and the GPU image's/opt/modelsexactly. CLAIR3 gets an empty model path and points--model_pathat the image'smodels/dir.sup_v430,sup_v420,260bps_sup_v400, or a user-supplied name) is fetched as a PyTorch directory (pileup.pt,full_alignment.pt) from HKU by the newCLAIR3_MODELstep.r9/r10names come from the converted Rerio catalogue.untarmodule is removed. This also takes the HKU wget out of CI for the test data, which was a known flaky step.DeepVariant/DeepSomatic 1.10. Each checkpoint now carries its make_examples flags in
model.example_info.json. The hard-coded--channel_list/--pileup_image_widthno longer matched: the DV ONT model is 199 wide with the new fuzzy channel, and the paired DS PacBio model is 147 wide with 9 channels. make_examples now gets the same--checkpointas call_variants and reads the flags from the model. I checked on the chr19 test BAMs that the example shapes match each model. DeepVariant's JSON enables the small model, which the nf-core subworkflow passes to postprocess. The DeepSomatic JSONs don't enable it (the paired ones settrained_small_model_pathbut notcall_small_model_examples), and make_examples_somatic now gets--nocall_small_model_examplesexplicitly, since the local module doesn't pass small-model calls on.bcftools norm (from #212). The C8 real-data run found that bcftools 1.24 aborts
BCFTOOLS_NORMwhen Clair3 writesNwhere GRCh38 has an IUPAC code (Bat chr3:16902883); 1.22 accepted it. Fixed on #212 with--check-ref w, which gives the same records as 1.22, and merged in here.DeepVariant patches. All four patches were redone on the 1.10 files. The old postprocess also carried two edits its
.diffdidn't record: the${meta.id}_germlinedefault prefix and theprocess_shortlabel. Without the prefix, the tumour-only germline VCF collided with PHASING_HAPLOTYPING's concat output name. I rebuilt every patched module as upstream@sha + .diff to check; DeepVariant was the only one that differed.Testing
-profile debug,test,apptaineron Mindwell. Snapshot drift: caller versions,CLAIR3_MODELreplacing WGET/UNTAR (clair_only downloads its non-bundled model, so both model paths are exercised), and the WhatsHap phasing stats that follow the new germline calls.prek run --all-filespasses.-resume, so only the callers and downstream steps differ. The comparison script isprojects/LRSOMATIC_MODUPD_C8/compare_callers.sh(per-caller PASS SNV/indel counts and A/B overlap). I'll add the numbers here.Follow-ups (not in this PR)
.examples. Enabling it (12–40% faster per the 1.10 release notes) needs the glob fix and dropping--nocall_small_model_examples.--genderfor sex-chromosome ploidy, and the pipeline knowsmeta.sex.PR checklist
docs/usage.mdis updated (clair3_model).CHANGELOG.mdis updated.🤖 Generated with Claude Code