Docker_v1.2.0 - #92
Merged
Merged
Conversation
The launcher required every subject and control to be the same file type. That
was broader than anything downstream needs, and it rejected generator inputs
outright since get_input_type never recognised the extension.
bam, cram and generator all funnel into the same concatenated generator in
runRufus.sh, and the Filter stage streams that generator, so they are
indistinguishable by the time reads are pulled. Those three may now be mixed
freely across -s and -c.
FASTQ still has to stand alone. A FASTQ subject populates _arg_fastqA/_arg_fastqB
and the Filter stage then reads only those mate files, so reads from any
bam/cram/generator alongside them would be counted but never filtered -- fewer
calls, no error. Output naming desyncs too, since post_process is handed
subject[0] while runRufus names off the first non-FASTQ subject.
Also in this change:
- Generators are rejected in windowed (-w) mode. -R is not applied to a
generator, so every array task would count and call the whole genome.
- -cr is emitted when any input is a CRAM, not just the first subject, which
broke mixed sets (e.g. a generator subject with a CRAM control: runRufus
kills the run on decoding a .cram with _arg_cramref unset). Both -r and -cr
are passed, because runRufus computes _arg_ref_cat from _arg_ref before the
control loop that would assign it from _arg_cramref; with -cr alone and a
non-CRAM subject the BWA prefix falls back to the reference path instead of
the extension-stripped prefix.
- Control generators work at all now. The branch set
parentGenerator="${parent}${region_postfix}" and wrote nothing, so it looked
for a per-region sibling (normal.generator.wg for a whole-genome run) that
nothing in this repo has ever created. It now copies the generator into the
run directory using the same naming the bam/cram branches produce, and checks
the file exists -- controls previously had no existence check.
- The launcher scans generator files for the paths they reference and binds
those directories, since a generator's data dependencies live in the file
body rather than on the command line. Strictly additive: only tokens that
resolve to something existing on the host are used, so runtime-assembled
paths and globs are skipped exactly as before. System directories and their
descendants are never bound -- binding /opt would shadow /opt/RUFUS.
- Each generator is then run inside the container and must produce SAM before
scripts are generated, which catches what the scan cannot see (paths from
environment variables, REF_PATH/REF_CACHE for CRAM decode, missing binaries).
Uses a detected runtime and skips with a warning when none is on PATH, as
setup has never required a container runtime on the submit host.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects in the generator support added by 8821de1, both found while testing a DSA control against a pre-built hash. 1. The preflight looked for a container runtime on PATH and skipped with a warning when it found none. But setup_slurm.sh is normally invoked from *inside* the container -- `apptainer exec rufus.sif bash /opt/RUFUS/singularity/setup_slurm.sh ...`, as every README example shows -- where no runtime binary exists and none is needed. It now detects that case via APPTAINER_CONTAINER/SINGULARITY_CONTAINER (falling back to /.singularity.d) and runs the generator directly; samtools is already on PATH. Entering the container is reserved for setup run on the host. Note the binds differ between the two: in-container we inherit the caller's exec binds, whereas the generated job scripts use BIND_MOUNTS. The failure text now says so. 2. Restore the <control><region_postfix> naming for control generators, which 8821de1 wrongly treated as dead code. It is load-bearing: RunJellyForRUFUS.sh early-returns when $GEN.Jhash exists, so a pre-built control hash placed beside the input as <control><region_postfix>.Jhash -- e.g. a DSA hash symlinked to DSA_SMHT004.1.generator.wg.Jhash -- is found and jellyfish is skipped entirely. The generator body is never executed then, and the file is legitimately empty. Renaming it meant the pre-built hash was missed and RUFUS would have built a control hash from whatever the generator contained, silently replacing the intended control. All three cases are now handled: a pre-built .Jhash beside the input is used as-is; a pre-scoped generator at that path is used as-is; only when neither exists is a runnable copy materialised, in the working directory rather than next to the user's input. The preflight skips generators with a pre-built hash for the same reason -- nothing will execute, so there is nothing to check. The dead-code call was mine and it was wrong: the convention is undocumented and appears only in run directories, which I did not check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
runRufus's modifiedJelly merge aborts when the subject hash and a pre-built control/DSA/exclude hash were built at different -s. The merge runs in the background with its exit unchecked, so the only symptom was an empty HashList that surfaced hours later as the misleading "No mutant hashes pulled from fastqs". Add a preflight -- after the control operand strings are assembled but before the multi-hour subject build -- that reads each pre-built control/exclude hash's -s from its header via `jellyfish info` and hard-exits with an actionable message if any differs from the intended subject size. Generator controls not yet on disk are skipped: they are built at the subject size and match by construction. Helpers normalize_hash_size (size token -> allocated power-of-two) and read_built_hash_size back it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
setup_slurm's getopts parser had no way to set the jellyfish hash size, so generated rufus_call.slurm scripts never carried -hs and fell back to the 16G whole-genome default -- which cannot merge against 64G DSA/tech controls, producing an empty HashList and the misleading "No mutant hashes pulled" failure hours in. Add a -H hash_size option (single-char, since the parser is getopts) mapping to runRufus's -hs, validated with the same size regex, and emit it from write_out_rest_of_rufus_args so it reaches the whole-genome and both windowed command-assembly paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jellyfish pre-faults the entire -s hash array at startup, so the hash size is a hard RAM floor, not a peak that ramps. A whole-genome -H 64G run under setup_slurm's 150G default -M is therefore OOM-killed in ~30s (the count array alone needs ~237 GiB). Add a setup-time check in arg_parser.sh that computes the floor for the effective hash size -- the -H override, else RUFUS's own 16G whole-genome / 1G windowed default -- rounding up to the power-of-two jellyfish actually allocates (so -H 48G is judged as the 64G array it rounds to), and errors before submission if -M is below it, reporting the required value with headroom guidance. Skips cleanly when -M carries no unit we can parse, so it never false-blocks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A multi-subject run (-s repeated) appended one `samtools view -h` line per subject to the same generator, so running the generator emitted two complete SAM streams, each with its own @hd. samtools aborts on a header mid-stream: - Filter (generator | collate | fastq) -- collate buffers before emitting, so it discarded the whole stream and Filter saw zero reads. Every region exited "no_reads_passed_filter" and the run produced no calls. - Counting (generator | fastq) -- streams, so it emitted subject 1's reads, hit the second header, and died. Its status was never checked, so jellyfish saw a clean EOF and built a hash from subject 1 alone, silently. Header emission is now tracked across the whole generator via _subj_hdr: the first emitting command carries it, later ones do not, and a pre-built .generator subject appended after the first has its header flags stripped. The FASTQ block continues the same flag instead of its own, so a bam+fastq mix stays single-headered too. Also reap the counting feeder explicitly and fail with exit 2 (tool failure, already handled by check_empty_hashes) when it dies, discarding the partial hash so a rerun cannot pick it up via the skip-if-exists check. A truncated read stream must not look like a complete count. Regression from 5f237af, which added -h to every generator line and replaced PassThroughSamCheck -- which parsed SAM itself and skipped @ lines anywhere -- with samtools collate/fastq, which cannot tolerate a repeated header. Verified on the two SMHT004 CRAMs over chr1:1000001-1050000: before, collate reported "Error reading input file" and 0 reads while counting truncated to 48540 of subject 1's 48839; after, both paths carry all 93431 reads (48839 + 44592) and Filter yields 46016 pairs. Single-subject output is unchanged at 48839. The counting exit-code contract still returns 0/1/2 for counted/no-coverage/tool-failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Covers the modality that regressed in 5f237af and is fixed in the preceding commit: a sample supplied as several files via repeated -s. setup_slurm.sh emits exactly that form from its comma-separated -s list, so this is a shipped path, but nothing in the suite exercised it. Splits the somatic tumor fixture by QNAME into two files -- a true partition, so the union is exactly the original and "split == single file" is a fair comparison -- and runs three arms: single-file baseline, two-part BAM, and two-part CRAM (the -T/-cr generator line is separate code from the BAM one). Asserts the split really is a partition, the generator carries both subjects but emits exactly one @hd, every planted somatic survives, and both split call sets equal the baseline. The @hd and record-count checks are what make a failure diagnosable. Without them a reintroduced duplicate header shows up only as "no calls", which is indistinguishable from a dozen unrelated failures. They fail on different regressions: @hd on a header emitted per subject (the generator still carries every record; samtools truncates or discards downstream), record count on a subject file dropped from the generator outright. The latter is not redundant with concordance -- each part is ~15x and the planted somatics sit near 0.46 VAF, so a single part still clears the shipped -m 5 and would likely produce the same calls. Setup runs in ONE container invocation. Container startup dominates this step and was observed at ~5 min per exec on a busy node, which turned a 9-minute test into an hour when the setup issued eleven of them. run_case sets a global instead of echoing its path: called as `CALLS=$(run_case ...)`, fail()'s RESULT line would be captured into the variable instead of the log run_all.sh greps, and `exit 1` would only leave the substitution subshell, so set -e aborted the script with no message anywhere. Observed for real on the stock-image control run. NOTE: f3_input_formats.sh has the same latent flaw, not touched here. Verified both directions against the fixtures. With the fix overlaid (job 17685641): baseline/multi-bam/multi-cram all 7 calls and byte-identical, commands=2 header-flags=1 @hd=1 records=40000 on both multi-subject generators, 5/5 planted somatics recovered. Against the stock image without the fix (job 17684774): FAILS at the multi-bam arm with `chr20 NO_VARIANTS no_reads_passed_filter` -- the same symptom the bug produced on real windowed SMHT004 data. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ator + F7 multi-subject test
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
1 participant
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.
RUFUS v1.2.0
Highlights
-srepeatedly to combine split read files for one sample.New features
-q1/-q2path. (Single-end: see Known limitations.)-smay be given multiple times to merge split reads for one sample; theSAM header is now emitted once per generator, not once per subject.
with clear errors on unsupported mixtures.
setup_slurmhash sizing:-hsis plumbed end-to-end; the helper rejects a-Mmemory requestbelow the jellyfish hash-size floor and fails fast on a subject/control hash-size mismatch (which
would otherwise silently yield zero mutant k-mers).
guard requires the git tag to match
RUFUS_VERSION.Fixes
AddSAtoReadSame.pl).BCFTOOLS_PLUGINSso a host environment can no longer break VCF post-processing.bcftoolscrash in the whole-genome post-processing (empty-region trim).bgzip -f/index -f).Testing & reliability
somatic + SV, BAM/CRAM concordance, specificity, paired FASTQ, single-end FASTQ, multi-subject —
plus a
run_allrunner.RUFUS.interpretreplay harness and a documented defect audit.Known limitations
currently over-filtered by strand-bias handling and the final VCF may be empty. Use paired-end
input. A single-end strand-bias fix is staged for a future release.
RUFUS.interpret: the coverage model is not built in the current run modes (see the interpretaudit), and strand-bias thresholds are provisional. These affect FILTER/genotype fields, not the
set of variants detected.