Repository navigation
Conversation
Correctness fixes in the pipeline AST: - Command.__eq__ no longer mixes in self.proc (runtime state), so a command's identity is stable once its job runs. exec_kwargs (env, cwd) now participates in __eq__/__hash__ (canonicalised for order-independence) and renders into str() as a conventional bash prefix, so two jobs differing only in env/cwd no longer collide. - ProcSub.__eq__ requires the same subclass: <(x) != >(x). - Pipeline file/stream conflicts raise ValueError instead of calling sys.exit(), and the guards test `is not None` so fd 0 is not skipped. Fix a deadlock when a proc-sub FIFO is never opened: if the outer command failed to spawn or exited early, the background task's blocking open() never returned and Context.cleanup() hung the whole run, including the Ctrl-C path. cleanup() now unblocks still-unopened FIFOs (holding both ends non-blocking), lets the unblocked task return without spawning, and re-raises task exceptions only after all resources are released. Replace the injected `perl -MFcntl` pipe-buffer command with a native Pipeline(pipe_size=...) option. The injected command received the inherited stdout rather than the internal pipe it was meant to grow, so F_SETPIPE_SZ hit a non-pipe fd and the pipe kept its default 64 KiB size. The pipes are created with os.pipe() here, so they are now sized directly (best-effort; logged fallback on non-Linux or over fs.pipe-max-size) before the children inherit them. skip_pipe existed only for the injection and is removed; the bwa alignment pipelines in command_strings.py request the intended 256 MiB via pipe_size. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the undocumented send(None)/send(job) generator protocol between the DAG, scheduler, and executors with an explicit, typed interface: DAG.mark_finished(job) returns the jobs a completion unblocked, and BaseScheduler (new ABC) exposes start() / job_finished(job) with the contract documented and covered by a conformance-test harness. Make interrupt handling asyncio-correct and opt-in. The old handler ran time.sleep(10) and asyncio.run(context.cleanup()) inside a signal handler while the executor's event loop was running -- asyncio.run raises RuntimeError from within a running loop, and the blocking sleep froze it. Handlers (SIGINT/SIGTERM) are now installed on the running loop via loop.add_signal_handler only for the duration of a run (and only on the main thread), the previous handlers are restored afterwards, and shutdown is fully async: SIGTERM -> bounded grace period -> SIGKILL -> awaited cleanup. BasePipeline opts in, preserving Ctrl-C behavior. Executor correctness fixes: - A job that fails to launch is recorded in jobs_with_errors and stops new launches, instead of being dropped silently; the catch is narrowed to OSError so genuine bugs still propagate. - A later pipeline stage failing to spawn now terminates the stages that already started (e.g. `sleep 600 | no_such_cmd` no longer leaks sleep past the end of the run). - A proc-sub inner command that fails to spawn fails its own job instead of aborting the whole run mid-flight; teardown paths log (rather than re-raise) launch errors surfaced from cleanup so sibling contexts are still released. - A sub-command killed by SIGPIPE (downstream reader exited early) is no longer treated as a job failure, matching non-pipefail bash. Scheduler/DAG validation: - Jobs that can never be scheduled (threads over budget, or a managed resource request over capacity) raise DagExecutionError up front instead of stalling the run silently. - Adding a second job with an identical pipeline raises instead of silently collapsing onto the first. - DAG.add_job accepts any iterable of dependencies; a list or generator used to pass validation but was silently discarded, leaving the job immediately ready. Jobs may now declare inputs/outputs metadata: declared inputs are verified before a run and declared outputs after (StorageProvider seam, local presence checks), and a failed or interrupted job's declared outputs are deleted so a partial file cannot be mistaken for a good one. DAG.skip_satisfied plus the has_all_outputs predicate can prune already-satisfied jobs before a run, groundwork for a future resume option. The long-dead Job.run() method is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- main() created the temp dir and only removed it after build_dag() and
run() returned, so any raise in between (an infeasible DAG, a launch
failure, a KeyboardInterrupt) leaked the directory. Wrap
build/run/check in try/finally so the temp dir is always removed, and
honor retain_tmpdir on failure too so a failed run can be inspected.
- check_execution raised a bare ValueError("Execution failed"); raise
DagExecutionError naming the failed jobs instead, to aid post-mortems.
The unexecuted-jobs check uses DagExecutionError as well.
- setup_logging set the level on the logger's parent, which resolved to
the root logger -- so -v/--debug turned on DEBUG for every third-party
library. Add logging.set_level(), which scopes the level to the
sentieon_cli package logger; the module loggers are at NOTSET and
inherit from it, and the root logger is left untouched.
- add_arguments mutated the shared class-level params specs (deleting
"flags" and injecting inferred "type" entries), so registering a
pipeline class on a second parser lost its short flags (e.g. -r, -t).
Build a fresh kwargs dict per argument instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove engine capabilities that nothing in the CLI uses: - DAG.skip_satisfied() and the has_all_outputs predicate; no pipeline offers a resume option, so the pre-run prune had no callers. - The storage module (StorageProvider/LocalStorageProvider) and the executor's input/output staging built on it, along with LocalExecutor._remove_outputs(); pipelines do not declare per-job files, so the staging and failed-output removal were dead paths. - Job.inputs/outputs/image; Job is back to (pipeline, name, threads, resources). Drop the tests that covered the removed functionality and condense some overly verbose code comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Engine updates
Move logging handlers to the package logger so verbosity is enforced per handler: the console follows -v/-d (default now INFO) while a new run.log always records DEBUG, both with timestamps. A new --log_dir argument (default: the output VCF with .vcf.gz replaced by _logs) holds run.log, a command.txt invocation record, and a task_logs/ directory for upcoming per-process tool logs. Runs now finish with a status/elapsed-time message, emitted from a finally so it also lands on failure paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Job now requires a keyword-only task_name grouping jobs by pipeline
stage, and assigns each job a job_id ({name}-{n}) from per-name
counters that reset once per CLI invocation, so ids stay unique across
multi-DAG runs. Jobs report themselves by job_id in scheduler and
error messages. These fields feed the upcoming per-process task logs;
neither is part of job identity, which stays keyed on the shell
pipeline alone.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each job gets a JobLogSink on its execution Context, so every process
it spawns -- including process substitutions, which previously
bypassed stderr redirection entirely -- writes stderr to its own
task_logs/{task}/{job_id}.{stage}.log file, beginning with a header
recording the task, job, and command. Stdout is captured alongside
(headerless, sharing the process's stage index when it would
otherwise reach the terminal. Both streams of one process share a
stage index, allocated in spawn order.
On failure, the run log now reports the failing sub-command with its
exit code, pid, log path, and the last 20 lines of its log, plus a
pointer to the task-log directory; interrupted and launch-failed jobs
list their logs. Successful jobs prune logs that hold nothing but a
header.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
EOF
)
Log restructure
Create the log directory only after validation: setup_logging is now console-only and a new start_run_logs(), called after validate(), derives and prepares the log directory. A typo'd output path again fails fast instead of being silently created, a rerun rejected by validation no longer destroys the previous run's logs, and a log path colliding with a regular file exits cleanly instead of raising. Key job-id counters on the sanitized name (sanitize moved to util.py) so log file paths stay unique even for names that differ only in unsafe characters. Make -v/-q/-d independent flags with explicit precedence: -d -v no longer downgrades DEBUG to INFO, and the new --quiet restores a warnings-only console. Document the flags and the per-run log directory in the README. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Spawn pipeline processes with RusagePopen, a subprocess.Popen subclass whose _try_wait() reaps with os.wait4() and records the child's rusage, replacing asyncio.create_subprocess_exec (which only provided spawn and reap here: every stdio stream is already a real FD). Waits move to async_wait() (Popen.wait in a worker thread), and the executor raises the loop's default thread pool to 512 lazily-created workers so blocked waits and proc-sub FIFO opens cannot starve each other. Context.cleanup() gains a non-blocking poll() sweep so children nobody awaited are still reaped, as asyncio's watcher used to do. Behavior parity otherwise: negative returncodes on signal death, SIGPIPE tolerance, and the signal/kill paths are unchanged. Metrics emission lands separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each reaped process now logs a DEBUG line to run.log with its kernel resource usage (utime, stime, peak RSS, page faults, block I/O, context switches), emitted whatever the job's outcome -- signal-killed children included. Successful jobs extend the "Finished command" INFO line with the job's summed user/sys CPU and the per-process peak RSS (a sum would overstate concurrent stages that peak at different times). ru_maxrss is normalized across platforms (bytes on macOS, KiB elsewhere), and the DEBUG line renders it without an internal space (maxrss=33.6MiB) so the key=value line stays space-tokenizable for grep/awk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test_simple_command now asserts rusage is recorded at reap, and a new signal-death test pins the RusagePopen._try_wait override: if a future CPython changes that private method's shape, the override silently stops being called and rusage stays None, so the test asserts a SIGTERM'd child reports both its negative return code and its rusage. New tests/unit/test_rusage_metrics.py drives a job through LocalExecutor (the first unit-level file to do so) and checks the per-process "rusage for" DEBUG record, the aggregated "Finished command" INFO line, and _maxrss_bytes unit normalization on both platform branches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two fixes from a code review of the branch. First, an exception escaping _drive now tears the children down (_shutdown) before propagating: it previously unwound into asyncio.run's shutdown, which joins the pool threads blocked in wait4 on children nobody signalled, hanging the CLI until the children exited on their own. BaseException, so cancellation and KeyboardInterrupt are covered; a teardown failure is logged rather than allowed to mask the original error. Second, the wait/FIFO-open thread pool size is a keyword parameter instead of a hidden constant: None on AsyncExecutor (an executor that spawns nothing leaves the loop's default pool alone) and 512 on LocalExecutor, beside shutdown_grace_period. async_wait() returns immediately for an already-reaped child instead of burning a pool dispatch and a slot. Also document a known limitation honestly: a reap via poll() -- or send_signal/kill, which poll internally -- uses the base Popen machinery and records no rusage. The executor waits every process it reports metrics for, so those paths only collect processes whose rusage is never read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add metrics logging
Combine the human pangenome with sample short-read fastq and aligned long-read data for variant calling: k-mer counting across both read sets, personalized diploid graph construction with vg, graph updates with PGHapUpdateAlgo using the long-read alignments and LongReadSV calls, re-alignment of extracted short reads to the personalized pangenome with liftover, short-read dedup and metrics, and PangenomeSV + DNAscope calling with LR readgroup attributes. The dedup and metrics job builders move from SentieonPangenome to BasePangenome so both pangenome pipelines share them; the emitted dnascope-pangenome commands are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ngenome Aligned short-read BAM/CRAM input via `--sr_aln`, mutually exclusive with fastq input. Aligned input is assumed deduplicated: no dedup or metrics jobs run, read extraction and k-mer counting share a single pass over the input (driver ReadWriter | pgutil extract | kmc, with the long-read FASTA streams concatenated), and the lifted alignment is written directly to the output. The calling stages rewrite the input readgroups with `--replace_rg` to set LR:0. Unaligned long-read (uBAM/uCRAM) input via `--lr_align_input` and `--lr_input_ref`. Each input is realigned with minimap2 using the new `minimap2_lr.model` bundle member; k-mer counting reads the original input files in parallel with the realignment. Also fix `cmd_samtools_fastq_minimap2` and `cmd_samtools_fastq_bwa` to decode the input file with `input_ref` rather than the alignment target reference, matching the flag's documented purpose. This changes behavior for dnascope-longread and dnascope-hybrid runs where the two references differ. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace `sort -k1,1 -k2,2n` with `bedtools sort -faidx` in the LongReadSV BED generation so the regions sort correctly for references with an unusual contig order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add `--pangenome_contig_prefix` (default GRCh38#0#) and pass it to PGHapUpdateAlgo, pgutil lift, and PangenomeSV so a non-default pangenome reference name uses a consistent prefix throughout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reject BAM/CRAM inputs with no @rg header lines (previously the LR attribute was silently never applied, or build_dag crashed), require readgroup IDs to be unique across all inputs, and report malformed @rg header lines with the offending file instead of a traceback. check_kmc_patch now parses the k-mer count from the KMC output and fails on a zero count, catching KMC builds that read nothing from piped stdin while still exiting successfully. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
In Sentieon 202503.04 the HybridStage1 `--hap_bam` output is unsorted and `-` sends it to stdout. Send the hap BAM to stdout and pipe it to `sentieon util sort`, and move the algo's fastq output to a named fifo, `stage1_hap.fq`, read by bwa in the first-stage alignment. The haplotype job runs with zero scheduler threads so it always executes concurrently with the first-stage job reading the fifo. Requires sentieon driver 202503.04 or later. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The two-DAG main() override performed its tmpdir cleanup inline after the last check_execution, so a failing run skipped it and leaked the directory. Wrap the body in try/finally, mirroring BasePipeline.main. Found by qualification (DEFECT_sentieon_pangenome_tmpdir_leak). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restore the exit-status-only probe; the piped-KMC defect is being addressed in the KMC build itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the hardcoded sentieon-version matrix value with
${{ vars.SENTIEON_VERSION }} in all three smoke jobs, matching the
pattern already used by the Docker workflow. Each "Install sentieon"
step now fails with an explicit error when the variable is unset,
instead of a confusing tar failure on a 404 from S3.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Hybrid pangenome
In addition to the rusage log lines, write two machine-readable TSV files to the run's log directory so metrics can be loaded into pandas/awk without parsing run.log: - process_metrics.txt: one row per reaped process, straight from rusage, with a job_id column linking each process to its job and a stage column matching the task_logs file names. Rows are written for every reaped process whatever the job's outcome; the exit code is the raw wait status. A process reaped without rusage keeps its pid/exit_code and leaves the usage columns empty. - job_metrics.txt: one row per job aggregated across its processes: status (ok/failed), wall clock, summed user/sys CPU, per-process peak RSS (matching the "Finished command" line's semantics), and process count. Jobs that never reach accounting (launch failure, interrupt) get no row. Headers are (re)written in RunLogs.create_dirs(), so a rerun into the same --log_dir replaces the previous run's rows; rows append as each job finishes accounting, which also keeps partial data on failure and lets the pangenome pipeline's two DAGs share one set of files. Whitespace in the command field is flattened so rows stay rectangular, and a metrics write failure degrades to a warning rather than failing the job. JobLogSink gains a non-allocating stage_index_for() lookup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The metrics job received both the bwa and mm2 deduped alignments, but the mm2 alignment is a re-alignment of a subset of the same reads, so the extracted reads were double-counted in the collected metrics. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add scripts/sad_lad_update.py to the dnascope-pangenome pipeline as a final ad-update job. The joint pileup calls the bwa and mm2 alignments together; the script sets each sample's FORMAT/AD to either FORMAT/SAD or FORMAT/LAD -- whichever likelihood vector (SPL/LPL) better supports the called genotype -- and derives FORMAT/DP and INFO/DP from that choice. The previous values are kept as AD_ORIG/DP_ORIG and the choice is recorded in FORMAT/ADSRC and FORMAT/ADRULE. DNAModelApply (or the transfer concat under --skip_model_apply) now writes an intermediate in tmp_dir and the ad-update job writes the real output. With --gvcf the update runs on the gVCF ahead of GVCFtyper, so the genotyped VCF at --output_vcf carries the updated counts; gVCF hom-ref blocks pass through unchanged. No job is added under --skip_small_variants. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`DNAscopeHybridPipeline.call_variants` returned its 23 jobs as a tuple for `build_dag` to wire by hand. `add_variant_calling` now builds each job and inserts it with its dependencies as it goes, using `DNAscopeStage`, `ModelApplyStage`, `TransferStage` and the `driver_job` / `rm_job` helpers where they fit. Cleanup jobs are simply not created under `--retain_tmpdir`. The two DNAscope passes are now named `dnascope-1` and `dnascope-2` (were `calling-1` and `call2`). Commands and dependency edges are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`DNAscopeLRPipeline.lr_call_variants` returned a 27-field dataclass of jobs that `build_dag` wired by hand. `add_small_variant_calling` now composes the small-variant stages and inserts each job with its dependencies as it goes: `DNAscopeStage` for the five calling passes, `TransferApplyStage` for the diploid and unphased tails, and `TransferStage` plus `ModelApplyStage` for the two haplotypes, whose model-apply jobs wait on both haplotype transfers. The passes are now named after the stage: `dnascope-diploid`, `dnascope-hap1`/`hap2`, `dnascope-unphased`, `dnascope-haploid`, with matching `model-apply-*` jobs, and the four annotation transfers carry `diploid`/`hap1`/`hap2`/`unphased` tags in their job names. Commands and dependency edges are unchanged; a new unit test pins the non-obvious edges. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`stages/alignment.py` holds the bwa and minimap2 alignment stages (from BAM/CRAM input and from fastq), the pangenome bwa-plus-extract stage, and a ReadWriter stage, together with the shared `aln_suffix` and `find_unzip` helpers. Alignment jobs are always DAG roots, so these stages expose `build()` for callers that insert the jobs themselves as well as the usual `add_to()`. The hybrid-pangenome pipeline's copy of the minimap2 realignment and both pangenomes' bwa-extract builders are gone; the dnascope pipeline builds its alignment cleanup jobs from the stage's `cleanup_paths`. The long-read pipeline keeps its `lr_align_inputs`, `lr_align_fastq` and `merge_input_files` methods as thin wrappers. Commands, job names and dependency edges are unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Replace the repeated version-check loops with `require_versions` / `versions_available`, and the repeated "argument is required" exits with `BasePipeline.required` or the values already carried by the stage context. Drop `BasePipeline.cnv_sex_args`, which lost its last caller when CNV calling moved into a stage. The long-read and hybrid tests no longer patch `check_version` in the pipeline modules: they skip version checks, so the patches were inert, and the modules no longer import the function. The pangenome KMC test, which does exercise the check, patches it in `util` instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Drop `cmd_bedtools_merge`, `cmd_vg_giraffe`, `cmd_vg_pack`, `cmd_sv_call`, `cmd_vg_surject` and `strip_ctg_prefix` from `command_strings`, `Shard.bcftools_str` from `shard`, and `get_read_length_aln` (with its regex and `shlex` import) from `util`. None had a caller in the package, the tests, or the docs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- `find_unzip` always warns; the per-caller log level is gone. - The bwa alignment stages report `cleanup_paths` only when the alignment is an intermediate that duplicate marking consumes, as the field's documentation already said; final outputs get none, and the pipelines build the `rm-*-aln` job only when there is something to remove. - `TransferApplyStage`'s DNAModelApply depends on the transfer's concat alone (the concat already depends on the raw-VCF producer), or on `upstream` when there is no transfer. The hybrid pipelines now use the composite too, and the long-read haplotype passes each run their own transfer-then-apply, so a haplotype's model-apply waits only on its own transfer. - `build_transfer_jobs` moves into `stages/transfer.py`, its only caller, and the top-level `transfer.py` is gone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`ShortReadPreprocessingStage` carries the duplicate-marking and metrics policy that lived in `DNAscopePipeline.add_sr_preprocessing`; `MosdepthStage` and `LongReadSVStage` replace the raw job construction in the long-read pipeline's `mosdepth` and `call_svs`. The alignment, mosdepth and LongReadSV version constants move next to the stages they guard. The pipeline methods remain as thin wrappers that hold the version-check policy, so the hybrid pipeline, which still inherits them, is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`DNAscopeHybridPipeline` inherited from both `DNAscopePipeline` and `DNAscopeLRPipeline` to reuse six methods. It now subclasses `BasePipeline`, declares every attribute its arguments need, and composes the short-read preprocessing, minimap2 realignment, mosdepth and LongReadSV stages through its own small wrappers. The bwt_max_mem computation becomes `util.set_bwt_max_mem`, shared with the dnascope pipeline, and the long-read `call_svs` loses the `replace_rg` parameter only the hybrid used. Two latent defects go away with the inheritance: `--lr_fastq_taglist` now reaches the minimap2 command (the inherited method read the long-read pipeline's `fastq_taglist`), and a hybrid built without the argument parser gets `minimap2_args="-Y"`, its documented default, instead of the long-read pipeline's `-YL`. A new test asserts, for every pipeline, that each argument is declared in `__init__` with its argparse default. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The dnascope and long-read pipelines renamed their input attributes during validation (`r1_fastq` to `sr_r1_fastq`, `sample_input` to `lr_aln`, and so on) so that the hybrid pipeline could inherit from both without the names colliding. The hybrid no longer inherits from either, so each pipeline now reads its own argument names throughout. Two validation tests that set the renamed attributes before `validate()` ran, and so exercised the wrong branch, now assert the messages they were written for. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The version constants no longer all live in the pipeline modules. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The hybrid pipeline's long-read input may be ONT, whose very long reads can carry more than 65535 CIGAR operations, which the BAM format cannot store; `-L` moves such CIGAR strings into the `CG` tag. The hidden `--minimap2_args` default therefore becomes `-YL`, matching the long-read pipeline and the minimap2 stages, instead of `-Y`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`SentieonPangenome` overrode `main()` to parse the reference index and population VCF before validation, call `build_first_dag()` instead of `build_dag()`, and gate the second, sex-aware DAG itself. It now uses the base flow like every other pipeline: `validate()` parses the index, shards and VCF contigs (as the hybrid pangenome already did), the main DAG builder is `build_dag()`, and `build_second_dag()` returns `None` when no sex-aware caller is enabled and calls `get_sex` itself, as the hybrid DNAscope pipeline does. The reference index is validated once rather than twice. A new test asserts that no pipeline overrides `main()`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Four pipelines carried an identical `transfer_config()` helper building a `TransferConfig` from `pop_vcf`, `shards`, `pop_vcf_contigs` and `fai_data`. A `TransferInputs` protocol names those four attributes and `TransferConfig.from_pipeline` reads them structurally, raising a clear error when there is no population VCF. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`LongReadSVStage` gains an explicit `output` (the derived `.sv.vcf.gz` path stays the default) and the LongReadSV algo options (`min_map_qual`, `min_sv_size`, `min_dp`, `min_af`), which is what the hybrid pangenome's graph-update SV call needed. Its hand-built `build_longreadsv_job` is gone; the job keeps its `longreadsv` name under the `pangenome-update` task and its command is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The smoke reference is not a recognised genome build, so no PAR BED resolves and CNV calling now fails validation up front. The smoke runs exercise alignment, duplicate marking and variant calling, not CNVscope, so they skip it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Code refactor
Drop DEFAULT_UTIL_SORT_ARGS and the matching "--cram_write_options version=3.0,compressor=rans" defaults from the alignment stages, the command builders, and the hidden --util_sort_args argument of the dnascope, dnascope-longread and dnascope-hybrid pipelines, and stop passing the same option to Dedup. The default pipeline now writes CRAM with the tools' own defaults; the intermediate BAMs written before duplicate marking keep --bam_compression 1. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CNVscope can write its per-probe debugging output with --dump_probes. Add the argument to the CNVscope algo and CNVscopeStage, and have both sex-aware callers supply it: <output>.cnv.probes in dnascope-hybrid and <output>_cnv.probes in dnascope-pangenome, next to each CNV VCF. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`sentieon pgutil gfa2fa` in driver release 202503.04 handles only GRCh38 pangenomes; on a CHM13 graph it fails with "Failed to parse ref CHM13#0#chr1 in S line". Have hybrid-pangenome choose the tool for the gfa2fa job in validate(): when the reference is not GRCh38 (per detect_reference_build) and the driver is 202503.04 or older, extract the graph's reference paths with `vg paths -x <gfa> -Q <pangenome_ref_name> -F`, which is slower but works with every release; otherwise keep pgutil. An unknown driver version (--skip_version_check, or a failed probe) also takes the vg path for a non-GRCh38 reference, with the reason logged at INFO. The job keeps its name, threads, task and dependencies. Add util.executable_version, which returns a tool's parsed version, and build check_version on it; unparseable --version output now logs an error instead of raising. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Extract the optional pangenome callers into reusable stages so both pangenome pipelines can compose the same building blocks: `SegdupStage` (`stages/segdup.py`), `ExpansionHunterStage` (`stages/expansion.py`), `T1KStage` (`stages/t1k.py`, with the default HLA/KIR loci) and the composite `PangenomeCNVStage` in `stages/cnv.py` (CNVscope and CNVModelApply, indel2cnv.py on the PangenomeSV output, and combine_cnv.py over the two). `cmd_segdup_caller` gains optional `lr_alignments` / `lr_bundle` arguments for long-read calling; `cmd_pyexec_indel2cnv` and `cmd_pyexec_combine_cnv` move the two script invocations into `command_strings.py`. `util` gains `caller_sex_arg`, the shared `--sex male|female` mapping for ExpansionHunter and segdup-caller. No pipeline uses the new stages yet -- `sentieon_pangenome.py` keeps its inline builders, so the module-level version constants are duplicated for now -- and the DAG snapshots of all 88 configurations are identical. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`SentieonPangenome` now builds its optional callers from the stages added in the previous commit, and everything both pangenome pipelines will need moved up to `BasePangenome`. `BasePangenome` gains the shared caller arguments -- `par_bed`, `sample_sex`, `segdup_caller`, `expansion_catalog` and the six `t1k_*` (the two loci defaulting to the constants in `stages/t1k.py`) -- with the matching `__init__` attributes, `has_cnv_model`, and the DAG-2 stashes `ploidy_json` / `sr_alignments` / `sr_replace_rg` (`sr_replace_rg` is unused until the hybrid pipeline needs it). `validate_t1k` moved up unchanged, and four thin helpers build the stages: `output_path(suffix)`, `add_t1k(dag, ctx, inputs, upstream)`, `add_expansion(dag, ctx, alignment)`, `add_segdup(dag, ctx, sr_alignment, *, lr_alignment, lr_bundle, overrides)` and `add_pangenome_cnv(dag, ctx, sv_vcf, inputs, *, replace_rg, interval)`. `--call_svs` stays on `SentieonPangenome`. `hybrid-pangenome` inherits the new flags, which are accepted but build no jobs yet. Deleted from `sentieon_pangenome.py`: the `SEGDUP_MIN_VERSION` / `EXPANSION_MIN_VERSION` / `T1K_MIN_VERSION` constants (now `*_MIN_VERSIONS` next to their stages) and the default T1K loci, the moved params and `__init__` lines, `validate_t1k`, `build_segdup_job`, `build_t1k_jobs`, `build_expansion_job`, `_add_cnv_jobs`, `_build_indel2cnv_job`, `_build_combine_cnv_job`, and the dead `self.rw_bam` that `build_dag` assigned but nothing read. The DAG snapshot over all 88 configurations is identical, including `compare --with-ids`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Give `hybrid-pangenome` the argument surface, validation, and first-DAG jobs of the optional callers that `dnascope-pangenome` already has. The new `--call_cnvs` flag opts into CNVscope calling, and the shared `--sample_sex`, `--par_bed`, `--segdup_caller`, `--expansion_catalog` and `--t1k_*` arguments come from `BasePangenome`. `validate_bundle` now records whether the bundle carries a 'cnv.model' file and requires `diploid_model` when `--segdup_caller` is set, since segdup-caller reads it from the bundle passed as its `--lr_model`. `validate()` runs `validate_segdup`, `validate_expansion`, `validate_t1k` and `validate_cnv` after the readgroup checks. segdup-caller and ExpansionHunter accept a single `--sr_aln` file (segdup-caller also a single `--lr_aln`), `--segdup_caller` cannot be combined with `--skip_small_variants`, and `--call_cnvs` requires SV calling and a bundle CNV model. CNV runs resolve the PAR BED file during validation and exit when none is available; a run without `--bed` warns that CNVscope will cover every contig. `--bed` now restricts CNV calling as well as small-variant calling. The first DAG always adds the `PloidyStage`, writing `<output>_ploidy.json` whatever the enabled callers, and runs T1K HLA/KIR on the short reads. It also stashes the short-read alignments, their `--replace_rg` rows, and the single long-read alignment for the second DAG. `--call_cnvs`, `--segdup_caller` and `--expansion_catalog` are validated but build no second-DAG jobs yet; `build_second_dag` follows in the next commit. The DAG snapshots gain one `estimate-ploidy` row per hybrid-pangenome config and are otherwise identical. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nd DAG `HybridPangenome.build_second_dag` builds the sex-aware callers once ploidy estimation has run, using the short-read alignments, the sample sex and the long-read alignment stashed by `build_dag`. With `--call_cnvs` the DAG holds the `PangenomeCNVStage` jobs: CNVscope over the short reads only -- restricted by `--bed` and carrying the short-read `--replace_rg` rows for `--sr_aln` input -- CNVModelApply, `indel2cnv.py` over the PangenomeSV VCF, and `combine_cnv.py` writing `<output>_cnv.vcf.gz`. `--expansion_catalog` adds ExpansionHunter over the short reads. `--segdup_caller` adds segdup-caller with both data types (`--short` and `--long`) and this pipeline's model bundle as both `--sr_model` and `--lr_model`, since it ships the `diploid_model`. The DAG-snapshot harness gains seven hybrid-pangenome configs for the new callers; every pre-existing config is unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Hybrid pangenome callers
The `-Q <pangenome_ref_name>` (--paths-by) selector passed to `vg paths` selected incorrectly on the updated graph during testing. Run `vg paths -x <gfa> -F` without a selector, which writes every path in the GFA file, and rename the builder to cmd_vg_gfa2fa, mirroring cmd_pgutil_gfa2fa, now that it takes only the FASTA and GFA paths. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The image's segdup-caller version check imported `SEGDUP_MIN_VERSION` from `sentieon_cli.sentieon_pangenome`, which moved to `sentieon_cli.stages.segdup.SEGDUP_MIN_VERSIONS` when the caller stages were extracted, so the build failed with an ImportError. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A wrong `--pangenome_ref_name` was silent: `vg haplotypes --set-reference` only warns, the sampled graph loses its reference paths, `vg convert -Q` writes no rGFA tags and `pgutil lift` exits 0 with every read unmapped. Read the reference name and contig prefix from the GBWT metadata in the GBZ header instead (new `pangenome_meta` module, well under a second per graph). The backbone reference is the unfragmented reference sample. Make `--pangenome_ref_name` and `--pangenome_contig_prefix` hidden overrides that must agree with the graph, check the backbone contigs and build against the reference FASTA, replace the `grch38.gbz` filename-suffix check, apply the GRCh38 contig-length check only to GRCh38 backbones, and add `check-sample-gbz` / `check-sample-gfa` jobs to both DAGs so a sampled graph or GFA without the reference fails the run. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lo4R3WSNwCYMcKzvGKCqSs
`validate_grch38_contigs` compared the reference index and the `--pop_vcf` with a hard-coded table of the 25 GRCh38 chromosome lengths, so it only ran for a GRCh38 backbone and never looked at the 170 unplaced contigs the HPRC graph also carries. Drop the table and compare the two files instead: every contig of the backbone reference detected in the GBZ must carry the same length in the `--pop_vcf` header as in the reference FASTA index. A CHM13 graph is now checked the same way a GRCh38 one is, a contig absent from the pop VCF or without a length is an error, and the message names the first ten offenders with both lengths. Only the hidden `--skip_contig_checks` flag bypasses the check; `--skip_pangenome_name_checks` does not. The pop VCF header is now parsed in pure Python. `vcf_contigs` and `vcf_id` shelled out to `bcftools view -h`, and `vcf_contigs` returned a fake three-contig dictionary in dry runs, which left the dry run checking nothing and the real run checking everything. `read_vcf_header` opens the file with `gzip` when its first two bytes say so -- the extension is not trusted -- and stops at the first line that is not a header line, so the body is never read: the header of the 10 GB population VCF parses in about 1 ms. Dry runs now parse it too, and fall back only when the file cannot be read at all, which is what the unit tests and the placeholder files behind them need. The `SentieonVcfID` comparison follows the same rule, so a dry run catches a pop VCF that does not belong with the model bundle. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RWPD3suVQRpMZbJHijXhao
Gbz name validation
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.
No description provided.