Repository navigation
Reject --align-chain up front where seed chaining cannot work - #693
Merged
Merged
Conversation
Seed chaining needs seed coordinates, which the labeled aligner has only with an annotation with coordinates on a BASIC-mode graph (the annotation buffer disables coordinates on CANONICAL and PRIMARY graphs). Otherwise, align_both_directions logged "Skipping seed chaining" and then called exit(1) from a worker thread, so align printed nothing for any read. align_to_graph now checks these requirements after loading the graph and the annotation and fails with an error saying what --align-chain needs. The message in the aligner no longer claims to skip chaining. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Problem
metagraph align --align-chainprinted nothing for any read whenever seed chaining could not run.Seed chaining needs seed coordinates. The labeled aligner has them only with an annotation with coordinates, and only on a BASIC-mode graph:
AnnotationBufferdisables coordinates on CANONICAL and PRIMARY graphs. In every other case,align_both_directions:exit(1)from a worker thread.So the run aborted with no output, and the message claimed the opposite of what happened. The broken cases:
Fix
align_to_graphchecks the requirements right after loading the graph and annotation. If they aren't met, it fails with:--align-chain requires a BASIC-mode graph and an annotation with coordinates (e.g., annotate --coordinates, then transform_anno --anno-type column_coord).AnnotationBufferfor those modes. That is out of scope.Test
Integration test
TestDNAAlign.test_align_chain_requires_coordinates:column_coordcolumn_coordIt passes locally (filtered run only).
Before / after
Setup: 24-genome k=31 graph, 30 ONT reads.
8b77fc8f9column_coordannotationNo effect on normal alignment.
metagraph align -p 8, CANONICAL, 945 long reads, 3 interleaved repeats each, median wall, master → this PR:Spreads overlap on all three; peak RSS is the same; output checksums are identical in all 18 runs.
Related observation, not addressed here: on BASIC graphs with a coordinate annotation, labeled alignment of reads with indels mostly fails, with or without
--align-chain. ONT 400 bp at 3% error: 23/100 aligned, vs. 97/100 with the plain column annotation. Illumina-like reads with substitutions only are unaffected (95/100 either way). The coordinate-consistency check inLabeledExtenderprobably requires exact coordinate distances, so indels break it.🤖 Generated with Claude Code
Summary by cubic
Fixes
metagraph align --align-chainso unsupported configurations fail up front with a clear message instead of printing no reads and aborting with a misleading error.Seed chaining needs seed coordinates, and the labeled aligner only has them with a coordinate annotation on a BASIC-mode graph. CANONICAL and PRIMARY graphs disable coordinates, so
--align-chainpreviously logged "Skipping seed chaining" and then exited from a worker thread.align_to_graphnow validates the graph mode and annotation type right after loading and returns--align-chain requires a BASIC-mode graph and an annotation with coordinates (...). The message in the aligner no longer claims it is skipping chaining. Adds an integration test covering the rejected and accepted cases.Written for commit 29083db. Summary will update on new commits.