v2.0.0: pyrodigal-gv gene caller, ProstT5+Foldseek structural phage detection, open-source MFD solver, direct CoverM, Snakemake 8+ - #68
Merged
Vini2 merged 46 commits intoAug 12, 2026
Conversation
FragGeneScan is split out of scan_smg into its own rule (call_genes) so the
caller is swappable without touching the marker-gene HMM search downstream.
pyrodigal-gv proteins are emitted with FragGeneScan-compatible headers
({unitig}_{start}_{end}_{strand}); get_smg_unitigs recovers the unitig name by
stripping the last three underscore fields, so any other naming scheme
silently empties the marker-gene set and disables bacterial filtering.
Ablated on two virome assemblies (one HPC-assembled, 259 Mbp) against a
bacterial marker-gene HMM search: pyrodigal-gv finds a net 6 more SMG-bearing
unitigs than FragGeneScan across both graphs, with 1 loss on a very short
(700 bp) unitig where FragGeneScan calls a short trailing ORF that
pyrodigal-gv's gene model does not propose. Short unitigs are unlikely to be
flagged as real phage candidates regardless, so this is an acceptable
trade-off. Details in notes/phables_audit.md (project-level, not in this repo).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
FD_Inexact.py now encodes the minimum flow decomposition through flowpaths.AbstractPathModelDAG instead of raw gurobipy calls. flowpaths defaults to HiGHS (via highspy) and isn't overridden here, so this is a genuine open-solver swap, not a Gurobi dependency in disguise. Two fixes found while validating the swap end-to-end against the bundled test fixture: - SolveInstances stopped creating results_MFD.txt / results_MFD_details.txt (always-empty files even before this change, but declared as required outputs by test_phables.smk -- a missing declared output is a hard Snakemake failure). Restored the two-line creation. - phables.yaml still pinned gurobi>=10.0.0 and the now-unused more-itertools; the switch to stdlib itertools.pairwise needs Python 3.10+, which the existing <3.11 upper bound didn't guarantee. Dropped gurobi, tightened the Python pin, added flowpaths via pip (it isn't on any conda channel -- checked conda-forge, bioconda, and anaconda.org directly). build/environment.yml is untouched: it never depended on Gurobi, since it only carries the CLI/Snakemake shell, not the solver. Validated by running phables.py's real pipeline against test_data/ directly (bypassing Snakemake, since the PHROGs MMseqs DB isn't available locally): 6/6 components resolved, 7 genomes, sensible lengths/GC/coverage, identical before and after the two fixes above. Not yet validated against a live Gurobi run -- no license survives the fresh clone of this repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… --phagedetection Adds prostt5-foldseek as an alternative to the mmseqs/PHROGs phage-gene detection path: pyrodigal-gv-called proteins -> ProstT5 3Di prediction (pholdlib) -> foldseek search against a hallmark PHROG structure subDB (head and packaging/connector/tail/lysis) -> phables_utils/hallmark_utils.py. hallmark_utils parses foldseek's own convertalis output directly rather than mimicking mmseqs' TSV format -- its own file format, its own parser, matching gene_utils.get_phrog_unitigs' exact (unitig_phrogs, phrog_dict) contract so component_utils.get_components needs no changes. It also asserts an invariant get_phrog_unitigs never had to: every phrog id in unitig_phrogs must have a category in the passed-in categories file, raising a clear error naming the drifted ids if a stale categories file is ever paired with a rebuilt DB, instead of a bare KeyError deep inside get_components. New Snakemake rules (predict_3di, build_hallmark_query_db, scan_hallmark) mirror the call_genes pattern, gated behind `if PD == "prostt5-foldseek"` in genes.smk. That gating is load-bearing, not just tidiness: scan_hallmark's hallmark_db input defaults to None in config.yaml when unused, and Snakemake parses every rule's input/output at load time regardless of the target list, so an un-gated reference breaks a plain mmseqs-path `phables run` with a SyntaxError even though the rule would never execute. build_foldseek_query_db.py encodes two non-obvious foldseek mechanics found while validating this against real data: `foldseek createdb` doesn't accept plain protein FASTA (use `tsv2db` on numbered AA/3Di/header TSVs instead, matching phold's own create_foldseek_db.py), and `foldseek search` between two coordinate-free DBs fails unless the auto-created empty _ca companion file is deleted first. Validated end-to-end through the real wired pipeline (not standalone scripts): 126 called proteins -> 126 3Di predictions -> 97 hallmark hits on the bundled test graph -> 4 resolved genomes. Cross-checked against the mmseqs path on the same graph: the 4 resolved genomes are byte-for-byte identical to 4 of the mmseqs path's 7 (same coverage/length/GC%/node order), confirming the integration is correct where it fires -- but 3 mmseqs-found components are missed under the current untuned --hallmark-evalue/--hallmark- minbits defaults, a real precision/recall calibration question rather than a bug. Full analysis in notes/phables_audit.md (project-level, not in this repo). Neither the hallmark DB nor the ProstT5 checkpoint are auto-downloaded yet -- both must be supplied explicitly via --hallmark-db/--hallmark-categories/ --prostt5-checkpoint. No hosted distribution point for the custom hallmark DB exists yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lax Python pin
Splits the single prostt5.yaml env into prostt5-{cpu,cuda,rocm}.yaml,
selected via --gpu-backend / config gpu_backend, since conda envs are
solved once from a static file and can't switch PyTorch builds at
runtime. cpu/cuda use a conda build-string wildcard (pytorch=*=cpu*,
pytorch=*=cuda*) rather than a pinned CUDA minor version, so the solver
picks whatever tagged build matches the rest of the env instead of this
needing to be bumped by hand as new PyTorch releases land -- confirmed
these wildcards resolve on both the pytorch and conda-forge channels.
rocm has no conda build at all (checked both channels), so it stays
pip-based against download.pytorch.org's rocm6.3 wheel index.
Wanted --no-deps on the pholdlib pip install to protect the conda-solved
torch build, but that isn't achievable through a conda/mamba pip:
section at all -- both write it to a real requirements.txt, and current
pip rejects --no-deps as a file directive (confirmed empirically under
both conda and mamba, not just from reading docs). Checked what actually
happens without it instead: pholdlib's own torch requirement is
unconstrained, so a real install against the conda-built env left torch
untouched; it reinstalled a couple of pholdlib's smaller transitive deps
to match its own metadata, which is harmless.
Adds --foldseek-gpu, mirroring phold's own GPU search code
(phold/features/run_foldseek.py) rather than the foldseek README alone:
the {target_db}_gpu naming convention and --gpu 1 --prefilter-mode 1
flags come from phold's actual working implementation, which is a
better bet than the README's suggestion that one padded DB works for
both CPU and GPU. Raises a clear error if foldseek_gpu is requested with
gpu_backend != cuda, since foldseek's GPU mode is CUDA-only.
Adds --prostt5-container for running predict_3di against a container
that already has pholdlib + torch installed (e.g. phold's own image)
instead of building a conda env. Deliberately does NOT combine
container: with conda: on this rule -- Snakemake's documented way to do
that builds a separate, isolated conda env inside the container rather
than exposing what's already installed there, which would just
reinstall a second redundant torch and ignore the image's own
verified-working one. Needs --use-singularity passed through phables'
existing snake_args passthrough.
None of the GPU/container code paths are verified against real hardware
-- this machine has neither CUDA, ROCm, nor Apptainer/Singularity. All
wiring is dry-run tested: env/container selection resolves correctly
for every --gpu-backend value and with/without --prostt5-container, and
invalid combinations (e.g. --foldseek-gpu with gpu-backend rocm) raise
a clear error at parse time instead of failing silently later.
Also relaxes phables.yaml's python pin from <3.11 to <3.13. Traced the
history: the upper bound was added to fix a solver conflict attributed
to pysam, but `mamba search -c gurobi gurobi>=10.0.0` shows that package
only ever published py310 builds -- almost certainly the real cause,
now moot since Gurobi is gone. Confirmed the remaining (post-Gurobi)
dependency set solves cleanly through 3.12; 3.13 has unrelated solver
friction, likely numpy<2.0.0 build lag.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both traced back to Gurobi/numpy-2.0-transition issues that no longer apply now Gurobi is gone and numpy 2 has been out for well over a year: - numpy<2.0.0 was added for GH issue Vini2#55 (June 2024), right when numpy 2.0 shipped and broke pandas' ABI compatibility across the ecosystem. That was real at the time. Verified today's stack doesn't have the problem: built a real env with numpy 2.5.1 + pandas 3.0.5 and tested phables' actual pandas usage (combine_cov.py, format_koverage_results.py -- DataFrame construction, axis=1/join="inner" concat, concat-as-append, read_csv/to_csv) against it directly, not just that the imports succeed. - The previous commit's <3.13 upper bound was itself downstream of the numpy<2.0.0 pin colliding with 3.13's package availability, not a real 3.13 problem -- confirmed by dry-run solving 3.10 through 3.13 cleanly once numpy was unpinned. Ran the full real pipeline (component detection, HiGHS solve via flowpaths, output writing) against a from-scratch env with numpy 2.5.1, pandas 3.0.5, python 3.12: byte-identical resolved_genome_info.txt versus the previous numpy<2/python3.10 run. Gene-caller test suite also passes unchanged (8/8) against the new env. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bumps snakemake>=7.14.0 -> snakemake>=8.0 (setup.py, build/environment.yml)
and the accompanying python_requires from >=3.9,<3.11 to >=3.11 -- every
8.x/9.x release on PyPI requires Python 3.11+, confirmed by attempting to
install snakemake>=8 into a 3.10 env, which refused to resolve any 8.x/9.x
version at all.
Why: Snakemake 8.0.0 introduced the external executor-plugin architecture
(snakemake-executor-plugin-slurm etc) and the set-resources/executor: keys
in workflow profiles. 7.x's older built-in --cluster/--drmaa flags don't
auto-translate rule resources like `gpu=1` into SLURM flags (`--gpus=1`)
at all -- that mapping is new plugin-side machinery. This is what lets
predict_3di/scan_hallmark's GPU work be routed to a GPU partition via an
optional profile, while every other rule stays on a cluster's default
partition, without hardcoding SLURM specifics into the Snakefile itself.
Bumping surfaced a real, fork-wide incompatibility, independent of the new
profile mechanism: every rule with a resources: block set both `mem_mb` and
`mem` (a string with a unit) for the same underlying memory requirement.
Snakemake 7 allowed this; Snakemake 9 rejects it outright ('The following
resources are equivalent and cannot be simultaneously provided to a rule').
Found across coverage.smk, genes.smk, and postprocess.smk (4 rules) by
actually attempting a dry-run under Snakemake 9, not by inspection. Fixed by
dropping the redundant `mem` string and keeping only `mem_mb` -- the
canonical resource key the SLURM executor plugin's own docs map to `--mem`.
Verified: dry-run under Snakemake 9.24.0 (the version `snakemake>=8.0`
resolves to today) succeeds past the point where it previously failed with
the resource conflict. Confirmed no Snakemake-8-removed directives
(dynamic(), version:, subworkflow, remote()) are used anywhere in this
workflow, so this migration doesn't lose any functionality on that front.
Not yet tested against a real SLURM cluster -- only Snakemake's own
--dry-run parsing and job-graph construction, no hardware submission.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
chore/snakemake8-migration fixed this in the pre-existing rules; the new call_genes/predict_3di/scan_hallmark rules added on this branch had the same copy-pasted pattern and needed the identical fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ents phables' own mmseqs-based phage detection (gene_utils.get_phrog_unitigs) reads product/category info from a bundled phrog_annot.tsv shipped in the repo. Compared it against two external sources: pharokka's phrog_annot_v4.tsv and phold's phold_annots_updated.tsv (the more recently curated one, per Vijini). The bundled file matched phrog_annot_v4.tsv almost exactly (9 differences out of 109 404 PHROGs) and differed from the updated one on 5 867 -- meaning phables has been shipping the outdated categorisation. The pattern in what changes is systematic, not noise: PHROGs the outdated file calls "unknown function" get correctly assigned to real hallmark categories in the updated file, with product descriptions that plainly justify it -- e.g. phrog_19684 "head-tail connector protein" unknown -> connector, phrog_72652 "tail tube" unknown -> tail, phrog_45215 "virion structural protein" unknown -> head and packaging. Every one of these was previously invisible to component_utils.get_components' category test, meaning components whose only PHROG evidence fell on one of these ~5 800 IDs were being excluded as if they had no phage-like content at all. Fixed by replacing the annot/category columns from the updated source while preserving the bundled file's own color column unchanged -- confirmed get_phrog_unitigs never reads that column (only strings[2] and strings[3]), so it's a pure display/passthrough field, safe to leave as-is rather than needing a color scheme for the newly-corrected categories. Verified: re-ran the real pipeline against phables' own bundled test_data/ -- component count unchanged (6), since none of that fixture's PHROG ids happen to be among the ~5 800 that changed. Spot-checked three of the corrected PHROGs directly in the new file to confirm the category values, not just the diff count. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…5-foldseek-detection
Only predict_3di (and scan_hallmark, when --foldseek-gpu is set) benefit from a GPU -- every other rule in this workflow is CPU-only. Deliberately not hardcoded into the Snakefile: none of the rules declare slurm_partition or a gpu resource directly, so a user without GPU access (or on a cluster with no GPU partition) gets an entirely CPU-routed run by default, with nothing to opt out of. This profile is the opt-in path for users who do want GPU routing: pass it via phables' existing snake_args passthrough (--workflow-profile phables/workflow/profiles/gpu-routing), and only predict_3di/scan_hallmark get routed to a GPU partition; everything else still defers to the cluster's own default profile/sbatch defaults. Needs the snakemake>=8 migration (separate commit) -- the set-resources/ executor: mechanism this profile uses (and the auto-translation of resources: gpu=N to SLURM's --gpus=N) is new in Snakemake 8's external executor-plugin architecture; Snakemake 7's older --cluster/--drmaa flags don't support it. Verified with --dry-run under Snakemake 9.24.0 (snakemake-executor-plugin- slurm installed): the profile is recognised and predict_3di/scan_hallmark resolve correctly with it; the same run also verified successfully without --workflow-profile at all (--phagedetection prostt5-foldseek with no GPU routing) and with the plain mmseqs default path, confirming GPU routing is genuinely opt-in rather than forced on anyone. Not tested against a real SLURM cluster -- no such access here, and the exact partition name/gres syntax needs matching to whatever cluster this is actually used on (noted in the profile's own comments). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1. `phables run`/`install`/`test` crashed before Snakemake ever started
whenever --output pointed at a directory that didn't already exist:
snaketool_utils' copy_config() opens f"{output}/phables.log" to write
its first log line, and that open doesn't create missing parent
directories. Fixed with a callback on --output itself (in
common_options, so it covers run/install/test uniformly) that creates
the directory as soon as the option's value is resolved -- before
--configfile/--log run their own default_to_output callbacks, since
--output is processed first.
2. --databases was config-file-only despite looking like a normal CLI
option; passing --databases directly failed with `snakemake: error:
unrecognized arguments: --databases` because nothing consumed it before
it fell through to Snakemake's own arg parser. Added a real --databases
click option in common_options; it flows into config the same way
--genecaller/--phagedetection etc already do, via merge_config=kwargs.
3. run_combine_cov used `sed -i '1d' {input}` to strip the coverage.tsv
header before summing, mutating its own declared input file. Not
idempotent: a retry after a partial failure would see an
already-stripped input and silently drop the first real data row as if
it were still the header, producing a wrong-but-plausible coverage.tsv
instead of erroring. Replaced with `awk 'NR>1 {...}'`, which skips the
header without touching the input at all -- verified identical output
to the old two-step version on representative CoverM-shaped data, and
confirmed it can be re-run any number of times against the same
untouched input with the same correct result.
Verified 1+2 together with a real dry-run: `--output <new-dir> --databases
<path> --dry-run` succeeds and the output directory exists afterward,
without a pre-existing mkdir and without needing --configfile to smuggle
--databases in.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
component_utils.get_components tested category membership with substring
matching against phrog_dict[phrog], which held the PHROG's product/annot
text concatenated with its category (e.g. "tail completion or Neck1
protein connector"). `"tail" in phrog_dict[phrog]` fires on that string
whether or not the PHROG's real category is tail -- and plenty of
connector-category genes describe the tail apparatus they connect to in
their product name.
Confirmed against the real, current annotation table (post the
phold_annots_updated.tsv fix): 110 PHROGs hit this exact failure mode --
103 false "tail" matches, 7 false "connector" matches -- almost all
connector-category genes ("tail completion", "tail terminator",
"head-tail adaptor", "baseplate wedge tail fiber protein connector", ...)
whose product text happens to mention another hallmark word.
The practical impact is narrow but real: since get_components only needs
*any* hallmark category present to keep a component (an OR across all
four), a false match rarely changes whether a component survives at all.
It does change `likely_complete`, which requires *all four* categories
(an AND) -- a component with real head/connector/lysis evidence plus a
connector-category gene whose product text happens to say "tail" would
be wrongly marked likely_complete=1 with no real tail gene anywhere on
it. Confirmed this concretely: reproduced the old string-concatenation
format standalone and showed it computes likely_complete=1 for exactly
this case (phrog_2 + phrog_5 + phrog_7), where the fixed code correctly
computes 0.
Fixed at the source rather than patching the substring tests: gene_utils.
get_phrog_unitigs now stores just the category in phrog_dict, not the
product text concatenated with it -- confirmed phrog_dict has no other
consumer anywhere in the codebase, so nothing downstream depended on the
product text being there. component_utils.get_components then does exact
equality instead of substring containment.
Verified with a test that empirically discriminates old vs new behaviour
(not just asserts the new one): the phrog_2+phrog_5+phrog_7 case above,
proven to compute likely_complete=1 under the old string format and 0
under the fix. Also re-ran the real pipeline against phables' own bundled
test_data/ -- component count unchanged (6), no regression.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Gene caller, HiGHS solver, ProstT5+Foldseek detection, Snakemake 8 migration, and bug fixes
install() passed **kwargs to run_snakemake but not merge_config=kwargs, so --databases never reached install.smk's config -- it silently fell back to <repo_root>/databases regardless of what was passed on the CLI. run() already did this correctly; install() just needed the same treatment.
…us chain cogent3>=2026.7.6a0 depends on scinexus>=2026.5.16, which only resolves to a tqdm build requiring python-3.14.0rc1 (needs the _python_rc virtual package) -- unsatisfiable on any real install. No bioconda cogent3 build through 2026.4.13a0 has a scinexus dependency at all.
At metagenome scale (thousands of genomes) the postprocess phylogenetic tree step is slow and most workflows want to run phylogenetics as a separate step anyway (per Vijini). Adds --build-tree/--no-build-tree (default off); genomes_aligned.fasta and genomes_phylogenetic_tree.tree are only added to postprocessTargets when set. Verified via dry-run: default run omits build_msa/build_tree from the job DAG entirely, --build-tree restores both.
…5-foldseek-detection
…: entrypoint The CLI entrypoint's --model-dir defaults to ~/.cache/prostt5 via argparse, but the script: entrypoint passes config['prostt5_model_dir'] straight through, which is None unless --prostt5-model-dir was explicitly set. hf_cache_present() then crashed building Path(None). Fallback now lives in run() itself so both entrypoints get it.
transformers' check_torch_load_is_safe() (CVE-2025-32434 hardening) refuses torch.load()-based checkpoint loading below torch 2.6, unconditionally -- this broke loading Rostlab/ProstT5_fp16 (no safetensors file) the moment the unpinned pytorch=*=cuda*/cpu* wildcard let the solver pick something older. Verified: dry-run solve of prostt5-cpu.yaml resolves pytorch 2.13.0 cleanly under the new pytorch[version=">=2.6",build=cpu*] spec; rocm6.3's wheel index has torch 2.7.0-2.9.1 available for the pip-installed rocm variant.
Reduces spurious low-confidence hallmark hits at the default threshold.
…g components component_utils.get_components already required single-unitig candidates to clear minlength before ever being resolved, but multi-unitig LINEAR paths had no length floor at all -- any component with a single phage-hallmark hit produced a "resolved genome" regardless of how short the MFD-resolved path came out. Pilot10 output showed this concretely: 72% of prostt5-foldseek's 227 resolved paths and 71% of mmseqs's 211 were under 5kb, well below any plausible phage genome size. Filters final_genomic_paths in short_utils.py/long_utils.py right before it feeds both all_resolved_paths (-> resolved_genome_info.txt) and write_path/write_path_fasta (-> resolved_paths.fasta, resolved_phages/*.fasta) -- a single insertion point so all three outputs stay consistent, rather than filtering only the returned list in phables.py after the FASTA files are already written. Circular paths are exempt: a closed cycle is completeness evidence independent of length, same reasoning the existing single-unitig gate already used. Verified: standalone filter-logic test against short/long/circular/linear GenomePath cases (boundary case at exactly minlength correctly dropped, matching the existing strict '>' convention). Real `phables test` run against the bundled fixture succeeds with all 7 genomes preserved (none happened to be sub-threshold linear paths in that tiny graph) -- confirms no regression, not the drop case itself. Also fixes test_phables.smk, found while validating this: its params block never got phagedetection/hallmark_* added when that feature landed, so `phables test` crashed unconditionally (AttributeError on snakemake.params.phagedetection) regardless of this change.
…nly) igraph 1.0.0 renamed get_all_simple_paths' path-length-limit kwarg from `cutoff` to `maxlen`, so the long-read branch crashed with "TypeError: got an unexpected keyword argument 'cutoff'" the moment it ran (get_all_sub_paths is only reached under --longreads, which is why the short-read path never surfaced this). Detects the kwarg name from the installed igraph's signature rather than pinning igraph, so this works either side of that release. Verified against igraph 1.0.0 that both spellings mean the same thing -- they count *edges*, so a value of 2 yields paths of up to 3 vertices, matching what the len(path)==3 / len(path)==2 branches below expect.
Hit on Setonix: predict_3di crashed on `from pholdlib.prostt5.device import parse_gpus` with `OSError: libomp.so: cannot open shared object file`. Root cause: importing pholdlib.prostt5 runs pholdlib/prostt5/model.py's `from transformers import T5EncoderModel, ...`, and recent transformers routes all model imports through a shared loss registry (transformers/loss/loss_utils.py) that unconditionally imports transformers/loss/loss_rnnt.py for ParakeetForRNNTLoss -- an audio ASR loss totally unrelated to T5/protein embeddings. That file does a plain `import torchaudio`, and torchaudio's compiled extension needs libomp.so (LLVM's OpenMP runtime), which this container's image doesn't have linkable. We never touch anything audio-related, so if the real torchaudio import fails with OSError, install a harmless stub in sys.modules first so transformers' import chain completes anyway. Verified the mechanism directly: reproduced the same OSError with a minimal package shaped like transformers' actual import chain (unconditional `import torchaudio` + a class referencing it only in a method body, never at class-definition time), confirmed the stub resolves it cleanly. Only engages when the real import already failed -- no-op everywhere torchaudio works normally.
The previous fix (afe38c5) unblocked the original OSError but introduced a new one: before transformers ever reaches the `import torchaudio` line that actually needed a stub, it first runs a lightweight availability check (is_torchaudio_available() -> importlib.util.find_spec("torchaudio")), which explicitly raises `ValueError: torchaudio.__spec__ is None` when sys.modules already has an entry for the name with no spec -- exactly what a bare types.ModuleType has by default. Hit this for real on Setonix. Fix: give the stub an actual (empty) importlib.machinery.ModuleSpec, plus a __version__ attribute as a defensive fallback (transformers falls back to reading torchaudio.__version__ directly if the installed-distribution metadata lookup ever fails). Verified all three points in the real chain locally: find_spec succeeds, the __version__ fallback path succeeds, and a deep unconditional `import torchaudio` + class referencing it only in a method body (matching loss_rnnt.py's actual shape) succeeds.
…karound Switching to the conda-env ROCm path (--gpu-backend rocm, no --prostt5-container) for Setonix after repeatedly hitting container-specific problems unrelated to phables/pholdlib itself: broken torchaudio needing libomp.so, --home overriding the container's apparent CWD so relative output paths silently resolved to the wrong place, and a missing MIOpen AI-heuristic kernel file for gfx90a. A plain conda env has no container namespace/bind-mount surface to hit any of these on. Pin torch==2.7.1 exactly (not just >=2.6) to match Setonix's own already-verified-working Pawsey container tag (quay.io/pawsey/pytorch:2.7.1-rocm6.3.3) -- confirmed the exact wheel exists on the rocm6.3 index for cp39-cp313. Also broadened predict_3di.py's torchaudio workaround from `except OSError` to `except (OSError, ImportError)`: a plain conda env installing only torch + pholdlib won't have torchaudio installed at all (unlike a generic "pytorch" container image, which tends to bundle the full audio/vision extras), so the same unconditional import inside transformers' loss registry would raise ModuleNotFoundError instead of OSError -- previously uncaught, meaning our own diagnostic `import torchaudio` would have crashed predict_3di.py unnecessarily in exactly this env. Verified both exception paths locally.
Per Vijini's recommendation: drop the koverage Snakemake-wrapper dependency
for the preprocess unitig-coverage step and call minimap2 + CoverM directly.
Reproduces Koverage's own real "coverm" mode chain exactly (confirmed against
its actual source, github.com/beardymcjohnface/Koverage workflow/rules/coverm.smk,
and its shipped default config.yaml -- not reimplemented from guesswork):
- same minimap2 mapping command (-ax sr --secondary=no) + samtools
sort/view -F 4/index chain
- same `coverm contig` --methods list/order: count, rpkm, tpm, mean,
covered_fraction, variance (phables never overrode Koverage's own default)
- same per-sample -> long-format Sample/Contig/<method...> TSV reshape
sample_coverm_coverage.tsv keeps the same shape, so run_combine_cov's awk and
coverage_utils.py's BAM globbing need no changes. Verified the reshape +
downstream Mean-summed-per-contig reduction end-to-end against synthetic
CoverM-style output.
Scope: only the preprocess step (feeds MFD via coverage.tsv). postprocess.smk's
koverage_genomes rule uses Koverage's separate native (non-CoverM) engine and
is untouched -- a different question, not covered by PLAN.md §4.8/the audit.
No CRAM/--reference wiring yet (§3.2) -- no pipeline produces CRAM reads yet,
so there's nothing to test that against.
Preserved bug-for-bug from Koverage's own rule, not newly introduced: no -t
passed to `coverm contig`, and -ax sr used even for --longreads runs. Both
flagged separately for follow-up.
Not yet dry-run against a real Snakemake install (none available in this
environment) -- run `phables run ... --conda-create-envs-only` and the
koverage-vs-CoverM-direct coverage.tsv regression check PLAN.md §4.8 calls
for before trusting this on real data.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
format_koverage_results.py's parsing (strings[4]=RPKM, strings[7]=Mean) was
suspected of assuming the wrong Koverage output columns, since Koverage's
`coverm` subcommand mode uses a different, shorter header. Traced
postprocess.smk's koverage_genomes rule (`koverage run --reads ... --ref
...`, no `coverm` subcommand) to Koverage's native "map" mode default target,
and confirmed its real sample_coverage.tsv header against upstream source
(coverage.smk's all_sample_coverage rule + sampleCoverage.py):
Sample Contig Count RPM RPKM RPK TPM Mean Median Hitrate Variance
0 1 2 3 4 5 6 7 8 9 10
Under that header the existing indices (2/4/7 = Count/RPKM/Mean) were already
correct -- no data bug. Documented both header layouts and why "map" mode
applies here, factored the row parsing into parse_koverage_row() with named
index constants, and added a standalone regression test (no pytest
dependency, since this repo has none) that locks the mapping in against a
synthetic row copied from Koverage's own format.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
coverm_map always mapped with -ax sr regardless of config["longreads"], matching Koverage's own coverm_map_pe upstream behavior exactly but now baked into phables' own rule file rather than hidden in a wrapper. Long- read (ONT/PacBio) runs via myloasm were thus having their unitig coverage computed with a short-read-tuned minimap2 preset feeding the MFD flow decomposition -- exactly the "wrong-but-plausible coverage numbers produce wrong-but-plausible genomes" risk PLAN.md's risk table calls out. LR (config["longreads"], set in 02_phables_preflight.smk, already included before coverage.smk) now selects the preset: -ax sr unchanged for short reads, -ax map-ont for long reads. --longreads is a bare boolean flag with no ONT/PacBio distinction anywhere in phables, and map-ont is confirmed correct for the only long-read data actually exercising this path so far (../metagenomic_phage_discovery/pilot10_longreads: all Oxford Nanopore, assembled with myloasm's default R10.4-targeting mode). Verified both branches with a Snakemake dry-run (phables-dev env, snakemake 7.32.4): confirmed the emitted minimap2 command switches -ax sr <-> -ax map-ont correctly with longreads False/True. Full run_pilot_phables.sh before/after regression on real long-read data still needed on Phoenix (GPU node, real databases) -- not run here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Real error on Setonix: LibMambaUnsatisfiableError, every samtools 1.14-1.24 rejected on a libdeflate lattice conflict. Root cause: coverm's own bioconda recipe already declares minimap2>=2.28 and samtools>=1.9 as its own dependencies (plus a large pile of other bundled tools -- bwa, fastani, gsl, skani, strobealign, etc.). Explicitly pinning a second, independent minimap2>=2.24/samtools>=1.15 on top of that forced mamba to jointly solve two separate constraint surfaces across coverm's already tightly-pinned dependency graph, rather than the one bioconda actually built/tested coverm against. Fix: list only coverm -- it pulls in a mutually compatible minimap2 + samtools on its own, which is everything the coverm_map rule's shell commands need. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Real error on Setonix: samtools index rejected the "ERR1301161.bam" output with "is in a format that cannot be usefully indexed" -- the file was actually SAM text, not BAM. samtools view's documented default output format is SAM regardless of input format or `>` redirection target name; -b/--bam must be passed explicitly (confirmed against htslib's own samtools-view docs). The prior version of this rule lacked -b -- likely dropped during an earlier LLM-summarized "verbatim" fetch of Koverage's upstream source used to write this rule, not present in a byte-exact read of the real file. Verified this time against samtools' documented flag semantics directly rather than a fetched transcription of someone else's shell command. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…overage Completes PLAN.md §4.8. The earlier preprocess-only change (a1ac8fd) swapped the MFD-critical coverage step to CoverM but left koverage installed and still running a nested Snakemake -- so §4.8's actual headline benefit ("removes a nested Snakemake dependency from inside what is already a Snakemake workflow") was not achieved. It is now. Of the three koverage-prefixed rules in the DAG, exactly ONE actually depended on koverage: - koverage_tsv -> plain metasnek fastq_finder call, no koverage. Renamed samples_tsv (localrules updated to match). - koverage_genomes -> the real dependency (conda: koverage.yaml). Replaced with coverm_map_genomes / coverm_bam2counts_genomes / coverm_combine_genomes, mirroring coverage.smk. - koverage_postprocess-> runs phables.yaml, no koverage. Renamed format_genome_coverage. koverage.yaml deleted; no live koverage reference remains in the workflow. ESTIMATOR CHANGE (unlike the preprocess swap, which was shape-preserving): koverage's native "map" mode emitted Sample Contig Count RPM RPKM RPK TPM Mean Median Hitrate Variance `coverm contig` emits Sample Contig Count RPKM TPM Mean Covered_fraction Variance All three values phables consumes (Count/RPKM/Mean) still exist, and format_koverage_results.py's indices move 2/4/7 -> 2/3/5 accordingly. But the two tools compute them via different code paths, so reported numbers may differ slightly. This is REPORT-ONLY (sample_genome_*.tsv are not read by anything downstream; the MFD math uses preprocess/coverage.tsv). Not numerically regression-tested against old output on real data -- worth doing per §4.8's own advice before relying on absolute values. Also: postprocess BAMs are marked temp(). koverage's native mode piped minimap2 into a python counter and never wrote a postprocess BAM, so keeping them would be a pure disk regression at 10k-sample scale; nothing consumes them after the counts step. Verified: regression test updated to lock in the CoverM indices (and to assert the legacy 4/7 indices are NOT in use -- confirmed it genuinely fails if they are reintroduced, catching RPKM silently reporting TPM); the three new rules structurally diffed against their coverage.smk counterparts and confirmed identical modulo ref/paths/names. NOT dry-run tested -- no snakemake available in this environment; run --conda-create-envs-only / a dry-run before trusting on real data. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Warrants a major bump: new gene caller (pyrodigal-gv), new phage-detection method (ProstT5+foldseek structural search), open-solver MFD swap (Gurobi -> flowpaths/HiGHS), Snakemake 8+ migration, and the koverage removal -- a substantial feature/behavior set, not a patch release. Updated phables.VERSION (the single real source, read by both setup.py and __main__.py's --version) plus six hardcoded "1.5.0" strings that don't read from it (module __version__ attributes and logger names) -- found via a plain grep for the old string, not guessed at from memory. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two real, confirmed-in-a-clean-env gaps found auditing the test suite before
the PR, neither caught before now because this Mac happens to have pandas/
biopython/pyrodigal-gv installed globally, masking both locally.
1. CI's python-version matrix was still ["3.9", "3.10"], despite setup.py's
own python_requires=">=3.11" (needed by snakemake>=8, per that migration's
own commit). Every CI run on this branch would fail `pip install .`
outright, on every combination. Bumped to ["3.11", "3.12"] in both the
`tests` and `finish` jobs.
2. Both real test files that import a workflow script directly (not through a
Snakemake rule, which would have its own per-rule conda env) need packages
build/environment.yml -- all CI installs -- doesn't provide:
- tests/test_gene_caller.py needs pyrodigal_gv (envs/genecall.yaml, a
per-rule env). Fixed by pip-installing it in the CI test step only,
not the base build/environment.yml (keeps end users who never touch
--genecaller pyrodigal-gv from an unneeded install).
- phables/tests/test_format_koverage_results.py needs pandas/biopython
purely because format_koverage_results.py imported them at module
level even though the row-parsing logic the test exercises never
touches either. Fixed properly instead of papering over it: moved
both imports inside main() (the only place they're used) -- runtime
behaviour is unchanged (envs/phables.yaml, the rule's real conda env,
already provides both), but the module is now importable standalone
for unit testing without a full data-science stack.
Verified for real: built an isolated venv (this Mac's own python3 has a
broken/incomplete pygments install unrelated to phables that blocks a plain
`pytest` invocation entirely), ran `pip install .` + the exact CI steps
against it -- all 12 tests pass. Also confirmed the reverse: simulated a
clean env with pandas/Bio import-blocked via a builtins.__import__ hook and
confirmed format_koverage_results.py still imports and its row-parsing
function still works correctly.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
--hallmark-categories' own help text already said "from build_hallmark_db.py" -- that script didn't exist anywhere in the repo. Ports the real script that built the actual reference hallmark_db used to validate --phagedetection prostt5-foldseek this whole session (previously living outside the fork, at ../metagenomic_phage_discovery/db/build_hallmark_db.py), unchanged in logic, into its promised home. Subsets phold's full structure DB down to the four structural/virion PHROG categories phables_utils/component_utils.py's get_components actually screens for (head and packaging, connector, tail, lysis), plus a separate integration/ excision channel kept apart rather than folded in (integrases are abundant on bacterial chromosomes/MGEs too, would dilute the structural signal). Confirmed its output format matches hallmark_utils.load_hallmark_categories()'s own docstring expectation exactly (phrog_id-with-prefix + category, header row) -- not assumed, read both sides. Verified by porting AND re-running against synthetic phold-format fixtures (phrog IDs, non-numeric IDs to confirm exclusion, a lookup file with an over-cap PHROG family to confirm the stride-sample math): correct category counts, correct stride-sampled row indices, correct file format, non-PHROG channels (acr_*, etc.) correctly excluded. docs/hallmark_db.md explains the category choice, the stride-sample rationale, exact usage, and carries the real reference-build numbers (5,389 hallmark PHROGs, 436 integration/excision, 49,869/2,610 structures after capping, 137MB, independently confirmed queryable via a self-search) as a sanity-check anchor for anyone rebuilding it. Registered in mkdocs.yml's nav. Also documents, explicitly, why this stays a manual step rather than something `phables install` fetches -- phold's structure DB is a multi-GB, separately-versioned asset this project shouldn't silently pin/re-host a copy of. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
createsubdb needs its own separate call for _ca (C-alpha coordinates), same as _ss/_h -- confirmed against steineggerlab/foldseek#97, which builds an AFDB subset the identical way (separate createsubdb calls against afdb, afdb_ss, and afdb_ca). Not a side effect of the main "" createsubdb call. Caught because the user's own real reference build (on HPC, against the actual full phold DB) has hallmark_db_ca/integration_excision_db_ca files that this script's suffix loop -- ("", "_ss", "_h") -- would never produce. Without _ca, foldseek's structural search/alignment is missing the actual 3D coordinates, not just AA+3Di sequence -- an incomplete subDB, not merely a smaller one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Changes the design from "every user runs build_hallmark_db.py themselves" to
"phables install fetches a pre-built tarball", matching how marker.hmm/the
PHROGs MMseqs profile DB already work -- per explicit direction: don't make
every installation build this.
- databases.yaml: new hallmark_db_url/_file/_folder keys. Named
hallmark_db_url, NOT hallmark_db -- config.yaml already uses the bare
"hallmark_db" key for the *runtime* --hallmark-db path, and configfile:
entries share one flat config dict, so reusing the name would have silently
clobbered the runtime value with this download URL. Caught before it
shipped by re-reading both files side by side, not by running into it.
- install.smk: new hallmark_db_download rule + target, alongside the existing
three. Extracts into a freshly-created output dir (mkdir -p {output}; tar
-xf ... -C {output}) rather than straight to DBPATH like
phrogs_mmseqs_download does -- that rule's tarball already contains its own
top-level folder; this one's tarball members are expected at the archive
root (see docs/hallmark_db.md's packaging section for the exact `tar -czhf`
invocation this assumes, including why -h/dereference is required for
createsubdb's .lookup/.source symlinks).
- 00_database_preflight.smk: when --hallmark-db/--hallmark-categories are
unset (None, config.yaml's default), resolve them to the DBPATH-relative
location phables install just fetched -- same "just works once installed"
behaviour --databases's own marker.hmm/phrogs_mmseqs_db already have. An
explicit --hallmark-db/--hallmark-categories still overrides this (only a
None gets filled in).
- __main__.py: updated --hallmark-db/--hallmark-categories help text to
describe the new auto-resolve behaviour instead of "(required for
--phagedetection prostt5-foldseek)", which is no longer accurate.
- docs/hallmark_db.md: reframed around "phables install handles this" as the
normal path; build_hallmark_db.py is now documented as the
rebuild/reproducibility script, not a per-user step. Added a "Packaging a
rebuild for redistribution" section with the exact tar command (verified
locally that -h genuinely dereferences symlinks into portable real content,
not just asserted).
hallmark_db_url is a real placeholder ("CHANGE-ME: ...") -- fill in once the
tarball is actually uploaded (e.g. to Zenodo).
Verified end-to-end with real Snakemake 9.25.1 dry-runs (not just read):
`phables install --dry-run` now includes hallmark_db_download as a target
alongside the other three downloads, and `phables run --phagedetection
prostt5-foldseek --dry-run` resolves cleanly with NO --hallmark-db/
--hallmark-categories flags passed at all, correctly finding the
placeholder files at the auto-resolved --databases-relative path. Full
pytest suite (12/12) still passes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Real hallmark_db_url filled in (https://zenodo.org/records/21884331/files/ hallmark_db.tar.gz?download=1), replacing the CHANGE-ME placeholder. This supersedes an intermediate record (21883465) that had a stray nested hallmark_db.tar.gz (~5MB, an earlier/partial tarball swept up by a `tar czf ... .` run from inside a directory that already had one) -- fixed upstream by re-packaging cleanly, not by anything on this side. Also corrected the -h/--dereference packaging guidance from the previous commit: real testing confirmed phables doesn't need .lookup/.source at query time at all (foldseek convertalis resolves target/query via the _h header db, which createsubdb builds properly for the subset -- .lookup/ .source are createsubdb's own build-time input, not consulted downstream). The actual hosted tarball has neither file, dereferencing is unnecessary, and the packaging section now recommends --exclude'ing them outright instead. Verified for real, twice (once per Zenodo record): downloaded the real tarball and inspected its full member listing (confirmed no .lookup/ .source/symlinks, files at archive root matching hallmark_db_download's extraction assumption); ran actual `phables install --databases <dir>` end-to-end against the live URL (not a dry-run -- a genuine download + extract, "Databases are successfully setup!"); ran `phables run --phagedetection prostt5-foldseek --dry-run` against that real installed database with zero --hallmark-db/--hallmark-categories flags and confirmed it resolves correctly. Full chain confirmed working on the actual artifact, not a placeholder. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Was still describing pre-fork phables -- the static --help block hadn't been updated since --databases, --build-tree, --genecaller, --phagedetection and its whole ProstT5/Foldseek/hallmark-* family, and --gpu-backend/--foldseek-gpu were added. Byte-diffed the replacement block against real `phables run --help` output (only difference: a stray backspace control char in Click's raw output, not meaningful content -- confirmed, not assumed). Added: - New flags to the "Run options explained" list (--build-tree, --genecaller, --databases). - A dedicated "Phage-gene detection: --phagedetection" subsection covering both methods and every prostt5-foldseek-only flag, including the auto-resolve behaviour for --hallmark-db/--hallmark-categories. - A worked example under "Example usage" for --phagedetection prostt5-foldseek, with and without --gpu-backend. - Fixed the `preprocess` output description, which only ever documented the mmseqs path (phrogs_annotations.tsv) -- now covers both outputs (hallmark_hits.tsv for prostt5-foldseek), added the missing gene-calling step, and noted coverage is now via CoverM (PLAN.md §4.8), not koverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 5 high |
| Security | 2 minor 1 high 3 critical 3 medium |
🟢 Metrics 132 complexity · 2 duplication
Metric Results Complexity 132 Duplication 2
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
The existing container/Dockerfile (3 prior commits: "first docker container", "Dockerfile with all dependencies included", "updating the dockerfile and how to use it") was a pre-fork artifact -- gurobi/optimizer:latest as a second, unused FROM base, `mamba install phables=1.3.2` (a PyPI version from long before this fork's pyrodigal-gv/ProstT5-Foldseek/koverage-removal/HiGHS work), libfabric built from source. None of it reflects the current codebase. Replaced entirely, not incrementally patched. New Dockerfile builds FROM quay.io/pawsey/pytorch:2.7.1-rocm6.3.3 -- the same base phold's own container (../phold/container/hpci/Dockerfile) uses, deliberately: it's the exact torch==2.7.1+rocm6.3 build envs/prostt5-rocm.yaml already pins and that's confirmed working on real Setonix MI250X hardware this session. Nothing in this Dockerfile reinstalls torch -- pholdlib and every other Python dependency pip-install directly into the base image's own system Python, so predict_3di sees the same already-correct torch/ROCm stack everything else does, per explicit direction: don't rebuild a separate (and possibly conflicting) prostt5 env for this container. Covers every envs/*.yaml this workflow has: foldseek (direct binary, matching phold's own install step exactly), minimap2/samtools/coverm/mmseqs2(=13.45111, the exact pin)/fraggenescan/hmmer/mafft via a standalone micromamba into /opt/conda, and phables' own Python deps (pyrodigal-gv, biopython, python-igraph, pysam, flowpaths, cogent3/piqtree, ...) via pip into the base image's Python. phables itself installed from this exact build context (COPY . + pip install, not a fetched git ref) so CI builds exactly what was committed. Real bugs caught and fixed on careful re-read before this ever ran anywhere: three unquoted `pkg>=X.Y` version specs (coverm, pholdlib, pyrodigal-gv) where the shell would have consumed `>` as redirection instead of passing it to micromamba/pip -- confirmed the actual shell behavior with a synthetic reproduction, not just inspection. Also confirmed the real FragGeneScan binary name (run_FragGeneScan.pl, not `fraggenescan`) against genes.smk's own invocation rather than guessing at the sanity-check command. CI (.github/workflows/build_container.yaml) builds and pushes to quay.io/<QUAY_USERNAME>/phables:<commit-sha> on every push/PR -- mirrors phold's own real, working workflow (../phold/.github/workflows/build_container.yaml) almost exactly, including the free-disk-space cleanup step that workflow's own comments explain is required (the ~14GB base image otherwise exceeds a fresh GitHub Actions runner's free disk on the first FROM line). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extends the existing predict_3di-only --prostt5-container mechanism to the
whole workflow. Real motivation: --use-conda's per-rule env creation has a
genuine race condition at HPC scale, hit in production this session -- every
rule's conda env is named by a hash of its env file, so every sample's
`phables run` needs the SAME env directory, but each sample runs as its own
separate Snakemake process (a SLURM array task). Snakemake only serializes
env creation WITHIN one process's DAG, not across processes, so two array
tasks both needing an env for the first time at the same moment can race
`mamba env create` into the same directory and corrupt it ("Fatal Python
error: init_fs_encoding ... no codec search functions registered"). A single
pre-built container with everything already installed has nothing left to
race.
New `--container` CLI flag / config['container'] -> CONTAINER_IMAGE global
(02_phables_preflight.smk). Every rule across coverage.smk, genes.smk,
gfa2fasta.smk, phables.smk, phylotree.smk, postprocess.smk now does:
conda:
None if CONTAINER_IMAGE else os.path.join("..", "envs", "X.yaml")
container:
CONTAINER_IMAGE
-- applied programmatically (a verified regex transform, not 15 manual edits)
to guarantee identical treatment everywhere, then confirmed via a real
Snakemake dry-run that both the untouched default path and the new
--container path resolve the identical 18-job DAG cleanly.
predict_3di (already container-capable via --prostt5-container) needed no
rule changes at all: PROSTT5_CONTAINER now falls back to CONTAINER_IMAGE when
unset, so --container alone routes predict_3di through it too, while
--prostt5-container still overrides it specifically if ever needed (e.g.
pointing just that rule at a different image).
docs/container.md explains the race condition, how to run
(--container ... --use-singularity --no-use-conda), and is explicit about
what's NOT yet verified: no real `docker build` or Apptainer/Singularity
execution against Setonix has happened, only the DAG-structural dry-run.
docs/usage.md and mkdocs.yml updated to match; --help block re-verified
byte-for-byte against the real `phables run --help` output after adding the
flag (same as the prior CLI-surface documentation pass this session).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cogent3 has no non-prerelease PyPI release, so a plain version-range pip install (cogent3<2026.7) fails outright (pip filters prereleases from range specs by default). Rather than pin an exact alpha version, just drop cogent3/piqtree from the image -- phylotree/--phylotree isn't part of this workflow's usage. Note --phylotree therefore isn't supported under --container (build_tree still points at CONTAINER_IMAGE, which now lacks these deps).
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.
43 commits. Bumps to 2.0.0
New
--phagedetection prostt5-foldseek— a second phage-gene detectionmethod alongside the existing PHROGs/MMseqs2 search: predicts each unitig
protein's 3Di structural alphabet with ProstT5
and searches it with Foldseek against a curated subDB of phage hallmark
protein structures (head-and-packaging/connector/tail/lysis PHROG
categories, subset from phold's
structure DB). Catches distant homologs sequence search misses, at the cost
of needing a GPU for reasonable throughput.
phables installfetches thepre-built hallmark subDB automatically (~105MB, hosted on
Zenodo);
--hallmark-db/--hallmark-categoriesauto-resolve once installed, same as the existingdatabases.
--gpu-backend {cpu,cuda,rocm}selects which PyTorch build theProstT5 conda env solves against (confirmed working on both NVIDIA/CUDA and
AMD MI250X/ROCm — Setonix);
--foldseek-gpuoptionally accelerates theFoldseek search itself (CUDA-only). Full writeup:
docs/hallmark_db.md.--genecaller {pyrodigal-gv,fraggenescan}(default:pyrodigal-gv) —pyrodigal-gv as a modern, faster drop-in replacement for FragGeneScan,
selectable per-run. Same
{unitig}_{start}_{end}_{strand}protein-idconvention preserved so both callers share every downstream consumer
unchanged.
FD_Inexact.pynow goes throughflowpaths (HiGHS-backed by default)
instead of raw
gurobipycalls. No commercial license needed anywhere inthe pipeline. Validated end-to-end against the bundled test fixture
(6/6 components resolved, sensible lengths/GC/coverage) — not yet
numerically diffed against a live Gurobi run (no license available to test
against).
koverageas a nestedSnakemake wrapper around CoverM) — both the preprocess (MFD-critical) and
postprocess (report-only) coverage steps now call
minimap2/samtools/covermdirectly. Removes a nested Snakemake dependency from inside analready-Snakemake workflow. The postprocess step's estimator changed
(CoverM's own columns vs. koverage's native-mode columns) — the three
values phables consumes still exist and the reader's indices were updated
to match, but this is report-only (
sample_genome_*.tsv, not read byanything downstream) and hasn't been numerically regression-tested against
old output on real data.
>=7.14.0) — needed for the executor-pluginarchitecture (
snakemake-executor-plugin-slurmetc.) thatresources: gpu=N→--gpus=Nauto-translation depends on.python_requiresbumped to
>=3.11to match (every 8.x/9.x Snakemake release on PyPIrequires it).
phables/workflow/profiles/gpu-routing/)— routes only
predict_3di/scan_hallmarkto a GPU partition via--workflow-profile, opt-in; every other rule (and every user without aGPU) is unaffected by default.
build_hallmark_db.py— the real script that built the hosted hallmarksubDB, ported in and documented (
docs/hallmark_db.md) for anyone whowants to verify it or rebuild against a newer phold snapshot.
Fixes bundled in along the way
get_all_simple_pathsrenamedcutoff→maxlen(long-readpath only).
get_components.--minlengthwasn't being applied to linear resolved paths, onlysingle-unitig components.
"tail"matching"tail fiber"and similar false positives).phables installsilently ignored--databases(missingmerge_config).--build-tree) — wasunconditional and slow at metagenome scale.
Testing
format_koverage_results.pycolumn-layout regression test, CLI smoke test) passes in a clean venv
mirroring CI exactly — not just locally, where several of these tests were
initially masked by globally-installed packages that CI's own
build/environment.ymldoesn't provide (fixed: CI's stale Python 3.9/3.10matrix incompatible with the
>=3.11requirement, and missingpyrodigal-gv/pandas/biopython for the two non-Snakemake-rule test files).snakemake --dry-run(Snakemake 9.25.1) against the bundledtests/data/ERR1301161fixture across every major conditional branch:mmseqs,prostt5-foldseek,--genecaller fraggenescan,--longreads,--gpu-backend cuda --foldseek-gpu, the documented--foldseek-gpu+non-CUDArejection,
--workflow-profile gpu-routing. All resolve cleanly.phables install+phables run --phagedetection prostt5-foldseek --dry-runagainst the actual hosted hallmark_db tarball(genuine download, not a placeholder) — confirms the whole auto-install →
auto-resolve chain works with zero
--hallmark-db/--hallmark-categoriesflags passed.
Adelaide) and AMD MI250X/ROCm (Pawsey Setonix), both via a plain conda env
(no container needed on either backend).
Known gaps
license to test against — inherent limitation).
on real data.
--dry-runparsing/job-graph construction only, not a real SLURM cluster submission.
mmseqsandprostt5-foldseekphage detection on a labelled benchmark set.Happy to split any of this into smaller follow-up PRs if that's easier to
review — flagging as one PR per earlier discussion, but the commit history is
intentionally kept granular (each commit is a self-contained, individually
reviewable change) in case that's useful.