WIP Minigraph postprocessor - #2006
Open
glennhickey wants to merge 5 commits into
Open
glennhickey wants to merge 5 commits into
glennhickey wants to merge 5 commits into
Conversation
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
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
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.
This introduces a kind of
gfaffixbut for minigraph. It goes snarl by snarls and tries to detect under-alignments by remapping traversals to the reference withminimap2. Traversals that don't map to the reference map to each other.In practice, this seems to catch real problem areas in both false insertions and inversions-falsely-represented-as-insertions. Pending more tests at scale before merge.