Skip to content

WIP Minigraph postprocessor - #2006

Open
glennhickey wants to merge 5 commits into
masterfrom
minigraph-cleanup
Open

glennhickey wants to merge 5 commits into
masterfrom
minigraph-cleanup

Conversation

@glennhickey

Copy link
Copy Markdown
Collaborator

This introduces a kind of gfaffix but for minigraph. It goes snarl by snarls and tries to detect under-alignments by remapping traversals to the reference with minimap2. 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.

glennhickey and others added 5 commits September 11, 2026 13:15
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant