Add sativaepang/* modules: reference, lootasks, looplace, looscore - #12910
Add sativaepang/* modules: reference, lootasks, looplace, looscore#12910erikrikarddaniel wants to merge 7 commits into
Conversation
Builds a SATIVA reference (refjson + model) from an alignment and taxonomy, optionally handing off a tree built elsewhere (e.g. RAxML-NG) via -reftree/-refmodel instead of running sativa-epang's own constrained RAxML search. First of four sativaepang/* modules exposing sativa-epang's staged leave-one-out CLI as separate nf-core modules. Tested against two tiers on both Docker and Singularity: the tool's own bundled 38-sequence example, and a curated 121-sequence GTDB archaeal 16S dataset. refjson embeds its own build invocation (working-directory-relative paths and all) so it isn't byte-reproducible across engines; masked via sanitizeOutput's unstableKeys, with explicit JSON-structure assertions covering what the module actually guarantees instead. sativa-epang has no --version flag and its own internal version string is stale (inherited from upstream SATIVA); the installed conda package version is instead read back out of the share/ path baked into the sativa-epang wrapper script at build time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup
Deals a sativaepang refjson into leave-one-out folds via -stage
loo-tasks, one self-contained task directory per fold plus a
manifest.json, ready for independent EPA-ng placement.
Works around a real sativa-epang bug: -stage loo-tasks with -r (a
refjson built from an externally-supplied -reftree/-refmodel, as ours
always is) leaves no RAxML_info file for the tool to recover its model
from, and embeds the .model file's raw content -- including RAxML-NG's
own trailing partition clause (", name = range") -- verbatim into
manifest.json. loo-place then rejects every fold ("Wrong model
specification"), since inline-mode EPA-ng doesn't accept that clause
the way file-mode does at the reference stage. Worked around by
exporting SATIVA_EPANG_MODEL ourselves, trimmed to the bare model
expression. Reported upstream; not yet fixed as of this commit.
Tested against both tiers on Docker and Singularity.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup
Places every leave-one-out fold with EPA-ng via -stage loo-place, writing a jplace (plus logs) into each fold directory. Unlike a typical module, this one takes a whole directory and mutates it -- but naively writing into the staged taskdir would risk mutating sativaepang/lootasks's own output in place (a symlink under local staging, or a real copy under stageInMode 'copy'), the same class of -resume-breaking bug fixed in nf-core#12799 (TRANSDECODER_PREDICT). Fixed the same way: stage a private, writable directory tree (real new directories for each fold, symlinked read-only contents) before running loo-place, so the upstream task's output is never touched. Verified this actually holds, not just assumed: manually staged a read-only unplaced taskdir through the exact mv+mkdir+symlink script this module runs and confirmed the original directory gained zero new files while the restaged local copy received real jplace/log files. Tested against both tiers on Docker and Singularity -- unlike sativaepang/reference's refjson, the jplace outputs here are byte-reproducible across engines, no unstableKeys masking needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup
Reads the placed leave-one-out folds and reports mislabelled sequences via -stage loo-score, completing the four-module chain (reference -> lootasks -> looplace -> looscore) that replaces sativa-epang's monolithic CLI with one nf-core module per natural stage. loo-score only ever reads taskdir (jplace files, manifest.json) and writes its own output elsewhere, so unlike looplace this one takes taskdir as a plain read-only input -- confirmed by hand before relying on it (ran loo-score against a taskdir and diffed its contents before/after). Tested against both tiers on Docker and Singularity. The gtdb test is a real detection assertion, not just process.success: that dataset carries a deliberate swapped-label pair (DupHaloA/DupSulfoA), and both are asserted present in the .mis output at the confidence the full chain actually produces. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup
CI's conda job failed: EPA-ng's placement likelihoods aren't byte-reproducible between the conda environment and the container builds, even though the underlying inputs and logs otherwise match. epa_info.log/epang.log also carry a wall-clock "Elapsed Time: Ns" line that flips between runs under load on any engine, causing the same class of flake on Docker/Singularity too. Mask all three (epa_result.jplace, epa_info.log, epang.log) via sanitizeOutput's unstablePatterns, matching the existing sigprofiler/snapaligner precedent in this repo. The structural asserts already in place (every fold directory contains a jplace) remain the actual correctness guarantee. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup
|
The conda CI job failed: EPA-ng's placement likelihoods aren't byte-reproducible between the conda environment and the container builds (identical inputs/logs, differing Pushed df09b4f masking all three via 🤖 Generated with Claude Code |
SPPearce
left a comment
There was a problem hiding this comment.
Can you have a pass to trim out a bunch of the LLM comments, both in the modules and the tests.
Any # comment you put into the script block will be present in the .command.sh I think.
Can you also use sanitizeOutput basically everywhere in the nf-test files.
| # \$taskdir is staged from sativaepang/lootasks's own output -- a symlink under | ||
| # local/shared-filesystem staging, but a real copy under stageInMode 'copy' (the | ||
| # default for cloud storage without Fusion). loo-place writes a jplace (and logs) | ||
| # inside every fold directory, which would mutate that other task's output in place | ||
| # and make this task uncacheable across -resume (same class of bug as | ||
| # nf-core/modules#12799). Give it a private, writable directory tree instead: fold | ||
| # subdirectories are real (new) directories here, their read-only contents | ||
| # (ref.nwk/ref.fasta/query.fasta) stay symlinks to avoid copying large alignments, | ||
| # and manifest.json at the top level is a plain symlink since loo-place never writes | ||
| # there. mv handles both staging modes: it moves a symlink as a symlink, and renames | ||
| # a real directory in place, so nothing is deleted either way. | ||
| mv "$taskdir" "${taskdir}.staged" | ||
| real_taskdir=\$(readlink -f "${taskdir}.staged") | ||
| mkdir "$taskdir" | ||
| find "\$real_taskdir" -mindepth 1 -maxdepth 1 | while read -r entry; do | ||
| name=\$(basename "\$entry") | ||
| if [ -d "\$entry" ]; then | ||
| mkdir "$taskdir/\$name" | ||
| find "\$entry" -mindepth 1 -maxdepth 1 -exec ln -s {} "$taskdir/\$name"/ \\; | ||
| else | ||
| ln -s "\$entry" "$taskdir/\$name" | ||
| fi | ||
| done |
There was a problem hiding this comment.
This is a lot of code, is it really required?
There was a problem hiding this comment.
Some form of private writable staging is required: sativa-epang writes results into every fold directory, and taskdir can be staged as a plain symlink straight into lootasks' own task directory (default local-executor staging) — writing into it in place would mutate that other task's output and break -resume the same way #12799 did.
You're right that the generic version was overkill though: it was walking an arbitrary file/directory tree, but lootasks' output schema is actually fixed (manifest.json + fold_*/{ref.nwk,ref.fasta,query.fasta}, confirmed against a real multi-fold run — nothing else ever appears). Pushed in adf8590: hardcoded to that shape instead, about a third of the size.
| # and manifest.json at the top level is a plain symlink since loo-place never writes | ||
| # there. mv handles both staging modes: it moves a symlink as a symlink, and renames | ||
| # a real directory in place, so nothing is deleted either way. | ||
| mv "$taskdir" "${taskdir}.staged" |
There was a problem hiding this comment.
Surely just stage the original with this name instead.
There was a problem hiding this comment.
Same reasoning as in my reply above — happy to take another look if you still see room to trim further once you've seen the shrunk version.
- Simplify the version-extraction eval line in all four modules (one
grep call instead of a sed one-liner), same output verified.
- Shrink looplace's private-writable-taskdir staging: the schema is
fixed (manifest.json + fold_*/{ref.nwk,ref.fasta,query.fasta}, never
anything else -- confirmed against a real 25-fold run), so the
generic find-based file/dir walk isn't needed. Kept the symlink
approach over stageInMode 'copy': lootasks already writes each
fold's ref.fasta as a real per-fold copy, so a second full copy here
would double an already GTDB-scale-relevant cost.
- Trim comments that were sitting inside script blocks (leaking into
.command.sh) or otherwise over-explaining, in both modules and tests.
- Add sanitizeOutput to looscore's two non-stub tests, matching the
other three modules -- also drops the redundant positional (0/1)
snapshot entries that duplicated the named outputs.
Addresses review from SPPearce on nf-core#12910.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup
|
Thanks for the review! Pushed adf8590 addressing all four points:
Re-verified all 12 tests (4 modules) pass under Docker, Singularity, and conda. 🤖 Generated with Claude Code |
nf-core modules lint's meta_yml checks compare the eval expression recorded in meta.yml against main.nf verbatim -- adf8590 simplified the eval command in main.nf but left meta.yml stale, failing lint on all four modules. Regenerated via `nf-core modules lint --fix`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup
|
Hi Daniel, Small correction to "not yet fixed": the loo-tasks model bug is fixed in 5c6bd1e on I can tag v0.9.3.7 and push it to bioconda if that is useful. Happy to do it now if |
PR checklist
topic: versions- See version_topicslabelnf-core modules test <MODULE> --profile dockernf-core modules test <MODULE> --profile singularitynf-core modules test <MODULE> --profile condaDescription
Adds four new modules exposing sativa-epang's staged
leave-one-out CLI, one nf-core module per natural stage, replacing what would otherwise be a single
monolithic-CLI wrapper:
sativaepang/reference— builds a SATIVA reference (refjson + model) from an alignment andtaxonomy, optionally handing off a tree built elsewhere (e.g. RAxML-NG, see
raxmlng/search) via
-reftree/-refmodelinstead of running sativa-epang's own constrained RAxML search.
sativaepang/lootasks— deals the refjson into leave-one-out folds via-stage loo-tasks, oneself-contained task directory per fold plus a
manifest.json.sativaepang/looplace— places every leave-one-out fold with EPA-ng via-stage loo-place,writing a jplace (plus logs) into each fold directory.
sativaepang/looscore— reads the placed folds and reports mislabelled sequences via-stage loo-score, completing the chain.These four modules will back a
fasta_tax_detectmislabels_sativaepangsubworkflow (openedseparately once this and raxmlng/search have landed) used by
nf-core/taxmarker.
Notable implementation details
lootasksworks around a real sativa-epang bug:-stage loo-tasks -r(a refjson built froman externally-supplied
-reftree/-refmodel, as ours always is) leaves noRAxML_infofile forthe tool to recover its model from, and embeds the
.modelfile's raw content — includingRAxML-NG's own trailing partition clause (
, name = range) — verbatim intomanifest.json.loo-placethen rejects every fold ("Wrong model specification"), since inline-mode EPA-ngdoesn't accept that clause the way file-mode does at the reference stage. Worked around by
exporting
SATIVA_EPANG_MODELourselves, trimmed to the bare model expression. Reportedupstream; not yet fixed as of this PR.
looplacestages a private, writable copy of its input taskdir before runningloo-place,rather than mutating the staged directory from
lootasksin place — the same class ofresume-breaking bug fixed in transdecoder/predict cannot be resumed: it writes checkpoints into its staged input, then fails on re-execution #12799 (
TRANSDECODER_PREDICT). Verified by hand: staging aread-only unplaced taskdir through the same mv+mkdir+symlink script confirmed the original
directory gained zero new files.
reference's refjson output embeds its own build invocation (working-directory-relativepaths and all), so it isn't byte-reproducible across engines — masked via
sanitizeOutput'sunstableKeys, with explicit JSON-structure assertions covering what the module actuallyguarantees instead.
--versionflag and its own internal version string is stale (inheritedfrom upstream SATIVA); the installed conda package version is read back out of the
share/path baked into the sativa-epang wrapper script at build time.
Test data
Both tiers now live in nf-core/test-datasets#2266 (merged): the tool's own bundled 38-sequence
tiny example, and a curated 121-sequence GTDB archaeal 16S dataset (which carries a deliberate
swapped-label pair used as a real detection assertion in
looscore's tests, not justprocess.success).🤖 Generated with Claude Code
https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup