Skip to content

feat(quant): fill in the mapping-output fields SAM allows to be absent (QUAL, M5/UR, MC/MQ, ZW, @RG, unaligned reads) - #1142

Open
BenjaminDEMAILLE wants to merge 5 commits into
COMBINE-lab:developfrom
BenjaminDEMAILLE:feat/mapping-output-completeness
Open

BenjaminDEMAILLE wants to merge 5 commits into
COMBINE-lab:developfrom
BenjaminDEMAILLE:feat/mapping-output-completeness

Conversation

@BenjaminDEMAILLE

Copy link
Copy Markdown
Contributor

Second of three, stacked on #1141. That PR fixes the statements the mapping output makes that are false (a synthesized CIGAR, AS describing a different alignment, MAPQ pinned to 1). This one fills in the fields SAM allows to be absent, which is a weaker claim and deliberately a separate decision: QUAL may be *, and M5, UR, MC, MQ, ZW may simply not be there. The file was still less useful than it had to be, and salmon already had every value in hand.

GitHub cannot point a base branch at a fork, so the diff shown here includes #1141 until that merges. The two commits on top of it are the content of this PR (~1.0k lines).

What gets written

Field Before After
QUAL * / 0xff The FASTQ's Phred+33, reversed alongside SEQ on the reverse strand. Not behind a flag: whenever the input carries qualities, the record does. A quality string whose length disagrees with the read is dropped rather than written (that mismatch is what makes a record unreadable) and warns once.
@SQ M5 absent The MD5 of each reference, which is what ties a BAM to the exact bases it was made against and what CRAM needs. Where indexing had to replace a base, the checksum hashes the N that was there (using ambiguous.bin from #1141), so the header names the reference the user supplied. Matches samtools dict.
@SQ UR absent The index the bases came from, as a file:// URI, the shape samtools writes.
MC / MQ absent The mate's CIGAR and mapping quality, which only became meaningful once the CIGAR was real and MAPQ was not a constant. A mate-aware tool no longer has to sort the file to find them.
ZW absent The placement's equivalence-class weight, which salmon already computed and nothing else in the file reports.
@RG + RG:Z absent Via --rgLine, in the spelling bwa and STAR users already type (tab-separated KEY:value, escaped \t accepted), parsed before any mapping work so a malformed line fails immediately rather than after the run.
@HD VN:1.0 SO:unknown VN:1.6 SO:unsorted GO:query, plus a @CO stating the one ordering guarantee. SO:unknown was not false, but it hid a property the output does have: records for one fragment are contiguous, so a reader can pair mates in one streaming pass instead of sorting first.
unmapped reads absent With --sampleUnaligned: FLAG 0x4, no reference, no position, sequence and qualities intact.

--writeQualities becomes accepted-and-default rather than warned-as-unimplemented. --sampleOut stays unimplemented and still warns.

--sampleUnaligned, and the orphan mate

A BAM containing only the reads that mapped is a record of one filtering decision, not of the experiment, and the reads it drops are exactly the ones anybody debugging a low mapping rate wants. --sampleUnaligned no longer requires --sampleOut; on its own it adds those records to --writeMappings/--writeBam, and warns when there is no mapping output for them to go into.

It also covers an orphan's unmapped mate, which is a read that did not align just as much as one whose whole fragment failed. The mapped mate carries MATE_UNMAPPED, so without the mate's record the file described a read it did not contain. The mate is written at the mapped mate's position so a sort keeps the pair together, and the two records point at each other. Without --sampleUnaligned the mapped record keeps RNEXT '*', since naming a mate that was not written is worse than admitting there is none.

Cost

M5 means hashing the whole transcriptome at startup, a few hundred megabytes of MD5. It is one independent hash per reference, so they run in parallel: 0.16s to 0.08s on a 44 MB transcriptome. QUAL and the extra tags are per-record work already dominated by realization; the --writeBam figures in #1141 (1.94s / 3.28s per 1M fragments) are measured with everything in this PR present.

MD:Z is the largest single contributor to file-size growth and is redundant with the reference, so making it optional is the obvious knob if that matters; not done here.

Verification

Against samtools 1.24, on 20k unique transcripts and 5k genes x 4 isoforms (1M fragments each), human chr22 (hg38) + GENCODE v44, and real reads (DRR028171, HiSeq 2500, 457366 pairs of 151 bases):

  • samtools fastq round-trips the whole real library back out of the BAM: all 457366 reads returned, not one sequence or quality string altered. This is the strongest check here, since it holds SEQ/QUAL orientation to account on every reverse-complemented record.
  • @SQ M5 matches samtools dict, and matches the input FASTA even for the transcript containing an N.
  • samtools quickcheck passes; SAM output round-trips through samtools view -b.
  • Integration tests cover the header (M5, UR, @RG), the tag set, the read group on every record, the orphan mate, and that unaligned records appear only when asked for.
  • Full workspace suite green.

Stack

  1. fix(quant): make each --writeMappings/--writeBam record describe the alignment it reports #1141 the false statements (real CIGAR, NM/MD, AS, MAPQ, ambiguous.bin).
  2. this PR the permitted omissions (above).
  3. feat(quant): --spliced, mapping output in genome coordinates (stacked on #1141, #1142) #1123 genome-coordinate output (--spliced), parked pending the demonstrated-need discussion there.
@rob-p rob-p added the postponed Deferred, not rejected — revisit in a future release cycle label Aug 22, 2026
@rob-p
rob-p marked this pull request as draft August 22, 2026 20:03
rob-p pushed a commit that referenced this pull request Aug 24, 2026
Follow-up to the #1151 review, which landed before the change could go in.

The `nh == 0` arm returned 255. No mapped record reaches it (a fragment with no
placements never enters the record loop), so this is about the record kinds that
have no placement by construction: an unmapped read, once such records are
written (#1142). MAPQ 255 on an unmapped record claims certainty about a
placement that does not exist, and 0 is what samtools convention expects there.

Whoever adds unmapped output should not have to remember this function's edge
case, which is the argument for fixing it now rather than then.

Refs #1151.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
BenjaminDEMAILLE and others added 5 commits August 30, 2026 13:29
Mapping runs ksw2 in KSW_EZ_SCORE_ONLY: it needs the score to rank and filter
placements, not the path that produced it, and skipping the traceback matrix is
the right trade for a kernel that runs once per candidate across the library.
That is also why no CIGAR survives to the record writer, and why the writer has
been synthesizing one from the read length.

This module re-aligns a placement with traceback, on demand, producing the CIGAR
plus the edit distance and MD string that describe it. Nothing calls it yet.

Three things keep it affordable, since it will run once per written record:

* Provably-ungapped fast path. Any alignment containing a gap pays at least
  gap_open + gap_extend and can do no better than match everywhere else, so it
  cannot exceed perfect - (gap_open + gap_extend). The ungapped alignment scores
  perfect - mismatches * (match + mismatch). When the second is at least the
  first, no gapped alignment can win and no DP runs at all. At salmon's defaults
  that admits up to one mismatch, which covers most reads at realistic error
  rates.
* Traceback skipped when the banded optimum equals the ungapped score over the
  full read: filling and walking the traceback matrix is the expensive half.
* Anchor repair by comparison rather than by DP. The mapper's position comes
  from a seed chain, which a mismatch near the 5' end pushes rightwards; sliding
  the read and counting mismatches finds the true start directly, unbounded by
  any band. An exact probe locates candidates first, with the exhaustive sweep
  as the fallback for the fraction of a percent the probe cannot place.

The `ambiguous` slice names reference offsets whose base indexing had to
replace, so NM and MD can report the base the user supplied rather than the
substitute. Callers pass an empty slice until the index records them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Indexing replaces non-ACGT bases with ACGT, as salmon has always done, because
the k-mer structure cannot hold them. Nothing recorded where, so an alignment
over such a position counted a match where the input had an N, and MD named the
substitute instead of the base the user supplied.

The build now writes ambiguous.bin, the offsets it replaced: 40 bytes for the
whole human transcriptome. Alignment still runs against the cleaned sequence,
the only thing it can do, but a record can now report what was actually there.

An index built before this file existed loads as "none recorded" and behaves
exactly as it did, so nothing has to be rebuilt to keep working; rebuilding is
what earns the correction.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
--writeMappings has always been a diagnostic view of salmon's mappings rather
than a general-purpose aligner output, and that is the right scope. But the view
stated things that were not true of the mappings it described.

* The CIGAR was synthesized from the read length: <readLen>M plus a clip at a
  transcript end. A placement with an indel was written as if it had none, so
  every base past the gap sat at the wrong reference offset, and the record
  contradicted its own placement.
* AS was the mapper's chain score, and nothing else in the record could be
  checked against it. Auditing AS against a recomputed NM over a whole output
  found 2.4% of records self-contradicting: a 15M1I1D15M1I1D... sawtooth at
  NM:i:80 on a 100 base read, beside an AS claiming about three mismatches.
* MAPQ was the constant 1, so `samtools view -q 255`, the standard "uniquely
  placed" filter, discarded the entire file.

Records now carry the CIGAR of an alignment actually realized for that
placement, with real I/D operations, the NM and MD that describe it, the score
of that same alignment, and MAPQ derived from the placement count on STAR's
scale (255 unique, 3, 1, 0), which is what the tools downstream already filter
on.

Three separate causes behind the self-contradicting records, each of which alone
produces a false one:

* The anchor search reached only indel_margin. That bounds indel size and says
  nothing about how wrong a seed-derived position can be: a chain on a repeat or
  a paralogous stretch can imply a start most of a read away. One offending read
  belonged 69 bases earlier, where it matched with three mismatches. Out of
  reach, the banded DP still returned an alignment: a sawtooth of one-base
  indels walking the read diagonally across the reference. The search now covers
  a read length either side.
* The search gave up entirely when the read did not fit inside the reference at
  its anchor, which is exactly the case most worth looking at: an anchor near a
  transcript end becomes a soft-clipped record that throws bases away when the
  read often belongs further in and matches in full. "Does not fit" is now
  treated as "matches nothing" so the search runs.
* ksw2's extension pins the read's first base to the window's first base, so a
  realized alignment can grow rightwards but never shift left, and a mismatch
  near the 5' end left the DP no way to say "the read started earlier" except by
  opening a leading insertion: 26I34M at NM 26 where the answer is 60M at NM 1.
  A leading insertion of n bases is itself the statement that the read began n
  bases earlier, so it is also the correction.

A repaired anchor is a hypothesis, not a verdict: a read with a genuine deletion
also looks badly placed to an ungapped scan, so both anchors are realized and
the better score wins. An alignment scoring below the mapper's own
min_accepted_score for the bases it claims is refused, and the record falls back
to the synthesized CIGAR with no NM or MD, which at least says "no base-level
alignment here".

Cost, measured on 1M fragments at -p 8, best of 3: a run without mapping output
is unchanged (0.89s), because nothing is realized unless records are written;
--writeBam goes from 1.04s to 1.94s on a single-mapping fixture and 1.36s to
3.28s at 2.5 placements per fragment, the cost being per record. NM and MD match
samtools calmd across 2M records, including for the transcript containing an N.

QUAL, M5/UR, MC/MQ, ZW, read groups and unmapped records are all still absent.
Those are omissions the format allows, unlike a CIGAR that does not describe the
alignment beside it, and they are the subject of a separate change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Everything here is an omission rather than a falsehood: SAM permits `QUAL` to be
`*`, and `M5`, `UR`, `MC`, `MQ` and `ZW` to be missing. The file was still less
useful than it had to be, and salmon already had every value in hand.

* QUAL, taken from the FASTQ and reversed alongside SEQ on the reverse strand,
  so `samtools fastq` can round-trip the library back out of the BAM. It is not
  behind a flag: whenever the input carries qualities the record does. A quality
  string whose length disagrees with the read is dropped rather than written,
  since that mismatch is what makes a record unreadable, and it warns once.
* `@SQ M5`, the MD5 of each reference, and `UR`, the index it came from. M5 is
  what ties a BAM to the exact bases it was made against, and what CRAM needs.
  Where indexing had to replace a base the checksum hashes the `N` that was
  there, so the header names the reference the user supplied. The whole
  transcriptome is hashed in parallel at startup: 0.16s to 0.08s on a 44 MB
  transcriptome, and it matches `samtools dict`.
* `MC` and `MQ`, the mate's CIGAR and mapping quality, which only became
  meaningful once the CIGAR was real and MAPQ was not a constant. A mate-aware
  tool no longer has to sort the file to find them.
* `ZW`, the placement's equivalence-class weight, which salmon already computed
  and nothing else in the file reports.
* `@RG` and `RG:Z` via `--rgLine`, in the spelling bwa and STAR users already
  type, parsed before any mapping work so a malformed line fails immediately
  rather than after the run. Read groups are how a merged BAM keeps track of
  which sample each record came from.
* `@HD VN:1.6 SO:unsorted GO:query` and a `@CO` stating the one ordering
  guarantee. `SO:unknown` was not false, but it hid a property the output does
  have: records for one fragment are contiguous, so a reader can pair mates in a
  single streaming pass instead of sorting first.

`--writeQualities` becomes accepted-and-default rather than warned-as-missing,
since qualities are now written whenever the input has them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A BAM containing only the reads that mapped is a record of one filtering
decision, not of the experiment, and the reads it drops are exactly the ones
anybody debugging a low mapping rate wants. The format has a place for them:
FLAG 0x4, no reference, no position, sequence and qualities intact.

--sampleUnaligned no longer requires --sampleOut (which remains unimplemented).
On its own it adds those records to --writeMappings/--writeBam, and warns when
there is no mapping output for them to go into.

It also covers an orphan's unmapped mate, which is a read that did not align
just as much as one whose whole fragment failed. The mapped mate carries
MATE_UNMAPPED, so without the mate's record the file described a read it did not
contain: samtools fastq could not round-trip it and anything following the flag
chased nothing. The mate is written at the mapped mate's position, so a sort
keeps the pair together, and the two records point at each other. Without
--sampleUnaligned the mapped record keeps RNEXT '*', since naming a mate that
was not written is worse than admitting there is none.

With this, samtools fastq round-trips a whole real library out of the BAM: all
457366 pairs of DRR028171 returned, not one sequence or quality string altered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@BenjaminDEMAILLE
BenjaminDEMAILLE force-pushed the feat/mapping-output-completeness branch from d30bb1b to f54ac79 Compare August 30, 2026 11:44
@BenjaminDEMAILLE
BenjaminDEMAILLE marked this pull request as ready for review September 26, 2026 11:16

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

postponed Deferred, not rejected — revisit in a future release cycle

2 participants