Repository navigation
FASTA reads, blank lines and mate names as STAR; spill folder; --outBAMcompression -1 - #358
Open
BenjaminDEMAILLE wants to merge 2 commits into
Open
BenjaminDEMAILLE wants to merge 2 commits into
BenjaminDEMAILLE wants to merge 2 commits into
Conversation
…ssion FASTA input is accepted and written with `*` for quality. Trailing blank lines end a file; data after one is an error. `.1`/`.2` after a read number is a mate suffix, and a pair's QNAME is mate 1's name. Sort spills stay in a prefix ending in `/`. `--outBAMcompression -1` compresses like level 1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 2, 2026
A FASTA record with no sequence line (`>e1` followed by `>e2`) was read as an empty read, written with SEQ `*`, and the run exited 0. STAR stops with "short read sequence line: 0" (readLoad.cpp, exit 104). Both readers now stop with the same message; an empty FASTQ sequence too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
Part of #354 (rest of finding 17, finding 18, items 28 and 29).
Stacked on #353, which is stacked on #352, because all of this touches
next_in_current_fileand #352 already adds the quality-length check there. Only the last commit is this PR. I will rebase it onmainonce those two are merged.1. FASTA reads
Problem
--readFilesIn r.fastopped withinvalid name prefix, and no file or read was named, while STAR accepts FASTA.What STAR does
Observed with Homebrew STAR 2.7.11b:
/.*.--outReadsUnmapped Fastxwrites>nameand the sequence.Change
next_in_current_filepeeks at the first byte, and>goes to the newnext_fasta. Quality stays empty.clip_readand three insrc/io/sam.rs, which now usealigned_quality.UnmappedFastqWriter::write_recordwrites FASTA when there is no quality.Measured against STAR
Same index contents, 202 FASTA reads (forward, reverse, one random, one 3 bases long) and 101 FASTA pairs. SAM fields 1-11, sorted:
--outSAMunmapped Within--outSAMunmapped Within--clip5pNbases 3 --clip3pNbases 4--clip3pAdapterSeq ACGTAC--outSAMmode NoQS--outReadsUnmapped Fastx, SE--outReadsUnmapped Fastx, PEBAM Unsorted SortedByCoordinateand--chimOutType WithinBAMalso ran;samtools quickcheckpasses on those files and the unsorted BAM has STAR's 200 records. I did not compare their contents field by field.The one PE difference is a half-mapped pair's extra unmapped-mate record, which has nothing to do with FASTA. It is finding 2 of #354 and happens with FASTQ too.
Not the same as STAR
Unmapped.out.mate1names are>junk 0:N:; here they are>junk. This also happens with FASTQ onmain(@q9 0:N:against@q9), so I left it out of this PR and added it to Tracking: fixes from the whole-repo audit (main at 32fbbc1) #354.--soloTyperun was made with FASTA. A barcode read with no quality gets empty Q30 counts.2. Blank lines
Problem
One blank line after the last record made the whole run exit 1 with
invalid name prefix. It only surfaced at the end of the input, so a long run died at the very end.What STAR does
It stops at the first line starting with a newline or a space, and exits 0. So a trailing blank line is harmless. But
@q1 ... \n\n@q2 ...mapsq1, ignoresq2, and exits 0.Change
Blank lines at the end of a file end it, including
\r\nand spaces. Anything after a blank line is an error that names the file.Deliberate divergence:
DIVERGENCE.md§4.8Reads after a blank line would be lost with exit code 0, so this stops the run where STAR carries on. This needs maintainer sign-off. Following STAR is a two-line change.
3. Mate names and QNAME
Problem
@SRR1.5.1and@SRR1.5.2were refused withread names do not match, while STAR accepts them.While testing that, I found a second difference.
PairedRead.namewas the stripped base name, so@p_1/@p_2got QNAMEp. STAR writes mate 1's name.mainp_1/p_2p_1pp_1S.0.1/S.0.2S.0.1S.0.1Change
strip_mate_suffixstrips.1/.2only when another.comes before it.SRR1.1/SRR1.2are still refused: there the number is the read's, and stripping it would make every out-of-step pair pass the check.PairedRead.nameis mate 1's name.STAR has no such check at all: it exits 0 on mates that are out of step. The check here is older than this PR and is useful, so I kept it. It should have a
DIVERGENCE.mdentry; I did not add one, since this PR only narrows it.4. Sort spill folder
Path::new("out/sub/").parent()isout, so--outFileNamePrefix /data/run1/spilled into/data, againstDIVERGENCE.md§4.3 ("beside the output"). A small or read-only parent would fail the run at its first spill.sort_temp_dirnow returns the prefix itself when it ends in a separator.5.
--outBAMcompression -1bgzf_compressionmapped -1 to no compression, and its comment said that is what STAR does. Measured on 3000 reads:mainSo STAR's -1 is its level 1. Our level 1 is smaller than STAR's because of the deflate library; that is older than this PR. STAR's manual calls -1 "default compression", which is not what the binary does, so I followed the binary and said so in the comment.
Tests
src/io/fastq.rs:fasta_reads_have_empty_quality_and_may_span_lines,blank_lines_end_a_file_but_not_before_more_reads,clip_read_of_a_fasta_read_has_no_quality,dotted_mate_suffix_needs_a_read_number_before_it,pair_is_named_after_mate1_as_star_doessrc/io/bam.rs:minus_one_compresses_like_level_one_and_zero_does_not,spill_files_stay_inside_a_directory_prefixcargo test --release(851 passed, with #352 and #353's tests),cargo clippy --all-targetsandcargo fmt --checkare clean on macOS. I did not runtest/parity_suite.sh. FASTQ input without a blank line takes the old path after one peeked byte; I did not measure that byte's cost, and I expect none, sincefill_bufis what the parser calls next anyway.Second commit, after review
Review found that a FASTA record with no sequence (
>e0,ACGT…,>e1,>e2, …) was read as an empty read: exit 0,e1written with SEQ*. STAR stops withshort read sequence line: 0(readLoad.cpp:35-39, exit 104).cd5e39fstops both readers with the same message, an empty FASTQ sequence included, locked bya_read_with_no_bases_is_an_error. The exit code is 1, as for every error in rustar-aligner. The review confirmed the rest against STAR: mixed FASTA/FASTQ, CRLF FASTA and FASTQ, gzip FASTA with trailing blank lines, andS.i.1/S.i.2names.🤖 Generated with Claude Code