Skip to content

Commit 6724959

Browse files
pinin4fjordsclaude
andcommitted
fix(sjdb): key SpliceJunctionDb in genome-absolute 0-based coordinates
The DB was keyed in chromosome-local 1-based coords (straight from the GTF) but consulted in genome-absolute 0-based at stitch time, so every sjdb lookup during alignment missed. Annotated splice events never got the sjdb_score bonus and the stricter overhang gate fired in their place, dropping ~50 % of GT/AG splices on the test profile. The stats-time site queried in genome-absolute 1-based-equivalent, so it accidentally matched on chr 0 (chr_start=0) but missed on every other chromosome -- producing inconsistent answers between SJ.out.tab and Log.final.out on the same BAM. Normalise the DB to genome-absolute 0-based at construction, matching the convention prepared_junctions and SpliceJunctionStats already use. Update the stats-time call site to query in the new space. Stitch-time needs no change. Fixes #27 Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 70be24d commit 6724959

5 files changed

Lines changed: 108 additions & 48 deletions

File tree

src/index/mod.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -83,20 +83,13 @@ impl GenomeIndex {
8383
log::info!("Extracted {} annotated junctions from GTF", raw.len());
8484
let jdb = SpliceJunctionDb::from_raw_junctions(&raw);
8585

86-
// `extract_junctions_from_exons` returns chromosome-local 1-based
87-
// intron coordinates (matching STAR's `sjdbList.fromGTF.out.tab`).
88-
// `prepare_junction` + `sjdbInfo.txt` expect 0-based absolute
89-
// genome offsets (matching STAR's `sjdbPrepare.cpp` and
90-
// `detect_splice_motif`'s `genome.sequence[donor_pos]` access),
91-
// so convert here.
9286
let prepared: Vec<PreparedJunction> = raw
9387
.iter()
94-
.map(|&(chr_idx, start_local_1b, end_local_1b, strand)| {
95-
let chr_off = genome.chr_start[chr_idx];
88+
.map(|&(chr_idx, intron_start, intron_end, strand)| {
9689
sjdb_insert::prepare_junction(
9790
chr_idx,
98-
chr_off + start_local_1b - 1,
99-
chr_off + end_local_1b - 1,
91+
intron_start,
92+
intron_end,
10093
strand,
10194
&genome,
10295
n_genome_real,

src/junction/gtf.rs

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,10 @@ fn parse_attributes(attr_str: &str) -> Result<HashMap<String, String>, Error> {
150150
/// `transcript_tag` is the GTF attribute key for the parent transcript
151151
/// (STAR: `sjdbGTFtagExonParentTranscript`, default `"transcript_id"`).
152152
///
153-
/// Returns: Vec<(chr_idx, intron_start, intron_end, strand)>
153+
/// Returns: Vec<(chr_idx, intron_start, intron_end, strand)> where
154+
/// `intron_start` / `intron_end` are genome-absolute 0-based positions
155+
/// of the first and last intronic bases (matching the convention used
156+
/// by `PreparedJunction` and the rest of the alignment pipeline).
154157
pub fn extract_junctions_configured(
155158
exons: Vec<GtfRecord>,
156159
genome: &Genome,
@@ -199,25 +202,27 @@ pub fn extract_junctions_configured(
199202
// Sort exons by position
200203
exons.sort_by_key(|e| e.start);
201204

202-
// Calculate junction coordinates from consecutive exons
205+
let chr_off = genome.chr_start[chr_idx];
206+
203207
for i in 0..exons.len() - 1 {
204208
let exon1 = &exons[i];
205209
let exon2 = &exons[i + 1];
206210

207-
// Intron coordinates (1-based, STAR convention)
208-
let intron_start = exon1.end + 1;
209-
let intron_end = exon2.start - 1;
211+
let intron_start_local_1b = exon1.end + 1;
212+
let intron_end_local_1b = exon2.start.saturating_sub(1);
210213

211-
// Validate junction
212-
if intron_end <= intron_start {
214+
if intron_end_local_1b <= intron_start_local_1b {
213215
log::warn!(
214216
"Invalid junction coordinates: {}-{} (possibly overlapping exons)",
215-
intron_start,
216-
intron_end
217+
intron_start_local_1b,
218+
intron_end_local_1b
217219
);
218220
continue;
219221
}
220222

223+
let intron_start = chr_off + intron_start_local_1b - 1;
224+
let intron_end = chr_off + intron_end_local_1b - 1;
225+
221226
junctions.push((chr_idx, intron_start, intron_end, strand));
222227
}
223228
}
@@ -357,9 +362,9 @@ mod tests {
357362
assert_eq!(junctions.len(), 1);
358363
let (chr_idx, start, end, strand) = junctions[0];
359364
assert_eq!(chr_idx, 0);
360-
assert_eq!(start, 201); // exon1.end + 1
361-
assert_eq!(end, 299); // exon2.start - 1
362-
assert_eq!(strand, 1); // + strand
365+
assert_eq!(start, 200);
366+
assert_eq!(end, 298);
367+
assert_eq!(strand, 1);
363368
}
364369

365370
#[test]
@@ -433,11 +438,8 @@ mod tests {
433438

434439
let junctions = extract_junctions_from_exons(exons, &genome).unwrap();
435440

436-
// Should have 1 unique junction (both transcripts share junction 201-299)
437-
// Note: T1 has 100-200, 300-400 and T2 has 100-200, 300-500
438-
// They share the junction from exon ending at 200 to exon starting at 300
439441
assert_eq!(junctions.len(), 1);
440-
assert_eq!(junctions[0], (0, 201, 299, 1)); // chr0, junction 201-299, strand +
442+
assert_eq!(junctions[0], (0, 200, 298, 1));
441443
}
442444

443445
#[test]
@@ -563,8 +565,8 @@ mod tests {
563565

564566
assert_eq!(junctions.len(), 1);
565567
let (_chr_idx, start, end, _strand) = junctions[0];
566-
assert_eq!(start, 201);
567-
assert_eq!(end, 299);
568+
assert_eq!(start, 200);
569+
assert_eq!(end, 298);
568570
}
569571

570572
#[test]
@@ -642,6 +644,6 @@ mod tests {
642644

643645
let junctions = extract_junctions_configured(exons, &genome, "Parent").unwrap();
644646
assert_eq!(junctions.len(), 1);
645-
assert_eq!(junctions[0], (0, 201, 299, 1));
647+
assert_eq!(junctions[0], (0, 200, 298, 1));
646648
}
647649
}

src/junction/mod.rs

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,12 @@ use crate::genome::Genome;
1919
use std::collections::HashMap;
2020
use std::path::Path;
2121

22-
/// Key for junction lookup: (chr_idx, intron_start, intron_end, strand)
22+
/// Key for junction lookup: (chr_idx, intron_start, intron_end, strand).
23+
///
24+
/// `intron_start` / `intron_end` are genome-absolute 0-based positions of
25+
/// the first and last intronic bases, matching the convention used by
26+
/// `PreparedJunction`, `SpliceJunctionStats`, and the alignment-time
27+
/// `genome_pos` variables.
2328
#[derive(Hash, Eq, PartialEq, Clone, Debug)]
2429
struct JunctionKey {
2530
chr_idx: usize,
@@ -102,12 +107,12 @@ impl SpliceJunctionDb {
102107
Self { junctions }
103108
}
104109

105-
/// Check if a junction is annotated in the GTF
110+
/// Check if a junction is annotated in the GTF.
106111
///
107112
/// # Arguments
108113
/// * `chr_idx` - Chromosome index
109-
/// * `start` - Intron start position (last exon base + 1)
110-
/// * `end` - Intron end position (first exon base of next exon - 1)
114+
/// * `start` - Genome-absolute 0-based position of the first intronic base
115+
/// * `end` - Genome-absolute 0-based position of the last intronic base
111116
/// * `strand` - Strand (0=unknown, 1=+, 2=-)
112117
///
113118
/// # Returns
@@ -397,4 +402,60 @@ mod tests {
397402
assert_eq!(novel_junctions.len(), 1);
398403
assert_eq!(novel_junctions[0].0.intron_start, 300);
399404
}
405+
406+
#[test]
407+
fn test_db_keyed_in_genome_absolute_zero_based_multi_chr() {
408+
use crate::junction::gtf::{GtfRecord, extract_junctions_configured};
409+
use std::collections::HashMap;
410+
411+
// Two-chromosome toy genome so chr_start[1] != 0.
412+
let genome = Genome {
413+
sequence: vec![0; 4000],
414+
n_genome: 2000,
415+
n_chr_real: 2,
416+
chr_start: vec![0, 1000, 2000],
417+
chr_length: vec![1000, 1000],
418+
chr_name: vec!["chr1".to_string(), "chr2".to_string()],
419+
};
420+
421+
let make_exon = |seqname: &str, start: u64, end: u64, transcript: &str| -> GtfRecord {
422+
let mut attrs: HashMap<String, String> = HashMap::new();
423+
attrs.insert("gene_id".to_string(), "G".to_string());
424+
attrs.insert("transcript_id".to_string(), transcript.to_string());
425+
GtfRecord {
426+
seqname: seqname.to_string(),
427+
feature: "exon".to_string(),
428+
start,
429+
end,
430+
strand: '+',
431+
attributes: attrs,
432+
}
433+
};
434+
435+
let exons = vec![
436+
make_exon("chr1", 100, 200, "T1"),
437+
make_exon("chr1", 300, 400, "T1"),
438+
make_exon("chr2", 100, 200, "T2"),
439+
make_exon("chr2", 300, 400, "T2"),
440+
];
441+
442+
let raw = extract_junctions_configured(exons, &genome, "transcript_id").unwrap();
443+
let db = SpliceJunctionDb::from_raw_junctions(&raw);
444+
assert_eq!(db.len(), 2);
445+
446+
// Junction on chr1: intron local 1-based 201..299
447+
// → genome-absolute 0-based: chr_start[0] + 200 .. chr_start[0] + 298
448+
assert!(db.is_annotated(0, 200, 298, 1));
449+
// Off-by-one in either direction must miss.
450+
assert!(!db.is_annotated(0, 201, 299, 1));
451+
assert!(!db.is_annotated(0, 199, 297, 1));
452+
453+
// Junction on chr2: same chr-local coords, but chr_start[1] = 1000
454+
// → genome-absolute 0-based: 1200 .. 1298
455+
assert!(db.is_annotated(1, 1200, 1298, 1));
456+
// The pre-fix chr-local 1-based key (201, 299) must not match on chr2.
457+
assert!(!db.is_annotated(1, 201, 299, 1));
458+
// The pre-fix stitch-time off-by-one (1199, 1297) must not match either.
459+
assert!(!db.is_annotated(1, 1199, 1297, 1));
460+
}
400461
}

src/junction/sj_output.rs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@ use std::io::{BufWriter, Write};
2121
use std::path::Path;
2222
use std::sync::atomic::{AtomicU32, Ordering};
2323

24-
/// Key for junction statistics
24+
/// Key for junction statistics.
25+
///
26+
/// `intron_start` / `intron_end` are genome-absolute 0-based positions of
27+
/// the first and last intronic bases, matching `SpliceJunctionDb` and
28+
/// `PreparedJunction`.
2529
#[derive(Hash, Eq, PartialEq, Clone, Debug)]
2630
pub(crate) struct SjKey {
2731
pub chr_idx: usize,
@@ -74,12 +78,12 @@ impl SpliceJunctionStats {
7478
}
7579
}
7680

77-
/// Record a junction from alignment (thread-safe)
81+
/// Record a junction from alignment (thread-safe).
7882
///
7983
/// # Arguments
8084
/// * `chr_idx` - Chromosome index
81-
/// * `start` - Intron start (1-based)
82-
/// * `end` - Intron end (1-based)
85+
/// * `start` - Genome-absolute 0-based position of the first intronic base
86+
/// * `end` - Genome-absolute 0-based position of the last intronic base
8387
/// * `strand` - Strand (0=unknown, 1=+, 2=-)
8488
/// * `motif` - Splice motif
8589
/// * `is_unique` - True if from unique mapping, false if multi-mapping
@@ -260,8 +264,8 @@ impl SpliceJunctionStats {
260264
.ok_or_else(|| Error::Index("Invalid chromosome index in junction".to_string()))?;
261265

262266
let chr_start_pos = genome.chr_start[key.chr_idx];
263-
let chr_pos_start = key.intron_start - chr_start_pos;
264-
let chr_pos_end = key.intron_end - chr_start_pos;
267+
let chr_pos_start = key.intron_start - chr_start_pos + 1;
268+
let chr_pos_end = key.intron_end - chr_start_pos + 1;
265269

266270
writeln!(
267271
writer,
@@ -538,8 +542,8 @@ mod tests {
538542
// First junction (sorted by position)
539543
let fields1: Vec<&str> = lines[0].split('\t').collect();
540544
assert_eq!(fields1[0], "chr1"); // chr
541-
assert_eq!(fields1[1], "100"); // start
542-
assert_eq!(fields1[2], "200"); // end
545+
assert_eq!(fields1[1], "101"); // chr-local 1-based start
546+
assert_eq!(fields1[2], "201"); // chr-local 1-based end
543547
assert_eq!(fields1[3], "1"); // strand
544548
assert_eq!(fields1[4], "1"); // motif (GT/AG)
545549
assert_eq!(fields1[5], "0"); // not annotated
@@ -550,8 +554,8 @@ mod tests {
550554
// Second junction (annotated, bypasses filters)
551555
let fields2: Vec<&str> = lines[1].split('\t').collect();
552556
assert_eq!(fields2[0], "chr1");
553-
assert_eq!(fields2[1], "300");
554-
assert_eq!(fields2[2], "400");
557+
assert_eq!(fields2[1], "301");
558+
assert_eq!(fields2[2], "401");
555559
assert_eq!(fields2[3], "2"); // - strand
556560
assert_eq!(fields2[4], "3"); // motif (GC/AG, direct encoding)
557561
assert_eq!(fields2[5], "1"); // annotated
@@ -597,7 +601,7 @@ mod tests {
597601
// Canonical (overhang 20 >= 12) should pass
598602
assert_eq!(lines.len(), 1);
599603
let fields: Vec<&str> = lines[0].split('\t').collect();
600-
assert_eq!(fields[1], "300"); // Only the canonical junction remains
604+
assert_eq!(fields[1], "301"); // Only the canonical junction remains
601605
}
602606

603607
#[test]

src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -793,8 +793,8 @@ fn extract_junction_keys(
793793
match op {
794794
CigarOp::RefSkip(len) => {
795795
let intron_len = *len;
796-
let intron_start = genome_pos + 1;
797-
let intron_end = genome_pos + intron_len as u64;
796+
let intron_start = genome_pos;
797+
let intron_end = genome_pos + intron_len as u64 - 1;
798798

799799
let motif = scorer.detect_splice_motif(genome_pos, intron_len, &index.genome);
800800
let strand = match motif.implied_strand() {
@@ -1857,8 +1857,8 @@ fn record_transcript_junctions(
18571857
CigarOp::RefSkip(len) => {
18581858
// This is a splice junction
18591859
let intron_len = *len;
1860-
let intron_start = genome_pos + 1; // 1-based, first intronic base
1861-
let intron_end = genome_pos + intron_len as u64; // 1-based, last intronic base
1860+
let intron_start = genome_pos;
1861+
let intron_end = genome_pos + intron_len as u64 - 1;
18621862

18631863
// Detect splice motif
18641864
let motif = scorer.detect_splice_motif(genome_pos, intron_len, &index.genome);

0 commit comments

Comments
 (0)