STAR-compat batch (--alignEndsType, --outSAMorder, chimeric/transcriptome) + PGO CI fix - #145
Merged
Conversation
Psy-Fer
enabled auto-merge (squash)
July 28, 2026 13:04
This was referenced Jul 28, 2026
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 28, 2026
…ead-length mismatch cap Adds the flags this theme needs that scverse#145 did not bring: `alignEndsProtrude`, `alignSoftClipAtReferenceEnds`, `alignInsertionFlush`, `alignTranscriptsPerReadNmax`, `outFilterMismatchNoverReadLmax`, `seedNoneLociPerWindow`, `seedSplitMin`. Off-menu values are rejected loudly rather than silently ignored. `--alignEndsType` and its `ext[mate][end]` matrix come from scverse#145; this branch builds on that rather than duplicating it. The scorer gains only the three fields scverse#145 does not have: `flush_right`, `soft_clip_at_reference_ends` and `p_mm_max_read`. Two behaviours land here: - `--alignInsertionFlush Right` now works. The insertion-placement scan accepts ties as well as strict improvements, then walks the insertion further right for as long as the read keeps matching, rejecting when it runs out of read on the B side (STAR -1000009). `None`, the default, is bit-identical to before. - `--outFilterMismatchNoverReadLmax` is enforced. STAR's `outFilterMismatchNmaxTotal` is the tightest of three caps: absolute, a fraction of the mapped length, and a fraction of the read length. Only the first two were applied, so the flag had nothing to bind on. Now in `AlignmentScorer::mismatch_nmax_total`. Note for a follow-up: the pre-existing `--clipAdapterType` validation sits inside the `solo_enabled()` branch of `try_parse_from`, so it only fires for STARsolo runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 28, 2026
An alignment may no longer be soft-clipped past the end of its chromosome when `--alignSoftClipAtReferenceEnds No` is given. `Yes`, the default, leaves behaviour unchanged. Sits directly after the end-to-end boundary check from scverse#145: both are "this extension may not run off the reference" rules and share the extension results. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 28, 2026
scverse#145 handles the same case with an explicit EXTEND_TO_END_KILL sentinel checked before the score clamp, so there is no divergence left to record. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 28, 2026
…it now owns scverse#145 landed `--outSAMorder` and `--alignEndsType` while this branch was open. The duplicate `--outSAMorder` declaration and its validation are removed, along with the test that covered them, and `alignEndsType` moves out of `NOT_YET_ACCEPTED` since it is now implemented upstream. The parameter-surface test needed no other change, which is the point of it: it reports the new number by itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 28, 2026
`extend_alignment` gained an `extend_to_end` argument when --alignEndsType landed. The two long-read chaining call sites pass `false`: they score a seed's own extension potential during chaining, and --alignEndsType applies at the read ends, not there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 29, 2026
…ead-length mismatch cap Adds the flags this theme needs that scverse#145 did not bring: `alignEndsProtrude`, `alignSoftClipAtReferenceEnds`, `alignInsertionFlush`, `alignTranscriptsPerReadNmax`, `outFilterMismatchNoverReadLmax`, `seedNoneLociPerWindow`, `seedSplitMin`. Off-menu values are rejected loudly rather than silently ignored. `--alignEndsType` and its `ext[mate][end]` matrix come from scverse#145; this branch builds on that rather than duplicating it. The scorer gains only the three fields scverse#145 does not have: `flush_right`, `soft_clip_at_reference_ends` and `p_mm_max_read`. Two behaviours land here: - `--alignInsertionFlush Right` now works. The insertion-placement scan accepts ties as well as strict improvements, then walks the insertion further right for as long as the read keeps matching, rejecting when it runs out of read on the B side (STAR -1000009). `None`, the default, is bit-identical to before. - `--outFilterMismatchNoverReadLmax` is enforced. STAR's `outFilterMismatchNmaxTotal` is the tightest of three caps: absolute, a fraction of the mapped length, and a fraction of the read length. Only the first two were applied, so the flag had nothing to bind on. Now in `AlignmentScorer::mismatch_nmax_total`. Note for a follow-up: the pre-existing `--clipAdapterType` validation sits inside the `solo_enabled()` branch of `try_parse_from`, so it only fires for STARsolo runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 29, 2026
An alignment may no longer be soft-clipped past the end of its chromosome when `--alignSoftClipAtReferenceEnds No` is given. `Yes`, the default, leaves behaviour unchanged. Sits directly after the end-to-end boundary check from scverse#145: both are "this extension may not run off the reference" rules and share the extension results. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 29, 2026
scverse#145 handles the same case with an explicit EXTEND_TO_END_KILL sentinel checked before the score clamp, so there is no divergence left to record. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 29, 2026
…it now owns scverse#145 landed `--outSAMorder` and `--alignEndsType` while this branch was open. The duplicate `--outSAMorder` declaration and its validation are removed, along with the test that covered them, and `alignEndsType` moves out of `NOT_YET_ACCEPTED` since it is now implemented upstream. The parameter-surface test needed no other change, which is the point of it: it reports the new number by itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE
added a commit
to BenjaminDEMAILLE/rustar-aligner
that referenced
this pull request
Jul 29, 2026
`extend_alignment` gained an `extend_to_end` argument when --alignEndsType landed. The two long-read chaining call sites pass `false`: they score a seed's own extension potential during chaining, and --alignEndsType applies at the read ends, not there. Co-Authored-By: Claude Opus 5 (1M context) <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.
STAR-compat batch (
--alignEndsType,--outSAMorder, chimeric/transcriptome) + PGO CI fixWorks through five STAR-faithfulness items surfaced in an older deep-dive review, plus fixes the flaky
linux-x86_64-v4release build. Two logically independent changesets on one branch — split into two commits if you'd rather land the CI fix on its own.Gate (whole branch): 560 lib + 26 integration = 586 tests pass ·
clippy --all-targets0 warnings ·fmtclean. Yeast SE Local 8788/8926 and PE 8390 both-mapped / 0 half-mapped — identical to baseline.Part 1 — STAR-compatibility batch
1.
--alignEndsType(read-end extension mode)Implements
Local(default),EndToEnd,Extend5pOfRead1,Extend5pOfReads12,Extend3pOfRead1— STAR'salignEndsType.ext[iMate][iEnd]boolean matrix (Parameters.cpp,extendAlign.cpp,stitchWindowAligns.cpp).extend_alignmentgains anextend_to_endbranch that forces extension over the full remaining read (no max-score tracking, no mismatch-limit break; chromosome-boundary hit kills the transcript), matching STAR'sextendToEndpath.finalize_transcriptderives per-end flags fromext[imate][iEnd]. Because it runs per-mate-slice,original_is_reverseequals STAR's(Str != iMate), so the mapping reduces toext[imate][is_reverse](start) /ext[imate][!is_reverse](end). Internal stitch extensions stay Local; forcing is applied only at the read-end extension (additive scoring makes deferring tofinalizeequivalent to STAR's in-stitch forcing).Validation: Local default byte-identical to baseline (SE 8788/8926, PE 8390/0).
--alignEndsType EndToEndvs STAR--alignEndsType EndToEndon 10k yeast: 98.4% position agreement (7924/8054), rustar 8821 vs STAR 8817 mapped — at parity with Local's tie character. +2 unit tests (test_extend_to_end_*).2.
--outSAMorder--outSAMorder(Paired|PairedKeepInputOrder). rustar's align pipeline already emits records in exact input (FASTQ) order regardless of thread count (verified empirically on a 4-thread run — identical ordering). The flag is now accepted (both values, effectively a no-op) for STAR/pipeline compatibility; unknown values are rejected. +3 param tests.3. PE chimeric
diffMatesgap relaxationThe specific "gap check wrongly filters inter-mate chimeras" claim doesn't hold under rustar's architecture:
detect_chimeric_oldruns strictly per-mate, and inter-mate is handled bydetect_inter_mate_chimeric, which imposes no inter-mate read-gap. Still made the function STAR-faithful:detect_chimeric_old_implgains an optionalmate_boundaryand waives the read-gap across it (STARReadAlign_chimericDetectionOld.cpp:67-71). The publicdetect_chimeric_oldand all per-mate callers passNone— zero behavior change. +1 unit test asserting the gap is enforced without a boundary and waived with one.4. Chimeric score-drop
read_len(verify)Verified self-consistent, no fix needed: the score-drop gate uses the length of the read passed in, so it scales correctly for both modes — per-mate length for rustar's PE pools (an intra-mate chimera spans one mate) and combined length for a combined read (matching STAR's
readLength[0]+readLength[1]). Added a clarifying comment.5. Multi-exon minus-strand transcriptome projection (test gap)
Added the missing regression test for a 2-exon reverse-strand transcript projection (
align_to_one_transcripttr_strand==2branch): exon-order reversal, t-space/read-coord inversion relative to transcript/read length, and N-op stripping. Passes first try — the projection code was already correct.Part 2 — CI fix: flaky
linux-x86_64-v4(and preventiveneoverse-v1)Symptom: the
linux-x86_64-v4release job intermittently died withIllegal instruction (core dumped)/ exit 132 in the PGO training run; re-running sometimes passed.Cause: the PGO instrumented binary was built at the shipped ISA (
-Ctarget-cpu=x86-64-v4, AVX-512) and then executed to collect the profile. GitHub'subuntu-latestrunners don't reliably have AVX-512, so on a non-AVX-512 runner the training run hit an AVX-512 instruction and SIGILL'd — flaky by which runner the job landed on.Fix: decouple the training ISA from the shipped ISA.
scripts/pgo-build.shgainsPGO_TRAIN_EXTRA_RUSTFLAGS(defaults toPGO_EXTRA_RUSTFLAGS), applied to the instrumented build only. The release matrix trains at a runner-safe ISA and ships at the optimized one:linux-x86_64-v4x86-64-v3(AVX2)x86-64-v4(AVX-512)linux-aarch64-neoverse-v1generic(armv8-a)neoverse-v1(SVE)PGO profiles are behavioral (block/branch frequencies) and transfer across
-Ctarget-cpulevels, so the shipped binaries remain fully optimized. Theneoverse-v1entry gets the same split preventively — identical failure mode on any non-SVE aarch64 runner.Validation: ran a full local
train=v3 / ship=v4PGO cycle — completes, ships a working v4-optimized binary (the-pgo-warn-missing-functionnotices are the expected benign cross-ISA function mismatches).rustcconfirmed to accept-Ctarget-cpu=genericfor aarch64; workflow re-validated as valid YAML.Files
src/params/mod.rs—AlignEndsTypetype +--alignEndsType/--outSAMorderparams, validation, testssrc/align/score.rs—align_ends_typeonAlignmentScorersrc/align/stitch.rs—extend_to_endbranch,finalize_transcriptext-flag wiring, testssrc/align/read_align.rs— per-mateimateargs tofinalize_transcriptsrc/chimeric/detect.rs—detect_chimeric_old_impl+diffMates, score-drop comment, testsrc/quant/transcriptome.rs— multi-exon minus-strand projection testscripts/pgo-build.sh,.github/workflows/release.yml— PGO train/ship ISA splitREADME.md—--alignEndsTypeand input-order/--outSAMorderfeature bullets