WIP: Toil times - #1958
Open
glennhickey wants to merge 23 commits into
Open
WIP: Toil times#1958glennhickey wants to merge 23 commits into
glennhickey wants to merge 23 commits into
Conversation
cactus-panpatch's main() was missing the standard per-tool boilerplate that every other cactus entry point runs after parsing options: - cactus_override_toil_options(options): sets toil defaults (retryCount, realtime logging, default memory) and exports CACTUS_MAX_MEMORY / CACTUS_DEFAULT_MEMORY. panpatch calls cactus_clamp_memory(), which reads those env vars, so without this it would KeyError. - setupBinaries(options): configures binariesMode/docker so cactus_call works as requested (panpatch runs vg, bgzip, etc. via cactus_call). - set_logging_from_options(options) and enableDumpStack(): consistent logging setup and debug stack dumps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t Slurm partition Newer Toil exposes a per-job `walltime` requirement that its Slurm backend uses to pick a partition (and set --time). This gives Cactus's many small "coordination" jobs a short walltime so Slurm routes them to a fast/short partition, while everything else falls back to Toil's --defaultWalltime. Machinery (src/cactus/shared/common.py): - cactus_fast_walltime(): reads CACTUS_FAST_WALLTIME (mirrors cactus_clamp_memory). - add_cactus_toil_options(): adds --fastWalltime (default 1800 = 30 min, very conservative for these jobs; 0 disables). - cactus_override_toil_options() exports CACTUS_FAST_WALLTIME so it reaches workers. - RoundedJob / ChildTreeJob now forward walltime to Toil. Application: - Wired add_cactus_toil_options(parser) into every tool's main(). - Tagged the small coordination jobs' scheduling call sites with walltime=cactus_fast_walltime() (the FAST set plus a hand-picked subset of the ambiguous ones; compute-heavy jobs left on --defaultWalltime). Docs (doc/progressive.md, doc/pangenome.md): recommend --defaultWalltime (+ the new --fastWalltime) instead of --slurmTime, which is now framed as a global override. IMPORTANT: toil-requirement.txt is temporarily pinned to Toil master because per-job walltime is not in a Toil release yet. Revert to a released toil[aws]==<ver> before merging. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Conflicts were all in call sites the walltime branch had tagged with walltime=cactus_fast_walltime(); resolved by keeping master's version of each call (new kwargs: cons_retain_pages, contig_sizes_id, split_log_id, disk estimates, the check_preprocessed_sequence / add_separate_ref_contigs_job / write_batch_summary jobs) and re-attaching the fast walltime. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019aWQPbf73PRcHZkw9YL2gf
Bumps the Toil master pin from 091ed886 to a9681fe0 ("Add `--doubleTime`
argument", #5576). --doubleTime is the walltime analogue of --doubleMem: a job
Slurm kills for exceeding its walltime is retried with twice the walltime. That
is what makes tight per-job walltime estimates safe -- an occasional
underestimate costs a retry, not the run -- so it is a prerequisite for setting
real times on the rest of the jobs.
Also documents it alongside --doubleMem in the cluster options.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019aWQPbf73PRcHZkw9YL2gf
Slurm picks a job's partition from its time limit, so the standing advice of --slurmTime 100:00:00 drops all of Cactus's jobs -- including the many thousands that finish in seconds -- into the slowest queue. This estimates a walltime per job from its inputs, the way memory is already estimated, so the short jobs can be routed to a short partition. Machinery (src/cactus/shared/common.py): - cactus_walltime(seconds, io_bytes) replaces cactus_fast_walltime(). Call sites pass their best estimate of the job's real runtime; the safety margin (--walltimeFactor, default 2.5), the floor (--minWalltime, 600s) and the ceiling (--maxWalltime) are applied centrally. io_bytes covers jobstore staging, which is what makes an otherwise trivial job slow. Mirrors cactus_clamp_memory, including reaching workers through the environment. - --doubleTime now defaults to true, the way --retryCount is defaulted to 5. It is what makes tight estimates safe: a job Slurm kills for running over is retried with twice the time rather than failing the run. - cactus_slurm_max_walltime() caps every estimate at the longest partition the jobs could land on. Toil raises, rather than falling back to a default, when it cannot find a partition that fits a walltime, so an over-estimate would otherwise kill the run at submission. - --fastWalltime is gone; there is one estimator now rather than a separate knob for the small jobs. Estimates, each fitted to real cluster logs (1.8M timed commands from the VGP 577-way, 291k from four HPRC pangenome runs): - cactus_consolidated: 800 * disk_gb**0.95, scaled up below 24 cores. Fitted to 576 alignments; every one of them lands at 65-75% of its request. - lastz: a per-divergence-class table in the config (<blast><lastzWalltime>), scaled by the actual chunk sizes. 827k runs: the closest class is p99 331s against 4324s for the most diverged, which is exactly the spread that a single global time throws away. - Red: 933 s/Gb of fasta (625 runs, p99 2799 s/Gb, divided by the 3x speedup). - hal2chains batches: the existing chain_pair_cost, calibrated at 1.78e-5 s per unit. Per pair it is noisy, but a batch sums many pairs -- replaying the LPT assignment, predicted batch time is within 1.02-1.55x of the truth. - hal2maf batches: per chunk per genome, plus one slow chunk for the tail that dividing by the core count ignores. Covers 98% of the 824 VGP batches. - taffy index/coverage/view: 3000s + 200 s/GB of MAF, tapered below 4.5 GB. Cost per GB is 3x higher on a small MAF than a large one, so a slope-only model asks far too much of the big ones. Also fixes the first pass's tagging: jobs that export, merge or import a large file are not the coordination tier, whatever they compute. hal2maf_ranges and the hal2maf batches import the whole HAL; the merged-MAF, pangenome and panpatch exports write hundreds of GB; save_preprocessed_files copies every preprocessed sequence. These now carry io_bytes from a real size in scope at the call site. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019aWQPbf73PRcHZkw9YL2gf
…etry Toil raises, rather than falling back, when no partition can fit a job's walltime -- and it does so on the batch system's own thread, so the whole workflow goes down, not just the job. Cactus now turns --doubleTime on by default, which retries a job Slurm killed for running over with twice the walltime. Together those meant an estimate clamped to the longest partition's limit would turn its own first retry into exactly that crash. So the automatic ceiling is now half the longest partition rather than all of it, leaving room for one doubling. A job that genuinely needs more than half still ends up in the long partition, it just spends one attempt getting there. The reserve is skipped with --doubleTime false, where there is nothing to retry into. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019aWQPbf73PRcHZkw9YL2gf
Extends the per-job estimates to the ~160 scheduling sites the first pass left on --defaultWalltime. Coverage is now 283 of 284 job-function sites and all 13 RoundedJob subclasses, against 126 and 1 before. Each rate is fitted to the mined cluster logs and carries its measurement in the comment beside it, in the style of the memory estimates already in these files. Where a job's cost has no size signal it says so and uses a constant. Two things this turned up that are worth naming: - cactus_consolidated has two regimes, selected by <bar bandingLimit> (what --maxLen sets), and they differ in shape rather than scale: progressive is 214 * disk_gb**0.906 (r=0.64), pangenome is 2522 * disk_gb**0.251 (r=0.26). Banding to 10 kb bounds bar's work per column, so cost follows reference columns rather than sequence volume -- adding haplotypes moves `disk` a great deal and the runtime hardly at all. Applying the progressive fit to a pangenome chromosome overshoots 13-17x at the median. Both coefficients are divided by 4: the 2x speedup already landed, and a further 2x expected but not yet measured. If that second 2x underdelivers, 5 of 576 progressive and 3 of 50 pangenome alignments run over, none by more than 1.5x, so one --doubleTime retry rescues each. - filter_paf read .size off a promise on the gzipped-PAF path, for disk and memory, both predating this branch -- a gzipped PAF with --pangenome raised AttributeError before it scheduled anything. All three now size from a paf_size captured before the rebinding. An adversarial pass over each module's diff found, and this fixes: a 577-genome CactusPreprocessor fan-out asking 46 h to do nothing but addChild (Toil chains a *lone* successor, so only a one-genome fan-out covers its child's work); align_toil's additive floor giving an evolver-sized test PAF 10 h; FasTAN borrowing Red's rate along with the 3x speedup divisor it has no claim to; hal2vg fitted to HAL sizes recovered from a clamped memory request, which manufactures a superlinear trend the data does not have; maf2bigmaf reusing a peak-disk multiplier as a compression ratio; hal2seqfile substituting the whole HAL for one genome of unknown length; and several call sites stacking their own 3-4x margin on top of --walltimeFactor, which is that factor's job. The gzip rate and the consolidated core scaling are now shared rather than copied into each module that needs them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019aWQPbf73PRcHZkw9YL2gf
The worst of it, in cactus-prepare-toil: cactusPrepare() never called cactus_override_toil_options(), the only thing that exports CACTUS_MAX_WALLTIME. Nothing else in the module has needed it before, but the plan now computes walltimes, so every one of them was running with no ceiling on what it could ask a Slurm partition for -- and Toil raises, on the batch system's own thread rather than the job's, when it cannot find a partition that fits. It also meant --walltimeFactor, --minWalltime and --maxWalltime, which this parser advertises, were silently ignored, along with the retryCount and realTimeLogging defaults the rest of Cactus gets. Compounding it: in toil mode the parser is Toil's own, whose --defaultCores defaults to 1, and main() copies that into blastCores/alignCores/preprocessCores. So the default `cactus-prepare-toil --batchSystem slurm` asked 15,000,000 s (173 days) per blast job, unclamped. The estimates now anchor on the p50/p90 rather than the p99 -- cactus_walltime multiplies by 2.5 and --doubleTime takes the tail -- and align and preprocess scale by cores through the same cons_core_scale the consolidated estimator uses, rather than being applied unscaled at their 64-core measurements. A 1-core blast is still a 13-day job; that is honest, and the clamp now bounds what it asks for. Also: guard vcfwave's new per-core divisor against --vcfwaveCores 0; raise merge_pafs to 4x its merged size, since catFiles moves the bytes again locally on top of the jobstore round trip; and give minigraph_map_one a real tail allowance (1500 left 17% over the observed max across 11,390 invocations, which is not one). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019aWQPbf73PRcHZkw9YL2gf
…sor job PreprocessSequence.__init__ takes (prepOptions, inSequenceID, chunksToCompute) and already derives its own size from inSequenceID.size on its first line. The inSequenceSize parameter belongs to BatchPreprocessor, further down the same file; I misread one for the other and passed it to the wrong constructor. The result was TypeError: PreprocessSequence.__init__() got an unexpected keyword argument 'inSequenceSize' on every preprocessing job, which failed 16 of 28 evolver tests -- everything that preprocesses a sequence, plus everything downstream of it, since the missing outputs then surfaced as FileNotFoundError. CI job 116490. Neither compileall nor importing the module catches a wrong keyword at a call site, which is why this got through. Every one of the 295 job-scheduling calls in the tree has now been bound against its target's signature with inspect.Signature.bind_partial, and this was the only mismatch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019aWQPbf73PRcHZkw9YL2gf
minigraph sometimes cannot align a haplotype's inverted copy of a region back to the reference copy and keeps it as an alt node. Downstream that is an insertion, not an inversion: cactus-align cannot merge two separate backbone nodes, and vcfbub flattens the giant allele, so the inversion never reaches the VCF. The new rgfa-collapse (cactus-gfa-tools) finds these and rewires them into proper inversion edges through the reference. minigraph's mapper reuses such edges rather than re-creating the redundant node, so this also keeps the redundancy from recurring on later construction. Off by default. Enable with graphmap/collapseInversions="1"; graphmap/ collapseOptions passes flags through (chunk size, minimum block, identity). Runs as a follow-on to minigraph construction, so it is per chromosome under --batch and whole-genome otherwise, matching however minigraph itself was run. Sites come from vg's snarl decomposition, so the step shells out to vg snarls / vg view; vg is already a pipeline dependency. Both graphs are exported while this is still experimental: the collapsed one takes the usual path, since it is what the rest of the pipeline maps to, and the input is written alongside as *.uncollapsed.gfa[.gz]. The per-site report goes to *.collapse.tsv so calls can be inspected without rerunning. Tested on the 6-genome yeast graph: 3 inversions / 31,484 bp found, 12,848,795 -> 12,818,780 bp, no dangling links. A full graphmap/split/align/join comparison on the same data showed graph length down 26,402 bp with path count and total path length unchanged, i.e. redundancy removed without clipping anything. The cactus-gfa-tools pin still needs bumping once rgfa-collapse is pushed there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
b37287b1 predates the tool, so downloadPangenomeTools would build everything except rgfa-collapse and the mv added in the previous commit would fail. 96d294ff is origin/inversion-collapse, which adds it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
Picks up three commits on inversion-collapse: the forward-duplication case wired like the inverted one, the non-reference -A/--alt-rounds pass (off by default), and the fix that takes a collapsed allele's entry and exit from the graph edges rather than from traversal order. The last one matters for correctness. Validated on a 12-haplotype chr15 graph through all four cactus stages: against the uncollapsed arm the collapsed graph loses 2,981 nodes, 3,907 edges and 395,002 bp, no haplotype loses a base, path fragments stay at 31, and HG03050.2 gains 233,803 bp. The previous pin lost NA18948.2 69,723 bp to an extra path fragment on the same input. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
Reporting and plumbing only -- rgfa-collapse's output is byte-identical to the pinned version on the validated 12-haplotype chr15 graph. Picks up: -N actually reaching the reference pass (it was hardcoded to 50), -P/--mm-p and -X/--mm-extra for the minimap2 settings that were hardcoded, refusals counted per orientation and in bp, and -R/--call-report writing one row per call before merging. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
The job split mgCores as -j cores/2 -t 2, so 64 cores meant 32 concurrent minimap2 processes with 2 threads each. That is the wrong shape for this workload: runtime is dominated by a handful of enormous single alignments -- individual minimap2 calls ran over three hours at -t 2 on the CHM13 chr9 satellite block -- and a single long alignment cannot be spread across concurrent chunks, so a wide -j leaves cores idle waiting on the tail. Measured average utilisation at -j 4 -t 2 was 3.25 of the 8 cores requested. A chunk holds a few hundred query sequences, so minimap2 keeps a high -t busy on its own. Use -j min(8, cores/8) -t cores/j: 64 cores becomes -j 8 -t 8, keeping some chunk-level concurrency rather than one very wide invocation. minimap2 runs about two threads more than -t asks for, so this deliberately leaves headroom instead of saturating. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
collapseInversions stays "0". The collapse is validated at whole-genome scale on four graphs
-- hprc-v2.1 grch38 and chm13, hprc-v2.1-mc-grch38-eval, hprc-sep1-mc-grch38-eval -- but only
end to end (graphmap -> split -> align -> join, with a per-haplotype path-bp and fragment-count
comparison) on 12 haplotypes of chr15. Flipping the default waits on that evidence at scale.
collapseOptions gains "-D -A 3 -x asm20 -M 1.05":
-D also collapse forward duplicates, not just inversions
-A 3 collapse traversals matching no reference onto a non-reference representative;
rounds converge (88 / 10 / 6 traversals on grch38), so 3 is measured, not arbitrary
-x asm20 minimap2 preset; asm5 finds fewer of these at high divergence
-M 1.05 refuse a call that would delete more than 1.05x the reference that haplotype has
not already consumed at that locus. Without it a tandem array is collapsed once
per copy: at chr19:37.27 Mb one haplotype received 31 individually-conserving calls
against a reference window that is itself ~20 copies of a 2,525 bp unit.
The pin moves a50f627 -> 2851444, which is required, not incidental: -M does not exist at
a50f627, so the option string above would fail outright, and a50f627 also predates the fix that
makes results independent of thread scheduling (calls were appended in completion order and the
rewiring loop mutates the graph as it walks them; whole-genome grch38 twice gave a byte-identical
SET of 2705 calls with 287 flipped between repaired and refused-tandem).
Measured with these options, 64 cores: grch38 5:55 / 7.6 GB, chm13 44:44 / 12.0 GB, plus 8-11
min of vg snarls. All output graphs pass duplicate=0 selfloop=0 dangling=0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
Two conflicts, both resolved by composing the two sides rather than choosing one. build-tools/downloadPangenomeTools: master pinned cactus-gfa-tools to a32588c (gaffilter -t and its options), this branch to 2851444 (rgfa-collapse, including the fix that makes results independent of thread scheduling). Neither contained the other, so either choice would have silently dropped the other's work -- taking master's loses -M, which the collapseOptions in this branch require, and taking this branch's loses gaffilter's -t/-Q/-e/--close-holes. Pinned instead to 0e3c952, a merge of the two lines made in cactus-gfa-tools for this purpose; its gaffilter.t passes (39 tests) and rgfa-collapse reproduces its chr19 baseline byte for byte. cactus_minigraph.py: master added a --mgSplitWholeGenomeRef guard around the GFA export, because in that mode the graph is still whole-genome and only reaches this path after the post-mapping prune. This branch added a collapsed/uncollapsed export branch. Master's guard now wraps that branch, so with mgSplitWholeGenomeRef nothing is exported here either way, and without it the collapse export happens as before. The interaction of the two features is untested -- both are off by default, so nothing ships in that state. The minigraph_construct_in_batches call takes master's signature (construct_seq_id_map, whole_genome_ref) with the collapse follow-on appended; the 5-tuple return and its unpack are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
Three problems, all of which meant the feature did nothing useful outside standalone cactus-minigraph. 1. The collapsed graph was never consumed. cactus-pangenome takes the graph from minigraph_construct_workflow's slots 0 and 1 and hands it to graphmap and rgfa-split; the collapse result sat in slots 2 and 3, read by nobody. With collapseInversions on, the pipeline ran rgfa-collapse, paid for it, and then mapped against the uncollapsed graph. The collapsed graph now REPLACES the constructed one in both slots, so every consumer -- graphmap, rgfa-split, the join, and the per-chromosome --mgSplit path, which gets it through val[0]/val[1] with no change at the call site -- sees the graph the collapse produced. Slots 2 and 3 now carry the pre-collapse graph and the per-call report, as side artifacts. 2. --lastTrain was broken, with the feature off. The workflow used to return (gfa, pansn_gfa, train); the collapse work inserted two elements ahead of train_id without moving its consumers, so cactus_pangenome.py read the collapse slot where it wanted the LAST scoring model. last_scores_id is None unless --scoresFile was passed, so with collapse off it silently stayed None and the .train model was never exported nor passed to the align stage; with collapse on, a GFA file id went where a scoring model was expected. Both consumers (the non-batch rv(2) and the --mgSplit val[2]) now read index 4. This was a regression against master that shipped in the default config. 3. The artifacts were never written in this path. The collapse export lived in export_minigraph_construct_output, which only standalone cactus-minigraph calls; cactus-pangenome exports through export_minigraph_wrapper. Both now go through export_collapse_artifacts, so <out>.sv.uncollapsed.gfa.gz and <out>.sv.collapse.tsv appear either way. The collapse runs on the PanSN graph and is renamed back with minigraph_gfa_from_pansn, because it finds sites with `vg snarls -n -P <reference sample>` and cactus names (id=EVENT|CONTIG) carry no PanSN sample for -P to match -- given the wrong -P, vg skips every snarl as a non-reference boundary and the tool reports nothing at all. It is skipped on the --mgSplit reference-only first pass, which has no non-reference nodes to collapse, and runs on each all-sample chromosome graph. testYeastPangenomeLocal now turns it on, via a config copy the way _run_evolver does, since there is no command-line flag for it -- note --collapse is an unrelated option about minimap2 self-alignments. _check_collapse_output asserts the collapsed graph, the kept pre-collapse graph and the report all exist and that vg validate passes on the result, which holds whether or not yeast contains a collapsible allele: what it pins down is that the integration runs at all. Static checks only -- the pipeline itself has not been run against this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
…found
Enabling collapseInversions on the --mgSplit tests immediately found a break the non-mgSplit test
could not see: rgfa-collapse named split pieces s537.1, and merge_sv_gfa renumbers segments by
adding an integer offset to the digits after the 's', so int("537.1") raised and the whole join
died. The pieces now take numeric ids (cactus-gfa-tools 2dea492, behaviour otherwise identical:
chr19 at -M 0 repairs the same 3 inversions and 1116 duplications over an identical graph), and
the pin moves 0e3c952 -> 2dea492.
Those names had survived the entire non-mgSplit pipeline -- graphmap, align, join, vg validate,
the VCF and the HAL -- so only the path that merges per-chromosome graphs noticed.
testYeastPangenomeMgSplitLocal is new: no yeast test covered plain --mgSplit at all, only
primates did, and the two mgSplit modes take different routes through the export (with
--mgSplitWholeGenomeRef the graph only reaches its final path after the prune). Both now run
with collapseInversions on, and both pass, as does the non-split test:
testYeastPangenomeLocal 652s 2 inversion alleles repaired
testYeastPangenomeMgSplitLocal 1271s chrXIV and chrXV, one allele each
testYeastPangenomeSplitLocal 1903s 13/13 chromosomes collapsed
Both confirm the two behaviours the --mgSplit wiring is supposed to have: the collapse is skipped
on the reference-only first pass, and runs on each all-sample chromosome graph.
One thing the contrast makes visible and worth knowing before enabling this in that mode: with
--mgSplitWholeGenomeRef the whole-genome reference is substituted into every per-chromosome graph,
so an inversion in it is repaired once per chromosome -- ten of thirteen here found the same one --
and the prune discards the surplus. Plain --mgSplit repairs each inversion once, on the
chromosome that owns it, and the total matches what the non-split run finds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
Two conflicts, both composed rather than chosen between. cactus_minigraph.py: master's --inGFA work added seed_gfa_id/graph_names to the minigraph_construct_in_batches call and in_gfa_id to the batch workflow; this branch added the collapse follow-on after the former and corrected the tuple comment on the latter. Kept both. The collapse's PanSN rename now takes graph_names rather than the keys of seq_id_map, for the reason master's own comment gives at its definition: the rename has to resolve every SN tag in the finished graph, which on the extend path is more genomes than minigraph is handed. graph_names is captured before seq_id_map is trimmed to the genomes still to construct, so passing seq_id_map would have dropped the seed graph's events and minigraph_gfa_from_pansn would have asserted on the first one it met -- with --inGFA and collapseInversions both on. evolverTest.py: master added an `extend` parameter and testYeastPangenomeExtendLocal to _run_yeast_pangenome, this branch a `collapseInversions` parameter; both are now parameters and both call sites are intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
22 commits, bringing --inGFA/--inGAF pangenome extension, the mgSplit whole-genome reference pass, the GAF overlap trim filter, hal2vg 1.2.3 and the faster minigraph fork. 28 conflicts, all where master added an argument to a call this branch had tagged with a walltime; resolved by keeping master's version and re-attaching the walltime. Two are worth noting because the resolution is not a straight combination: - merge_pafs_sized (new on master) resolves the merge inputs' promised sizes at runtime, which is strictly better than predicting the merged size from fasta bytes the way this branch was doing. The walltime moved inside it, so it is now computed from the real sizes and the estimate it replaces is gone. - GFA_RENAME_SECS_PER_GB and RAW_BYTES_PER_GZ_BYTE moved from cactus_graphmap to cactus_minigraph. cactus_minigraph now needs them, and graphmap already imports from minigraph, so leaving them where they were would have made the import circular. They sit next to minigraph_gfa_from_pansn, the job the rename figure actually describes. Master's 13 new scheduling sites are tagged, so coverage stays complete: 297 of 297 job-function sites and 13 of 13 RoundedJob subclasses, with every one of the 308 scheduling calls binding cleanly against its target's signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019aWQPbf73PRcHZkw9YL2gf
… combination that cannot work Three fixes, each found by running a yeast test rather than by reading, and each a silent type error that git could not flag as a conflict: 1. minigraph_gfa_from_pansn now returns (gfa id, genome set), not a bare id. Passing rename_job.rv() on handed the whole tuple to graphmap, which died with "'tuple' object has no attribute 'size'". Now rv(0). 2. minigraph_construct_workflow has four return paths and master added two of them; both handed back three elements while the constructing path returns five, so cactus_pangenome's rv(3)/rv(4) read off the end. Toil surfaces that as "IndexError: tuple index out of range" from _fulfillPromises, nowhere near the cause. All four paths now return the same five slots; the resume path has nothing to collapse, so its two collapse slots are None. 3. collapseInversions and --inGAF cannot be combined at all. The collapse splits nodes at alignment block boundaries and rewires the edges, so a GAF made against the pre-collapse graph no longer tiles it and gaf2unstable fails its ui_len assertion. master's new check_reusable_gaf catches it, but diagnoses --mgSplit, which is the wrong trail. The combination is now refused at startup with the real cause and the three ways out (drop --inGAF, add --remap, or turn the collapse off). getattr rather than attribute access because cactus-panpatch shares that function and defines neither option. Verified on yeast, each a full cactus-pangenome run with collapseInversions on: non-mgSplit passed 650s plain --mgSplit passed 1255s --mgSplitWholeGenomeRef passed 1765s --inGFA (no --inGAF) passed 884s 2 inversion alleles repaired --inGFA with --inGAF refused at startup, 0 failed jobs, right message The pinned tool versions were refreshed first (downloadPangenomeTools, hal2vg v1.2.3): a stale bin/minigraph without --par-align and a stale bin/rgfa-collapse without -M had each been masquerading as a code failure. Still not exercised by any test: the "nothing left to construct" resume path, whose five-slot return above is by inspection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YWGkqE3ND862JF8QRZERV
Brings the rgfa inversion collapse (--collapseInversions) and the five-slot minigraph_construct_run return. Three conflicts, all the usual shape -- their new arguments against this branch's walltime -- resolved by keeping theirs and re-attaching the walltime. Their return-tuple fix wins outright: truncating it to three slots left cactus_pangenome reading rv(3)/rv(4) off the end. The new collapse_inversions job gets a walltime keyed on the thing that actually sets its runtime. Their own note records individual minimap2 calls running over three hours at -t 2 on CHM13 chr9, and says a wide -j cannot spread one alignment across chunks -- so the cost is the slowest single alignment, not the total work over the core count, and dividing by cores would have been wrong in the same way it was for the hal2maf batches. The estimate is therefore keyed on the threads one alignment gets, plus a size term for the bulk that does use the full width. collapse_minimap2_threads() is shared between the estimate and the job so the two cannot drift: the job now asks it for the same -j/-t split it was computing inline. A 138 MB chromosome reference asks 8.9 h at 2 cores and 2.4 h at 8 or more, where the split caps threads-per-alignment. Coverage stays complete: 299 of 299 job-function sites, 13 of 13 RoundedJob subclasses, 310 scheduling calls binding cleanly against their signatures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019aWQPbf73PRcHZkw9YL2gf
From the first at-scale Slurm run with walltimes on (HPRC, GRCh38+CHM13, --doubleTime true). It got through preprocessing with no memory kills and no partition errors -- the ceiling clamped to 7 days off a 14-day partition exactly as intended -- but 60 of its 519 sanitize_fasta_header jobs were killed on time. Every one of them then succeeded on the --doubleTime retry, and none needed a second doubling, so the requirement sat between the ~620 s they asked for and the ~1300 s they were given. The shortfall is staging, not compute: those jobs' own commands measured 68 s and 56 s at the median and 237 s summed at the p99, against a walltime of 620 s. What they were actually doing was moving ~4 GB each through a jobstore with 519 of them on it at once. 100 MiB/s was the first guess for that and it is an order of magnitude out under that load, so it drops to 25. The floor goes to half an hour because a short partition is an hour: every request under that routes identically, so precision below it buys nothing, while being short by a minute costs a whole retry. sanitize_fasta_header now asks 30 minutes against the ~22 it needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019aWQPbf73PRcHZkw9YL2gf
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.
start trying to have cactus assign times to toil jobs, with the hopes of getting small jobs onto faster partitions.
todo: