Add Harun's redone aligner as opt-in engine with pymetagraph bindings by adamant-pwn · Pull Request #672 · ratschlab/metagraph

adamant-pwn · 2026-10-21T13:05:43Z

Add Harun's redone alignment engine (graph/alignment_redone) as opt-in code, together with the pymetagraph Python binding that adaptive-sampling experiments use. metagraph align, the server and query --align keep using the existing aligner, so default behaviour does not change.

This replaces the previous contents of this branch. The branch now merges Harun's last align_refactor commit (2d1734717, Nov 2025) into current master, plus Sayan's LabeledSeeder::get_anchors fix. Sayan's experiments ran on exactly this engine. The October 2026 changes are preserved on codex/align_refactor-oct2026 and can come back one at a time where they are worth keeping.

Changes relative to Harun's commit:

SSHash sub-k seeding removed. It depended on an SSHash fork commit that master no longer pins. SSHash graphs now seed like hash graphs.
--separate-header left out. That annotation feature is ported separately on feature/separate-header.
One shared options initialiser. The old and new engines' identical option initialisers now share one template; initialize_redone_aligner_config serves the new engine.
One canonical-graph test skipped for protein, as the existing aligner tests already do.

The Python extension builds only with -DBUILD_PYTHON_BINDINGS=ON. That option compiles position-independent static libraries, uses shared Boost and drops jemalloc. The default static build is unchanged.

Local checks (GCC 16, Release):

DNA unit tests: 3904/3904
Protein unit tests: 3197/3197
Alignment tests: DNA 368/368, Protein 337/337
Compared with a build of sg/readfish-bindings on the same k=31 graph, annotation and 350 simulated reads, per-read results are byte-identical in five of the six binding modes (align, query, align without annotation, without extension, full-k seeds). connect_anchors=False now differs by design: before the fix below it was ignored when chains were extended. 278/350 rows are identical and 342/350 have the same label; the differing rows report shorter spans.

Fixes from a review of this PR (all also present in Harun's commit):

connect_anchors=False was ignored with extension. align_query now passes it on. Chains whose anchors are not connected keep placeholder nodes between anchors; the extender reports those chains unextended instead of crashing on them.
Query::generate_rc reverse-complemented the not-yet-constructed member instead of the local copy. It worked only because the compiler constructs the result in place; the tests now cover reverse-complement queries on BASIC graphs.
query_batch: a read that throws (for example "Failed to connect anchors") is now logged and reported as unmapped instead of failing the batch while other tasks still write into it.
The binding docs no longer claim that metagraph align or the server's /align use this engine.

Known limitations:

Invalid options or missing graph/annotation files make metagraph's config parser and loaders call exit(), which ends the Python process instead of raising.
The engine ignores the alignment work caps (alternative paths, nodes per query character, RAM per alignment). The binding accepts max_alternative_alignments and max_num_nodes_per_seq_char without effect.

Not yet tested: integration tests, Sayan's real data, and installing the binding with pip.

AnnotationKeyMapper came from #683, now merged into master. The new engine maps its annotation rows through it, and this PR only adds the mapper's SSHash shortcut (DBGSSHash::get_base_node).

🤖 Generated with Claude Code

The redone aligner's annotation buffer repeated the node-to-row mapping that AnnotationKeyMapper now provides. Use the mapper, and give it the SSHash shortcut through DBGSSHash::get_base_node, which this branch adds. The mapping is unchanged for basic, canonical, wrapped-primary and SSHash graphs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

build_graph_batch returns primary graphs already wrapped in CanonicalDBG, so test the wrapper it returns, and the primary graph inside it for the rejection case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Building a primary graph for the protein alphabet crashes, as other canonical-graph tests already account for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Keep the swap_dir build directory and take master's v6 build option. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Map oriented nodes to annotation rows in one place Add AnnotationKeyMapper, which returns the node whose annotation row an oriented node uses: the node itself in basic graphs, the base node of a CanonicalDBG, and the canonical k-mer's node in canonical graphs. The aligner's annotation buffer now uses it instead of its own mapping. Unwrapped primary graphs are rejected, as the buffer already assumed. Taken from the annotation-supported traversal integration (#670), where the traversal and both aligners share it, without its SSHash fast path, which needs DBGSSHash::get_base_node from #672. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test primary graphs as the test helper builds them build_graph_batch returns primary graphs already wrapped in CanonicalDBG, so test the wrapper it returns, and the primary graph inside it for the rejection case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test primary graphs only for DNA Building a primary graph for the protein alphabet crashes, as other canonical-graph tests already account for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> # Conflicts: # metagraph/src/graph/annotation_key_mapper.cpp # metagraph/src/graph/annotation_key_mapper.hpp

Query::generate_rc reverse-complemented the member rc_ before it was constructed instead of the local copy. It only worked because the compiler elided the copy into rc_. The extender overload of align_query dropped connect_anchors_in_chain, so connect_anchors=false had no effect when chains were extended. Now that it reaches the extender, chains whose anchors were not connected keep placeholder nodes between anchors, which extension cannot start from; report such chains unextended instead of crashing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

An exception from one read inside a pool task failed the whole batch from the first future, while the remaining tasks still wrote into the result vector. Catch it per read, log it and report the read as unmapped. Also correct the docs: neither `metagraph align` nor the server's /align endpoint uses the alignment_redone engine. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch moved graph_index_to_sshash into DBGSSHash; the duplicate check from #682 still called the free function. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

On a succinct graph with A = TTACGAAA and B = ACGTTGCA, k = 5 and minimum seed length 3, the redone engine labels ACGT and CGTTG as B and TACGA as A, in basic and canonical mode. The existing aligner finds no alignment for ACGT on the basic graph, as its only suffix match is the dummy source k-mer $ACGT, and for CGTTG also reports TTG as an alignment to A that spells ACG. Its test is disabled until that is fixed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Sub-k seeding on a CanonicalDBG read nodes[i] for every seed start, while nodes holds one entry per full k-mer, so starts in the last k - min_seed positions and queries shorter than k read past the end. Treat those positions as unmatched, as the forward check above already does. Correct the sub-k label tests. The existing aligner's partial ACG match for CGTTG is on the reverse strand and A does contain ACG, so its labels are right; its only defect here is the missed ACGT on basic graphs. The redone engine misses TGCA, which ends B, on basic graphs, as sub-k seeds are only extended forward there; that case is a disabled test. Both tests also cover primary graphs, where the out-of-bounds read occurred. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- About 175 lines of commented-out code (debug output and abandoned variants) and two unreachable blocks after an unconditional return in the seeder, including an SSHash gap-scoring copy. - Unused members: chains_per_char, Cigar::extend_clipping and get_num_ref, LabeledSeeder::get_buffer, and Extender::extend's no_bwd/no_fwd parameters, which no caller set. - Commented-out tests: five copies of existing-aligner tests that use options this engine does not have (min_cell_score, the seed complexity filter, forward_and_reverse_complement) are deleted; they still run in test_aligner.cpp. align_long_gap_after_seed now runs and passes. align_low_similarity4 and its primary variant run for minutes, as the engine has no work budget, so they are skipped with that reason. Two commented-out variants are dropped: one expected an alignment that does not cover the query, and with seeds shorter than k the engine does not recover align_insert_long_offset's long insertion. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 3 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic}

The existing-aligner test now requires the full-length alignment to spell the query on either strand, not just to have its length. The redone test no longer requires ACGC to have no alignment: a partial alignment of ACG is legitimate, so any alignment must instead spell part of its label's sequence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The redone copy of spell_path matched align::spell_path with offset 0. Its documentation moves to the shared declaration. The std::hash specialisations for Match, Anchor and Alignment had no users. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

get_sequence_ids was a pure virtual on SequenceGraph for one caller that only uses DBGSSHash, and always returned a single id. It becomes the non-virtual DBGSSHash::get_sequence_id, and seq_index goes. The DBGSSHash::max_index override equalled num_nodes() in both modes a DBGSSHash can have, so it is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The --align-no-connect-anchors and --align-no-extend-chains flags only carried two binding keyword arguments through Config; no command used them. The binding now reads those keywords directly, and Config matches master again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 17 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic}

A stretch of npos nodes does not throw: its characters come from overlapping neighbouring nodes, or are '$' where none covers them. Only an invalid edge throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

max_nodes_per_seq_char now bounds the wavefront search, as in the existing aligner: once max_nodes_per_seq_char times the query length nodes were explored, the search stops at the first entry behind the furthest query position reached, and the best alignment found so far is backtracked. A gap fill stopped this way no longer throws; its chain yields no alignment. align_low_similarity4 and its primary variant run again, port the existing aligner's checks, and finish in under a second. max_num_seeds_per_locus now drops all seeds shorter than k at a query position that has more than that many, as in the existing aligner. The binding no longer accepts max_alternative_alignments, which the engine never used, since it reports only the best alignment. The binding takes the CLI defaults (budget 5, 1000 seeds per locus). On the 350-read binding comparison (k = 31, seeds of 19), the seed cap changes nothing. With a budget of 5 or 10, 2 reads end 7-12 characters earlier, with the same labels; with 20 or more, the output is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 5 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic}

Entries that extend the furthest query position by a deletion stay at that position, so the budget never stopped them. Once over budget, only entries that reach further into the query, or reach the front at the cost of the first entry there, continue. The binding comparison output is unchanged at budgets of 5 and 10. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

- The reverse-complement pass of sub-k seeding on a CanonicalDBG kept traversing mismatching branches and exact ones past length k. Neither yields an anchor there, and the latter trips the length assertion on primary graphs. It now only follows exact matches shorter than k. - Extension asserted that every backtracking start scores at most the best explored score. Entries the node budget leaves unexplored can score higher, so they are skipped instead. - align_low_similarity4 sets the minimum seed length to k, as the existing aligner's version does. The binding comparison output is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

adamant-pwn · 2026-10-29T01:14:58Z

Closing in favour of smaller PRs that ported the fixes from here into the current aligner (#689–#697). Compared on the same data, accuracy was similar and this version was several times slower with about 2× the memory. The full work remains on the align_refactor branch for reference.

hmusta added 30 commits October 26, 2024 15:21

skip extend if end trim can't be filled

8710cf5

minor

878e665

test

7237b95

minor

4d940b4

per anchor

0442497

don't merge anchors after distances have been calculated

d11e6c0

fixes

ebe6dbf

added test

5946b2c

minor

15a4de9

don't extend beyond anchors that have been hit

05031c0

minor

28d9cdb

fixes for multi-node seeds

b61d749

silence messages

9b1f21e

cleanup

0ac0728

change output format

4028016

testing alignment in cli

50f41bf

always terminate at query end

be63707

store partial alignments

6949413

fixes

b41ce64

this works

f92295a

suppress messages

34ade4a

logging

ef4ab71

extra checks

32c08f9

lots of fixes

6e43489

test

64cf90a

fixed

1a361b3

suppress

93064a9

better term condition

14ee597

minor

e6d6783

cut chaining short

30d9a1e

adamant-pwn and others added 6 commits October 28, 2026 11:02

Test primary graphs as the test helper builds them

bf8b00f

build_graph_batch returns primary graphs already wrapped in CanonicalDBG, so test the wrapper it returns, and the primary graph inside it for the rejection case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge the mapper test fix from #683

ed9ebaa

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Test primary graphs only for DNA

13684d8

Building a primary graph for the protein alphabet crashes, as other canonical-graph tests already account for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge the protein test guard from #683

98c3751

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge master: SSHash v6 (#684)

1e3a976

Keep the swap_dir build directory and take master's v6 build option. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

adamant-pwn and others added 8 commits October 28, 2026 15:41

Merge master: AnnotationKeyMapper (#683)

65d93e9

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> # Conflicts: # metagraph/src/graph/annotation_key_mapper.cpp # metagraph/src/graph/annotation_key_mapper.hpp

Merge master: SSHash input validation (#682)

641ee9d

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Qualify the SSHash index conversion in the duplicate check

305a060

This branch moved graph_index_to_sshash into DBGSSHash; the duplicate check from #682 still called the free function. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cubic-dev-ai Bot reviewed Oct 28, 2026

View reviewed changes

Comment thread metagraph/tests/annotation/test_aligner_labeled.cpp Outdated

Comment thread metagraph/tests/annotation/test_aligner_labeled_redone.cpp

adamant-pwn and others added 4 commits October 28, 2026 17:58

cubic-dev-ai Bot reviewed Oct 28, 2026

View reviewed changes

Comment thread metagraph/src/graph/alignment/alignment.hpp Outdated

adamant-pwn and others added 2 commits October 28, 2026 18:03

Describe how spell_path spells npos nodes

fe87445

A stretch of npos nodes does not throw: its characters come from overlapping neighbouring nodes, or are '$' where none covers them. Only an invalid edge throws. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cubic-dev-ai Bot reviewed Oct 28, 2026

View reviewed changes

Comment thread metagraph/src/graph/alignment_redone/aln_seeder.cpp

adamant-pwn and others added 2 commits October 28, 2026 18:24

adamant-pwn marked this pull request as draft October 28, 2026 18:23

adamant-pwn closed this Oct 29, 2026

adamant-pwn mentioned this pull request Oct 29, 2026

Add experimental pymetagraph bindings for in-process queries #698

Open

Navigation Menu

Repository navigation

Add Harun's redone aligner as opt-in engine with pymetagraph bindings - #672

Add Harun's redone aligner as opt-in engine with pymetagraph bindings#672
adamant-pwn wants to merge 320 commits into
masterfrom
align_refactor

adamant-pwn commented Oct 21, 2026 •

edited

Loading

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

adamant-pwn commented Oct 29, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

adamant-pwn commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adamant-pwn commented Oct 29, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adamant-pwn commented Oct 21, 2026 •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading

cubic-dev-ai Bot left a comment •

edited

Loading