You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Following on from #503/#504/#505, I have a working DRAM-v reimplementation for the DRAM2 Nextflow workflow that runs in a fork, and I would like to contribute it upstream. Opening this before sending code, because it is a large enough change that I would rather agree the shape with you first than drop it on you unannounced.
Context for why it exists: we needed AMG annotation for a virome catalogue, and viral mode is not yet in the Nextflow workflow, so we built it out. It has been in production use on a real vOTU catalogue.
Worth saying up front: this builds on your existing VOG support rather than replacing it. HMM_SEARCH_VOG and hmm_parser's {db_name}_id/{db_name}_ids columns already give the flag engine everything it needs, so none of the annotation-side plumbing has to change.
Enabled behind a single --use_dramv flag, which force-enables use_pfam and use_vog.
bin/dramv_flags.py plus a DRAMV_FLAGS module wired into ANNOTATE. Computes is_transposon, amg_flags and auxiliary_score as per-gene columns and publishes annotations_with_flags.tsv. Flag and score semantics are ported from v1 mag_annotator/annotate_vgfs.py (commit 6cd68f9), including the B-flag score downgrade.
A V flag, firing when a gene's VOGdb hit has functional category Xr (viral replication) or Xs (virion structure), read from vog_annotations_latest.tsv.gz. Consumes your existing VOG output as-is.
A geNomad adapter, so auxiliary_score works without VirSorter. geNomad's gene-level TSV is mapped onto the VirSorter category semantics the v1 scoring algorithm expects. Includes a position-overlap join for CheckV-trimmed proviruses, where gene ids do not match directly, and an opt-in catalog mode for sample-prefixed gene ids. This is effectively Suggestion: Decouple DRAM-v from VirSorter Requirement #35.
distill.py --amg_only with --max_auxiliary_score, filtering to strict AMG candidates (M present, A/P/T/N absent).
27 unit tests covering flag computation, scoring branches, the geNomad parser and the database loader.
A README chapter.
Without VirSorter or geNomad input everything still runs, with auxiliary_score falling back to 5 for every gene, matching v1 behaviour.
What is not implemented
No VirSorter affi-contigs.tab parser. The scoring function takes a virsorter_categories dict, so the hook exists, but nothing populates it from VirSorter today. Only geNomad is wired.
The geNomad to VirSorter category mapping is lossy: it collapses the phage/prophage distinction (v1's 0/3 and 1/4), which is harmless because the scoring algorithm treats those pairs as equivalent.
No CAZyme handling in the AMG path. The auxiliary-score flag is KEGG-anchored and under-resolves CAZymes, so we ran dbCAN separately.
Not tested against DRAM v1 output for exact parity. Semantics were ported by reading v1, and are unit tested, but I have not run a side-by-side on a shared dataset. If you want that as a merge condition, say so and I will do it.
Proposed sequence
I would rather send several small PRs against dev than one large one:
DRAMV_FLAGS: the flag engine, amg_database.tsv, constants, unit tests, ANNOTATE wiring, behind --use_dramv. Produces amg_flags and is_transposon only.
distill.py --amg_only.
geNomad adapter, auxiliary_score, and --max_auxiliary_score.
Docs.
(1) is already carved onto a branch off your current dev, so I can give exact numbers: 1,659 insertions and 0 deletions. Only 57 of those lines touch existing files, and every one is an addition, spread across annotate.nf, workflows/dram.nf, conf/modules.config, nextflow_schema.json and nextflow.config. Its 16 unit tests pass against the Rule-Parser commit your dev currently pins, so it needs no submodule bump, and it is clean under your pre-commit hooks.
dev currently pins, so it needs no submodule bump, and it is clean under your
pre-commit hooks.
Things I would want your steer on
Do you want this at all, or is viral mode already on your roadmap in a form this would cut across? I would rather find out now.
distill.py. Our fork rewrote it onto polars before Feature/dram ag distill with new rule parser for distill, traits, and product #480 landed, so the two versions have diverged substantially. I do not intend to bring any of that across. I would port only the --amg_only option block onto your current file. Flagging it because the fork diff looks alarming and mostly is not relevant.
Rule-Parser.dramv_flags.py works against a local copy of ID_EXPR_DICT for two reasons. First, vogdb_id/vogdb_ids are not in it. Second, several entries assume str.extract_all returns capture groups when it returns full matches, so pfam_hits yields [PF01609.1] rather than PF01609 and never intersects a bare accession. I have filed that separately with a reproducer (ID_EXPR_DICT regexes assume str.extract_all returns capture groups; it returns full matches Rule-Parser#3) rather than guessing at the intended token form. I will not touch the submodule pointer in any DRAM PR.
Pfam.is_transposon and the T flag are computed from Pfam hits, so they are only as good as the Pfam database. I see the anno_dbs route for it is commented out in workflows/dram.nf pending the mmseqs profile rebuild in Pfam #498, though --use_pfam itself still works and the search path in db_search.nf is intact. So --use_dramv runs today, but T will be unreliable until that rebuild lands. Happy to gate or document that however you prefer, and if it would help I can hold the flag work until Pfam is back.
dbCAN. We independently implemented rundbcan/easysubstrate before Feature/update to dbcan3 #500 merged. Yours is the one to keep; I mention it only so the duplication does not confuse anyone comparing the fork.
Timeline, in the interest of being straightforward
We have a manuscript in preparation that uses this and currently cites the fork. That is not a reason for you to hurry, and I am not asking you to. I would just rather be open about the motivation than have it look like an unexplained push later.
Where to start
(1) is ready to open whenever you want it. If you would rather check the shape on something smaller first, I am happy to lead with (2) instead, which is a single CLI option on distill.py and about 60 lines. Either way, tell me which you prefer and whether the four-way split above is the right carve, and I will send that one first.
Following on from #503/#504/#505, I have a working DRAM-v reimplementation for the DRAM2 Nextflow workflow that runs in a fork, and I would like to contribute it upstream. Opening this before sending code, because it is a large enough change that I would rather agree the shape with you first than drop it on you unannounced.
Context for why it exists: we needed AMG annotation for a virome catalogue, and viral mode is not yet in the Nextflow workflow, so we built it out. It has been in production use on a real vOTU catalogue.
Worth saying up front: this builds on your existing VOG support rather than replacing it.
HMM_SEARCH_VOGandhmm_parser's{db_name}_id/{db_name}_idscolumns already give the flag engine everything it needs, so none of the annotation-side plumbing has to change.Fork: https://github.com/tpall/DRAM/tree/dev
What is implemented
Enabled behind a single
--use_dramvflag, which force-enablesuse_pfamanduse_vog.bin/dramv_flags.pyplus aDRAMV_FLAGSmodule wired intoANNOTATE. Computesis_transposon,amg_flagsandauxiliary_scoreas per-gene columns and publishesannotations_with_flags.tsv. Flag and score semantics are ported from v1mag_annotator/annotate_vgfs.py(commit 6cd68f9), including the B-flag score downgrade.Vflag, firing when a gene's VOGdb hit has functional category Xr (viral replication) or Xs (virion structure), read fromvog_annotations_latest.tsv.gz. Consumes your existing VOG output as-is.bin/assets/amg_database.tsv, vendored from v1, plus a newessential_viral_functioncolumn and a corresponding informationalNflag, following Martin et al. 2025 (doi:10.1038/s41564-025-02095-4).Ndoes not propagate toMand does not change any existing filter. This is directly responsive to Inquiry: Expanding DRAM-v workflow for the identification of broader Auxiliary Viral Genes (AVGs) #495.auxiliary_scoreworks without VirSorter. geNomad's gene-level TSV is mapped onto the VirSorter category semantics the v1 scoring algorithm expects. Includes a position-overlap join for CheckV-trimmed proviruses, where gene ids do not match directly, and an opt-in catalog mode for sample-prefixed gene ids. This is effectively Suggestion: Decouple DRAM-v from VirSorter Requirement #35.distill.py --amg_onlywith--max_auxiliary_score, filtering to strict AMG candidates (Mpresent,A/P/T/Nabsent).Without VirSorter or geNomad input everything still runs, with
auxiliary_scorefalling back to 5 for every gene, matching v1 behaviour.What is not implemented
affi-contigs.tabparser. The scoring function takes avirsorter_categoriesdict, so the hook exists, but nothing populates it from VirSorter today. Only geNomad is wired.Proposed sequence
I would rather send several small PRs against
devthan one large one:DRAMV_FLAGS: the flag engine,amg_database.tsv, constants, unit tests,ANNOTATEwiring, behind--use_dramv. Producesamg_flagsandis_transposononly.distill.py --amg_only.auxiliary_score, and--max_auxiliary_score.(1) is already carved onto a branch off your current
dev, so I can give exact numbers: 1,659 insertions and 0 deletions. Only 57 of those lines touch existing files, and every one is an addition, spread acrossannotate.nf,workflows/dram.nf,conf/modules.config,nextflow_schema.jsonandnextflow.config. Its 16 unit tests pass against the Rule-Parser commit yourdevcurrently pins, so it needs no submodule bump, and it is clean under your pre-commit hooks.devcurrently pins, so it needs no submodule bump, and it is clean under yourpre-commit hooks.
Things I would want your steer on
distill.py. Our fork rewrote it onto polars before Feature/dram ag distill with new rule parser for distill, traits, and product #480 landed, so the two versions have diverged substantially. I do not intend to bring any of that across. I would port only the--amg_onlyoption block onto your current file. Flagging it because the fork diff looks alarming and mostly is not relevant.dramv_flags.pyworks against a local copy ofID_EXPR_DICTfor two reasons. First,vogdb_id/vogdb_idsare not in it. Second, several entries assumestr.extract_allreturns capture groups when it returns full matches, sopfam_hitsyields[PF01609.1]rather thanPF01609and never intersects a bare accession. I have filed that separately with a reproducer (ID_EXPR_DICTregexes assumestr.extract_allreturns capture groups; it returns full matches Rule-Parser#3) rather than guessing at the intended token form. I will not touch the submodule pointer in any DRAM PR.is_transposonand theTflag are computed from Pfam hits, so they are only as good as the Pfam database. I see theanno_dbsroute for it is commented out inworkflows/dram.nfpending the mmseqs profile rebuild in Pfam #498, though--use_pfamitself still works and the search path indb_search.nfis intact. So--use_dramvruns today, butTwill be unreliable until that rebuild lands. Happy to gate or document that however you prefer, and if it would help I can hold the flag work until Pfam is back.rundbcan/easysubstratebefore Feature/update to dbcan3 #500 merged. Yours is the one to keep; I mention it only so the duplication does not confuse anyone comparing the fork.Timeline, in the interest of being straightforward
We have a manuscript in preparation that uses this and currently cites the fork. That is not a reason for you to hurry, and I am not asking you to. I would just rather be open about the motivation than have it look like an unexplained push later.
Where to start
(1) is ready to open whenever you want it. If you would rather check the shape on something smaller first, I am happy to lead with (2) instead, which is a single CLI option on
distill.pyand about 60 lines. Either way, tell me which you prefer and whether the four-way split above is the right carve, and I will send that one first.